From cd8e073cabc9a39abb14527b6e968ece1586b7ec Mon Sep 17 00:00:00 2001 From: Alan Young Date: Tue, 28 Apr 2015 12:28:12 +0200 Subject: 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 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 writeBuffer_ = Collections.synchronizedList(new ArrayList()); + 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_; } -- cgit v0.10.2-6-g49f6