From 4731c87476ba72503608e63acf6e53c65bfed7d0 Mon Sep 17 00:00:00 2001
From: Brennan <brecon@microsoft.com>
Date: Mon, 23 Nov 2020 11:40:58 -0800
Subject: [PATCH] [Java] Don't call onClose when WebSocket connection is not
 open (#28004)

---
 .../signalr/OkHttpWebSocketWrapper.java       | 62 ++++++++++++++-----
 .../microsoft/signalr/WebSocketTransport.java |  2 -
 2 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/OkHttpWebSocketWrapper.java b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/OkHttpWebSocketWrapper.java
index ae27363720..3825de8c8d 100644
--- a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/OkHttpWebSocketWrapper.java
+++ b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/OkHttpWebSocketWrapper.java
@@ -30,7 +30,7 @@ class OkHttpWebSocketWrapper extends WebSocketWrapper {
     private WebSocketOnClosedCallback onClose;
     private CompletableSubject startSubject = CompletableSubject.create();
     private CompletableSubject closeSubject = CompletableSubject.create();
-    private final ReentrantLock closeLock = new ReentrantLock();
+    private final ReentrantLock stateLock = new ReentrantLock();
 
     private final Logger logger = LoggerFactory.getLogger(OkHttpWebSocketWrapper.class);
 
@@ -82,7 +82,12 @@ class OkHttpWebSocketWrapper extends WebSocketWrapper {
     private class SignalRWebSocketListener extends WebSocketListener {
         @Override
         public void onOpen(WebSocket webSocket, Response response) {
-            startSubject.onComplete();
+            stateLock.lock();
+            try {
+                startSubject.onComplete();
+            } finally {
+                stateLock.unlock();
+            }
         }
 
         @Override
@@ -97,39 +102,64 @@ class OkHttpWebSocketWrapper extends WebSocketWrapper {
 
         @Override
         public void onClosing(WebSocket webSocket, int code, String reason) {
-            onClose.invoke(code, reason);
+            boolean isOpen = false;
+            stateLock.lock();
             try {
-                closeLock.lock();
+                isOpen = startSubject.hasComplete();
+            } finally {
+                stateLock.unlock();
+            }
+
+            logger.info("WebSocket closing with status code '{}' and reason '{}'.", code, reason);
+
+            // Only call onClose if connection is open
+            if (isOpen) {
+                onClose.invoke(code, reason);
+            }
+
+            try {
+                stateLock.lock();
                 closeSubject.onComplete();
             }
             finally {
-                closeLock.unlock();
+                stateLock.unlock();
             }
-            checkStartFailure();
+            checkStartFailure(null);
         }
 
         @Override
         public void onFailure(WebSocket webSocket, Throwable t, Response response) {
-            logger.error("WebSocket closed from an error: {}.", t.getMessage());
+            logger.error("WebSocket closed from an error.", t);
 
+            boolean isOpen = false;
             try {
-                closeLock.lock();
+                stateLock.lock();
                 if (!closeSubject.hasComplete()) {
                     closeSubject.onError(new RuntimeException(t));
                 }
+
+                isOpen = startSubject.hasComplete();
             }
             finally {
-                closeLock.unlock();
+                stateLock.unlock();
             }
-            onClose.invoke(null, t.getMessage());
-            checkStartFailure();
+            // Only call onClose if connection is open
+            if (isOpen) {
+                onClose.invoke(null, t.getMessage());
+            }
+            checkStartFailure(t);
         }
 
-        private void checkStartFailure() {
-            // If the start task hasn't completed yet, then we need to complete it
-            // exceptionally.
-            if (!startSubject.hasComplete()) {
-                startSubject.onError(new RuntimeException("There was an error starting the WebSocket transport."));
+        private void checkStartFailure(Throwable t) {
+            stateLock.lock();
+            try {
+                // If the start task hasn't completed yet, then we need to complete it
+                // exceptionally.
+                if (!startSubject.hasComplete()) {
+                    startSubject.onError(new RuntimeException("There was an error starting the WebSocket transport.", t));
+                }
+            } finally {
+                stateLock.unlock();
             }
         }
     }
diff --git a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/WebSocketTransport.java b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/WebSocketTransport.java
index 42ef00231f..25543221ca 100644
--- a/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/WebSocketTransport.java
+++ b/src/SignalR/clients/java/signalr/core/src/main/java/com/microsoft/signalr/WebSocketTransport.java
@@ -86,8 +86,6 @@ class WebSocketTransport implements Transport {
     }
 
     void onClose(Integer code, String reason) {
-        logger.info("WebSocket connection stopping with " +
-                "code {} and reason '{}'.", code, reason);
         if (code == null || code != 1000) {
             onClose.invoke(reason);
         }