Swiftdiff options
| author | Nick Hudson <nick.hudson@isode.com> | 2014-10-24 15:28:28 (GMT) |
|---|---|---|
| committer | Nick Hudson <nick.hudson@isode.com> | 2014-10-24 16:02:48 (GMT) |
| commit | 244aff320257d178bbe35d87b0e09d939bd2f893 (patch) | |
| tree | 6175d827be9bf72629016fb5ae166432ee65f94f | |
| parent | dabadc693220858ecb34af7e8df5f4e0b32461fc (diff) | |
| download | stroke-244aff320257d178bbe35d87b0e09d939bd2f893.zip stroke-244aff320257d178bbe35d87b0e09d939bd2f893.tar.bz2 | |
Don't disregard data that arrives on network just prior to socket closing
The JavaConnection code which reads from a socket detects a socket
closure and emits a disconnected signal.
It was noticed that on some occasions, data was arriving on the socket
just before it was closed, and this data was never passed to the
application.
This happens when the server writes e.g. a "BYE" message and closes
the socket straight away: when JavaConnection is woken to read the
message, it does so and then goes on to notice that the connection has
been closed and throws an IOException without passing the message back
to the application.
This patch fixes the problem by making sure that any data read prior
to the close being noticed is sent to the application before the closed
signal is emitted
Test-information:
It was possible to provoke the problem by deliberately breaking socket
connections - if you do this often enough you see cases where data
read from the socket is lost.
After this patch, such cases do not result in data loss.
Also tested with email client and verified that connections to
icloud.com which previously had provoked this problem when
authentication failed now seem to return all data reliably.
Change-Id: Ieba0f4186b7c91e55f5f1a4b3b64bc923006b933
| -rw-r--r-- | src/com/isode/stroke/network/JavaConnection.java | 72 |
1 files changed, 55 insertions, 17 deletions
diff --git a/src/com/isode/stroke/network/JavaConnection.java b/src/com/isode/stroke/network/JavaConnection.java index 807d971..46f43ab 100644 --- a/src/com/isode/stroke/network/JavaConnection.java +++ b/src/com/isode/stroke/network/JavaConnection.java @@ -27,6 +27,26 @@ import com.isode.stroke.eventloop.EventLoop; import com.isode.stroke.eventloop.EventOwner; public class JavaConnection extends Connection implements EventOwner { + + /** + * Wrapper class that is used by the "doRead" method so that it can + * return both a ByteArray and an indication of whether the socket + * got closed. + */ + private static class ReadResult { + public ByteArray dataRead_; + public boolean socketClosed_; + + ReadResult(boolean socketClosed) { + dataRead_ = new ByteArray(); + socketClosed_ = socketClosed; + } + + ReadResult(ByteArrayOutputStream byteArrayOutputStream, boolean socketClosed) { + dataRead_ = new ByteArray(byteArrayOutputStream.toByteArray()); + socketClosed_ = socketClosed; + } + } private class Worker implements Runnable { @@ -96,12 +116,12 @@ public class JavaConnection extends Connection implements EventOwner { ByteArray dataRead; if (readNeeded) { - try { - dataRead = doRead(); - if (!dataRead.isEmpty()) { - handleDataRead(dataRead); - } - } catch (IOException ex) { + ReadResult rr = doRead(); + dataRead = rr.dataRead_; + if (!dataRead.isEmpty()) { + handleDataRead(dataRead); + } + if (rr.socketClosed_) { handleDisconnected(Error.ReadError); return; } @@ -210,19 +230,31 @@ public class JavaConnection extends Connection implements EventOwner { } /** - * Called when there's something that's come in on the socket - * @return a ByteBuffer containing bytes read (may be empty, won't be null) - * @throws IOException if the socket got closed + * Called when there's something that's come in on the socket. The ReadResult + * returned will contain any data that was read from the socket and a + * flag to say whether the socket has been closed. + * <p>If the socket has been closed, it may still be the case that data + * was read before the close happened. + * @return a ReadResult containing bytes read (may be empty, won't be null), + * and an indication of whether the was closed. */ - private ByteArray doRead() throws IOException { + private ReadResult doRead() { ByteBuffer byteBuffer = ByteBuffer.allocate(1024); - int count = socketChannel_.read(byteBuffer); + int count; + try { + count = socketChannel_.read(byteBuffer); + } + catch (IOException e) { + // Nothing read and the socket's closed + return new ReadResult(true); + } if (count == 0) { - return new ByteArray(); + // Nothing read, but socket's open + return new ReadResult(false); } - + boolean isClosed = false; ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(1024); @@ -231,18 +263,24 @@ public class JavaConnection extends Connection implements EventOwner { byte[] result = new byte[byteBuffer.remaining()]; byteBuffer.get(result); byteBuffer.compact(); - byteArrayOutputStream.write(result); - count = socketChannel_.read(byteBuffer); + try { + byteArrayOutputStream.write(result); + count = socketChannel_.read(byteBuffer); + } + catch (IOException e) { + // Force exit from loop and indicate socket closed + count = -1; + } } if (count == -1) { /* socketChannel input has reached "end-of-stream", which * we regard as meaning that the socket has been closed */ - throw new IOException("socketChannel_.read returned -1"); + isClosed = true; } /* There is no need to close the ByteArrayOutputStream */ - return new ByteArray(byteArrayOutputStream.toByteArray()); + return new ReadResult(byteArrayOutputStream, isClosed); } |