CBL-2410: Don't tear down response timer in timeout situation (#1265)

* CBL-2410: Don't tear down response timer in timeout situation

If onClose is reached with _timedOut being true, then that means not only did the timer already fire and tearing it down is extraneous at this point, but it is almost certainly the case that this method is reached synchronously from the timer callback via timer -> close/requestClose -> platform -> onClose.  This means that it is a hang situation in which the timer waits forever for its callback to finish, and the callback is waiting for the current method to finish.

Co-authored-by: Jim Borden <jim.borden@couchbase.com>

Cherry-picked from 4f8d51a on the branch of hotfix/2.8.7

Networking/WebSockets/WebSocketImpl.cc

@@ -460,7 +460,15 @@ namespace litecore { namespace websocket {
                 return; // Guard against multiple calls to onClose
 
             _pingTimer.reset();
-            _responseTimer.reset();
+
+            if(!_timedOut) {
+                // CBL-2410: If _timedOut is true then we are almost
+                // certainly in this method synchronously from the _responseTimer
+                // callback which means resetting here would cause a hang.  Since
+                // _timedOut is true, this timer has already fired anyway so there
+                // is no pressing need for a tear down here, it can wait until later.
+                _responseTimer.reset();
+            }
 
             if (status.reason == kWebSocketClose) {
                 if (_timedOut)

Replicator/tests/ReplicatorAPITest.cc

@@ -791,5 +791,52 @@ TEST_CASE_METHOD(ReplicatorAPITest, "c4Replicator Zero Memory", "[C][Replicator]
     }
 }
 
-
 #endif
+
+TEST_CASE_METHOD(ReplicatorAPITest, "Connection Timeout stop properly", "[C][Push][Pull][.Slow]") {
+    // CBL-2410
+    C4SocketFactory factory = {};
+    _mayGoOffline = true;
+
+    SECTION("Using framing") {
+        factory.open = [](C4Socket* socket, const C4Address* addr,
+                          C4Slice options, void *context) {
+            // Do nothing, just let things time out....
+        };
+
+        factory.close = [](C4Socket* socket) {
+            // This is a requirement for this test to pass, or the socket will
+            // never actually finish "closing".  Furthermore, this call will hang
+            // before this fix
+            c4socket_closed(socket, {});
+        };
+    }
+
+    SECTION("Not using framing") {
+        factory.framing = kC4NoFraming;
+        factory.open = [](C4Socket* socket, const C4Address* addr,
+                          C4Slice options, void *context) {
+            // Do nothing, just let things time out....
+        };
+
+        factory.requestClose = [](C4Socket* socket, int code, C4Slice message) {
+            // This is a requirement for this test to pass, or the socket will
+            // never actually finish "closing".  Furthermore, this call will hang
+            // before this fix
+            c4socket_closed(socket, {});
+        };
+    }
+
+    _socketFactory = &factory;
+
+    C4Error err;
+    importJSONLines(sFixturesDir + "names_100.json");
+    REQUIRE(startReplicator(kC4Passive, kC4OneShot, &err));
+
+    // Before the fix, offline would never be reached
+    waitForStatus(kC4Offline, 16s);
+    c4repl_stop(_repl);
+    waitForStatus(kC4Stopped, 2s);
+    _socketFactory = nullptr;
+}
+