diff options
author | Jonathan Nieder <jrn@google.com> | 2016-11-04 15:59:32 -0700 |
---|---|---|
committer | Jonathan Nieder <jrn@google.com> | 2016-11-04 19:33:02 -0400 |
commit | 881e6b2cbbba61669f9ac83e357f141743c5305a (patch) | |
tree | dbe89213e9a85fe2eef6f80606ef102bc7e453cf | |
parent | e67d59df3fb8ae1efe94a54efce36784f7cc83e8 (diff) | |
download | jgit-881e6b2cbbba61669f9ac83e357f141743c5305a.tar.gz jgit-881e6b2cbbba61669f9ac83e357f141743c5305a.zip |
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 e67d59df3fb8ae1efe94a54efce36784f7cc83e8
(StreamCopyThread: Do not let flush interrupt a write, 2016-11-04),
which introduced this bug.
Change-Id: I4aceb5610e1bfb251046097adf46bca54bc1d998
-rw-r--r-- | org.eclipse.jgit/src/org/eclipse/jgit/util/io/StreamCopyThread.java | 6 |
1 files changed, 1 insertions, 5 deletions
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/io/StreamCopyThread.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/io/StreamCopyThread.java index 3ee1a2d6fa..ae760e3b8e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/io/StreamCopyThread.java +++ b/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); |