Browse Source

StreamCopyThread: Do not drop data when flush is observed before writing

StreamCopyThread.flush was introduced in
61645b938bc934fda3b0624c5bac1e3495634750 (Add timeouts to smart
transport clients, 2009-06-19) to support timeouts on write in JSch.
The commit message from that change explains:

   JSch made a timeout on write difficult because they explicitly do
   a catch for InterruptedException inside of their OutputStream.  We
   have to work around that by creating an additional thread that just
   shuttles data between our own OutputStream and the real JSch stream.

The code that runs on that thread is structured as follows:

	while (!done) {
		int n = src.read(buf);
		dst.write(buf, 0, n);
	}

with src being a PipedInputStream representing the data to be written
to JSch.  To add flush support, that change wanted to add an extra step

		if (wantFlush)
			dst.flush();

but to handle the case where the thread is blocked in the read() call
waiting for new input, it needs to interrupt the read. So that is how
it works: the caller runs

	pipeOut.write(some data);
	pipeOut.flush();
	copyThread.flush();

to write some data and force it to flush by interrupting the read.

After the pipeOut.flush(), the StreamCopyThread reads the data that was
written and prepares to copy it out.  If the copyThread.flush() call
interrupts the copyThread before it acquires writeLock and starts
writing, we throw away the data we just read to fulfill the flush.
Oops.

Noticed during the review of e67d59df3f
(StreamCopyThread: Do not let flush interrupt a write, 2016-11-04),
which introduced this bug.

Change-Id: I4aceb5610e1bfb251046097adf46bca54bc1d998
stable-4.6
Jonathan Nieder 8 years ago
parent
commit
881e6b2cbb
  1. 6
      org.eclipse.jgit/src/org/eclipse/jgit/util/io/StreamCopyThread.java

6
org.eclipse.jgit/src/org/eclipse/jgit/util/io/StreamCopyThread.java

@ -155,11 +155,7 @@ public class StreamCopyThread extends Thread {
break;
synchronized (writeLock) {
if (isInterrupted()) {
continue;
}
boolean writeInterrupted = false;
boolean writeInterrupted = Thread.interrupted();
for (;;) {
try {
dst.write(buf, 0, n);

Loading…
Cancel
Save