From d099e7b16d7125b90db6985d0fd317b6145da536 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Fri, 16 Jan 2015 15:55:06 +0100 Subject: [PATCH] Improve handling of InterruptedException InterruptedExceptions should be treated as the users intention to 'cancel' the current thread's task. There is no such thing as a spurious interrupt (not to be confused with "spurious wakeups"). --- .../org/jivesoftware/smack/ReconnectionManager.java | 10 ++++++---- .../src/main/java/org/jivesoftware/smack/Roster.java | 3 ++- .../java/org/jivesoftware/smack/DummyConnection.java | 1 - .../smack/test/util/WaitForPacketListener.java | 1 - .../org/jivesoftware/smack/tcp/XMPPTCPConnection.java | 11 +++++++---- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/smack-core/src/main/java/org/jivesoftware/smack/ReconnectionManager.java b/smack-core/src/main/java/org/jivesoftware/smack/ReconnectionManager.java index b52c8873a..7065f8c8c 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/ReconnectionManager.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/ReconnectionManager.java @@ -156,13 +156,15 @@ public class ReconnectionManager { } } catch (InterruptedException e) { - // We don't need to handle spurious interrupts, in the worst case, this will cause to - // reconnect a few seconds earlier, depending on how many (spurious) interrupts arrive while - // sleep() is called. - LOGGER.log(Level.FINE, "Supurious interrupt", e); + LOGGER.log(Level.FINE, "waiting for reconnection interrupted", e); + break; } } + for (ConnectionListener listener : connection.connectionListeners) { + listener.reconnectingIn(0); + } + // Makes a reconnection attempt try { if (isReconnectionPossible(connection)) { diff --git a/smack-core/src/main/java/org/jivesoftware/smack/Roster.java b/smack-core/src/main/java/org/jivesoftware/smack/Roster.java index 012c1ccaa..d14b90870 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/Roster.java +++ b/smack-core/src/main/java/org/jivesoftware/smack/Roster.java @@ -248,7 +248,8 @@ public class Roster { } } catch (InterruptedException e) { - LOGGER.log(Level.FINE, "spurious interrupt", e); + LOGGER.log(Level.FINE, "interrupted", e); + break; } long now = System.currentTimeMillis(); waitTime -= now - start; diff --git a/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java b/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java index 04dfbc7e7..8dbd8eec5 100644 --- a/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java +++ b/smack-core/src/test/java/org/jivesoftware/smack/DummyConnection.java @@ -201,7 +201,6 @@ public class DummyConnection extends AbstractXMPPConnection { return (P) queue.poll(wait, TimeUnit.SECONDS); } catch (InterruptedException e) { - // TODO handle spurious interrupts throw new IllegalStateException(e); } } diff --git a/smack-core/src/test/java/org/jivesoftware/smack/test/util/WaitForPacketListener.java b/smack-core/src/test/java/org/jivesoftware/smack/test/util/WaitForPacketListener.java index 16c906570..095582ee0 100644 --- a/smack-core/src/test/java/org/jivesoftware/smack/test/util/WaitForPacketListener.java +++ b/smack-core/src/test/java/org/jivesoftware/smack/test/util/WaitForPacketListener.java @@ -49,7 +49,6 @@ public class WaitForPacketListener implements PacketListener { } } catch (InterruptedException e) { - // TODO better handling of spurious interrupts throw new IllegalStateException(e); } } diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java index a8d3c934f..3e71b3c27 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java @@ -1216,9 +1216,11 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { } catch (InterruptedException e) { throwNotConnectedExceptionIfDoneAndResumptionNotPossible(); - // If the method above did not throw, we have a spurious interrupt and we should try to enqueue the - // element again - LOGGER.log(Level.FINE, "Spurious interrupt", e); + // If the method above did not throw, then the sending thread was interrupted + // TODO in a later version of Smack the InterruptedException should be thrown to + // allow users to interrupt a sending thread that is currently blocking because + // the queue is full. + LOGGER.log(Level.WARNING, "Sending thread was interrupted", e); } } } @@ -1253,7 +1255,8 @@ public class XMPPTCPConnection extends AbstractXMPPConnection { } catch (InterruptedException e) { if (!queue.isShutdown()) { - LOGGER.log(Level.FINER, "Spurious interrupt", e); + // Users shouldn't try to interrupt the packet writer thread + LOGGER.log(Level.WARNING, "Packet writer thread was interrupted. Don't do that. Use disconnect() instead.", e); } } return packet;