From 25b3f354216683e568a54b93be1c6a931cd79b26 Mon Sep 17 00:00:00 2001
From: Florian Schmaus <flo@geekplace.eu>
Date: Thu, 28 Mar 2019 13:52:17 +0100
Subject: [PATCH] Ensure that shutdown() terminates reader/writer threads

In case an exception happens in connect()/login() the
'disconnectedButResumable' boolean may (still) be set. Which causes
only one of the reader and writer threads to exit, typically the
reader thread, because shutdown() will bail out very early. This
leaves a dangling (writer) thread causing memory leaks and deadlocks
on a subsequent connect()/login().
---
 .../smack/tcp/XMPPTCPConnection.java          | 24 +++++++++++--------
 1 file changed, 14 insertions(+), 10 deletions(-)

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 2441845c6..a08221ddb 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
@@ -508,10 +508,6 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
     }
 
     private void shutdown(boolean instant) {
-        if (disconnectedButResumeable) {
-            return;
-        }
-
         // First shutdown the writer, this will result in a closing stream element getting send to
         // the server
         LOGGER.finer("PacketWriter shutdown()");
@@ -534,13 +530,25 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
         packetReader.shutdown();
         LOGGER.finer("PacketReader has been shut down");
 
-        try {
+        final Socket socket = this.socket;
+        if (socket != null && socket.isConnected()) {
+            try {
                 socket.close();
-        } catch (Exception e) {
+            } catch (Exception e) {
                 LOGGER.log(Level.WARNING, "shutdown", e);
+            }
         }
 
         setWasAuthenticated();
+
+        // Wait for reader and writer threads to be terminated.
+        readerWriterSemaphore.acquireUninterruptibly(2);
+        readerWriterSemaphore.release(2);
+
+        if (disconnectedButResumeable) {
+            return;
+        }
+
         // If we are able to resume the stream, then don't set
         // connected/authenticated/usingTLS to false since we like behave like we are still
         // connected (e.g. sendStanza should not throw a NotConnectedException).
@@ -561,10 +569,6 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
         writer = null;
 
         initState();
-
-        // Wait for reader and writer threads to be terminated.
-        readerWriterSemaphore.acquireUninterruptibly(2);
-        readerWriterSemaphore.release(2);
     }
 
     @Override