[FIXED JENKINS-7871] Fixed a race condition in FilePath.copyTo

that results in missing tail bits when the master is heavily loaded.
This commit is contained in:
Kohsuke Kawaguchi committed 2011-04-21 13:39:48 -07:00
1 parent 5d413e9e95
commit 957ffd3009
5 files changed
+125 -9

No files matched your search

@@ -23,12 +23,24 @@
*/
package hudson;
import hudson.remoting.Future;
import hudson.remoting.VirtualChannel;
import hudson.util.IOException2;
import hudson.util.NullStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import org.apache.commons.io.output.CountingOutputStream;
import org.apache.commons.io.output.NullOutputStream;
import org.jvnet.hudson.test.Bug;
/**
* @author Kohsuke Kawaguchi
@@ -61,6 +73,89 @@ public class FilePathTest extends ChannelTestCase {
}
}
/**
* As we moved the I/O handling to another thread, there's a race condition in
* {@link FilePath#copyTo(OutputStream)} — this method can return before
* all the writes are delivered to {@link OutputStream}.
*
* <p>
* To reproduce that problem, we use a large number of threads, so that we can
* maximize the chance of out-of-order execution, and make sure we are
* seeing the right byte count at the end.
*
* Also see JENKINS-7897
*/
@Bug(7871)
public void testCopyTo3() throws Exception {
final File tmp = File.createTempFile("testCopyTo3","");
FileOutputStream os = new FileOutputStream(tmp);
final int size = 90000;
byte[] buf = new byte[size];
for (int i=0; i<buf.length; i++)
buf[i] = (byte)(i%256);
os.write(buf);
os.close();
ExecutorService es = Executors.newFixedThreadPool(100);
try {
List<java.util.concurrent.Future> r = new ArrayList<java.util.concurrent.Future>();
for (int i=0; i<100; i++) {
r.add(es.submit(new Callable<Object>() {
public Object call() throws Exception {
class Sink extends OutputStream {
private Exception closed;
private volatile int count;
private void checkNotClosed() throws IOException2 {
if (closed != null)
throw new IOException2(closed);
}
@Override
public void write(int b) throws IOException {
count++;
checkNotClosed();
}
@Override
public void write(byte[] b) throws IOException {
count+=b.length;
checkNotClosed();
}
@Override
public void write(byte[] b, int off, int len) throws IOException {
count+=len;
checkNotClosed();
}
@Override
public void close() throws IOException {
closed = new Exception();
if (size!=count)
fail();
}
}
FilePath f = new FilePath(french, tmp.getPath());
Sink sink = new Sink();
f.copyTo(sink);
assertEquals(size,sink.count);
return null;
}
}));
}
for (java.util.concurrent.Future f : r)
f.get();
} finally {
es.shutdown();
}
}
public void testRepeatCopyRecursiveTo() throws Exception {
// local->local copy used to return 0 if all files were "up to date"
// should return number of files processed, whether or not they were copied or already current