Swiftdiff options
| author | Alan Young <consult.awy@gmail.com> | 2015-04-28 10:28:12 (GMT) |
|---|---|---|
| committer | Alan Young <consult.awy@gmail.com> | 2015-05-01 08:17:31 (GMT) |
| commit | cd8e073cabc9a39abb14527b6e968ece1586b7ec (patch) | |
| tree | ae2655de2bc46e632e6f75a6150d742b26813d00 | |
| parent | de8538282dfbb73212fa241638147a64bbe9cee5 (diff) | |
| download | stroke-cd8e073cabc9a39abb14527b6e968ece1586b7ec.zip stroke-cd8e073cabc9a39abb14527b6e968ece1586b7ec.tar.bz2 | |
Revisit handling of JavaConnection race conditions.
JavaConnection.disconnect() can be called at any time when opening or
using the connection. Its use can interfere with tests for
selector_.isOpen(). Protect relevant code by synchronization blocks.
This should not cause any contended synchronization issue as contention
can only occur while disconnecting. This reverts the relevant part of
7d2101b9.
Move selector_.select() call to end of Worker.run() while loop so that
any write that is done while still connecting (if that is permitted and
possible), and before selector_ first become non-null, is still picked
up.
Remove redundant checks on disconnecting_ and calls to
handleDisconnected(null).
Move a couple of fields which are only used in Worker nested class into
Worker.
Update copyright.
Change-Id: I2eabad79c69fe4e9206942c8025e0ac012bffdb0
| -rw-r--r-- | src/com/isode/stroke/network/JavaConnection.java | 64 |
1 files changed, 32 insertions, 32 deletions
diff --git a/src/com/isode/stroke/network/JavaConnection.java b/src/com/isode/stroke/network/JavaConnection.java index 9b171d9..34f332b 100644 --- a/src/com/isode/stroke/network/JavaConnection.java +++ b/src/com/isode/stroke/network/JavaConnection.java @@ -1,8 +1,4 @@ /* - * Copyright (c) 2010 Remko Tronçon - * All rights reserved. - */ -/* * Copyright (c) 2010-2015, Isode Limited, London, England. * All rights reserved. */ @@ -52,6 +48,8 @@ public class JavaConnection extends Connection implements EventOwner { private final HostAddressPort address_; private final List<byte[]> writeBuffer_ = Collections.synchronizedList(new ArrayList<byte[]>()); + private SelectionKey selectionKey_; + private boolean disconnected_ = false; public Worker(HostAddressPort address) { address_ = address; @@ -77,26 +75,9 @@ public class JavaConnection extends Connection implements EventOwner { return; } handleConnected(false); + while (!disconnecting_) { - - /* This will block until something is ready on the selector, - * including someone calling selector.wakeup(), or until the - * thread is interrupted - */ - try { - selector_.select(); - } catch (IOException e) { - disconnected_ = true; - handleDisconnected(null); - break; - } - /* Something(s) happened. See what needs doing */ - if (disconnecting_) { - handleDisconnected(null); - /* No point doing anything else */ - break; - } boolean writeNeeded = isWriteNeeded(); boolean readNeeded = selectionKey_.isReadable(); @@ -148,7 +129,18 @@ public class JavaConnection extends Connection implements EventOwner { */ selector_.wakeup(); } - } + + /* This will block until something is ready on the selector, + * including someone calling selector.wakeup(), or until the + * thread is interrupted + */ + try { + selector_.select(); + } catch (IOException e) { + disconnected_ = true; + break; + } + } handleDisconnected(null); } finally { if(socketChannel_ != null) { @@ -159,8 +151,9 @@ public class JavaConnection extends Connection implements EventOwner { } if(selector_ != null) { try { - selector_.close(); - selector_ = null; + synchronized (selectorLock_) { + selector_.close(); + } } catch (IOException e) { } } @@ -343,8 +336,12 @@ public class JavaConnection extends Connection implements EventOwner { disconnecting_ = true; // Check "isOpen" to Avoid Android crash see // https://code.google.com/p/android/issues/detail?id=80785 - if (selector_ != null && selector_.isOpen()) { - selector_.wakeup(); + if (selector_ != null) { + synchronized (selectorLock_) { + if (selector_.isOpen()) { + selector_.wakeup(); + } + } } } @@ -353,8 +350,12 @@ public class JavaConnection extends Connection implements EventOwner { worker_.writeBuffer_.add(data.getData()); // Check "isOpen" to Avoid Android crash see // https://code.google.com/p/android/issues/detail?id=80785 - if (selector_ != null && selector_.isOpen()) { - selector_.wakeup(); + if (selector_ != null) { + synchronized (selectorLock_) { + if (selector_.isOpen()) { + selector_.wakeup(); + } + } } } @@ -381,10 +382,9 @@ public class JavaConnection extends Connection implements EventOwner { private final EventLoop eventLoop_; private boolean disconnecting_ = false; - private boolean disconnected_ = false; private SocketChannel socketChannel_; - private volatile Selector selector_; - private SelectionKey selectionKey_; + private Selector selector_; + private final Object selectorLock_ = new Object(); // use private lock object that is not visible elsewhere private Worker worker_; } |