From 07d6b9203cc76fa0b3294441902d2c7b0cb050ff Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 5 May 2014 17:45:40 +0200 Subject: [PATCH] Synchronize PacketWriter shutdown try the best to send the queue elements and the closing stream element. --- .../org/jivesoftware/smack/PacketWriter.java | 26 +++++++++++++++++-- .../jivesoftware/smack/PacketWriterTest.java | 7 ++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/PacketWriter.java b/smack-tcp/src/main/java/org/jivesoftware/smack/PacketWriter.java index 52bfb62e3..18f7b555a 100644 --- a/smack-tcp/src/main/java/org/jivesoftware/smack/PacketWriter.java +++ b/smack-tcp/src/main/java/org/jivesoftware/smack/PacketWriter.java @@ -23,6 +23,8 @@ import org.jivesoftware.smack.util.ArrayBlockingQueueWithShutdown; import java.io.IOException; import java.io.Writer; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -48,6 +50,8 @@ class PacketWriter { volatile boolean done; + AtomicBoolean shutdownDone = new AtomicBoolean(false); + /** * Creates a new packet writer with the specified connection. * @@ -65,6 +69,7 @@ class PacketWriter { protected void init() { this.writer = connection.writer; done = false; + shutdownDone.set(false); queue.start(); writerThread = new Thread() { @@ -115,6 +120,16 @@ class PacketWriter { public void shutdown() { done = true; queue.shutdown(); + synchronized(shutdownDone) { + if (!shutdownDone.get()) { + try { + shutdownDone.wait(connection.getPacketReplyTimeout()); + } + catch (InterruptedException e) { + LOGGER.log(Level.WARNING, "shutdown", e); + } + } + } } /** @@ -163,7 +178,7 @@ class PacketWriter { writer.flush(); } catch (Exception e) { - LOGGER.warning("Error flushing queue during shutdown, ignore and continue"); + LOGGER.log(Level.WARNING, "Exception flushing queue during shutdown, ignore and continue", e); } // Delete the queue contents (hopefully nothing is left). @@ -175,7 +190,8 @@ class PacketWriter { writer.flush(); } catch (Exception e) { - // Do nothing + LOGGER.log(Level.WARNING, "Exception writing closing stream element", e); + } finally { try { @@ -185,6 +201,11 @@ class PacketWriter { // Do nothing } } + + shutdownDone.set(true); + synchronized(shutdownDone) { + shutdownDone.notify(); + } } catch (IOException ioe) { // The exception can be ignored if the the connection is 'done' @@ -213,4 +234,5 @@ class PacketWriter { writer.write(stream.toString()); writer.flush(); } + } diff --git a/smack-tcp/src/test/java/org/jivesoftware/smack/PacketWriterTest.java b/smack-tcp/src/test/java/org/jivesoftware/smack/PacketWriterTest.java index 8748049e4..8d78c3728 100644 --- a/smack-tcp/src/test/java/org/jivesoftware/smack/PacketWriterTest.java +++ b/smack-tcp/src/test/java/org/jivesoftware/smack/PacketWriterTest.java @@ -83,6 +83,8 @@ public class PacketWriterTest { // Not really cool, but may increases the chances for 't' to block in sendPacket. Thread.sleep(250); + // Set to true for testing purposes, so that shutdown() won't wait packet writer + pw.shutdownDone.set(true); // Shutdown the packetwriter pw.shutdown(); shutdown = true; @@ -90,13 +92,16 @@ public class PacketWriterTest { if (prematureUnblocked) { fail("Should not unblock before the thread got shutdown"); } + synchronized (t) { + t.notify(); + } } public class BlockingStringWriter extends Writer { @Override public void write(char[] cbuf, int off, int len) throws IOException { try { - Thread.sleep(Integer.MAX_VALUE); + wait(); } catch (InterruptedException e) { }