Merge pull request #2635 from guusdk/OF-2921_Deadlock

OF-2921: Prevent deadlock by not broadcasting synchronously
igniterealtime · Jan 4, 2025 · cdece9b · cdece9b
2 parents 54992b6 + 6428f38
commit cdece9b
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 15 deletions.
diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/SessionManager.java b/xmppserver/src/main/java/org/jivesoftware/openfire/SessionManager.java
@@ -291,7 +291,9 @@ public synchronized void terminateDetached(LocalSession session) {
                     Presence presence = new Presence();
                     presence.setType(Presence.Type.unavailable);
                     presence.setFrom(session.getAddress());
-                    router.route(presence);
+
+                    // Broadcast asynchronously, to reduce the likelihood of the broadcast introducing a deadlock (OF-2921).
+                    TaskEngine.getInstance().submit(() -> router.route(presence));
                 }
 
                 session.getStreamManager().onClose(router, serverAddress);
@@ -1360,7 +1362,9 @@ public void onConnectionClose(Object handback) {
                         Presence presence = new Presence();
                         presence.setType(Presence.Type.unavailable);
                         presence.setFrom(session.getAddress());
-                        router.route(presence);
+
+                        // Broadcast asynchronously, to reduce the likelihood of the broadcast introducing a deadlock (OF-2921).
+                        TaskEngine.getInstance().submit(() -> router.route(presence));
                     }
 
                     session.getStreamManager().onClose(router, serverAddress);

diff --git a/...r/src/main/java/org/jivesoftware/openfire/websocket/WebSocketClientConnectionHandler.java b/...r/src/main/java/org/jivesoftware/openfire/websocket/WebSocketClientConnectionHandler.java
@@ -170,20 +170,18 @@ public void onTextMethod(String stanza)
     public void onError(Throwable error)
     {
         Log.debug("Error detected; connection: {}, session: {}", wsConnection, wsSession, error);
-        synchronized (this) {
-            try {
-                if (isWebSocketOpen()) {
-                    Log.warn("Attempting to close connection on which an error occurred: {}", wsConnection, error);
-                    wsConnection.close(new StreamError(StreamError.Condition.internal_server_error), !isWebSocketOpen());
-                } else {
-                    Log.debug("Error detected on websocket that isn't open (any more):", error);
-                    wsConnection.close(null, !isWebSocketOpen());
-                }
-            } catch (Exception e) {
-                Log.error("Error disconnecting websocket", e);
-            } finally {
-                wsSession = null;
+        try {
+            if (isWebSocketOpen()) {
+                Log.warn("Attempting to close connection on which an error occurred: {}", wsConnection, error);
+                wsConnection.close(new StreamError(StreamError.Condition.internal_server_error), !isWebSocketOpen());
+            } else {
+                Log.debug("Error detected on websocket that isn't open (any more):", error);
+                wsConnection.close(null, !isWebSocketOpen());
             }
+        } catch (Exception e) {
+            Log.error("Error disconnecting websocket", e);
+        } finally {
+            wsSession = null;
         }
     }