diff --git a/libsignal-service/src/main/java/org/whispersystems/signalservice/internal/websocket/LibSignalChatConnection.kt b/libsignal-service/src/main/java/org/whispersystems/signalservice/internal/websocket/LibSignalChatConnection.kt index 671bd7787e..e47f816806 100644 --- a/libsignal-service/src/main/java/org/whispersystems/signalservice/internal/websocket/LibSignalChatConnection.kt +++ b/libsignal-service/src/main/java/org/whispersystems/signalservice/internal/websocket/LibSignalChatConnection.kt @@ -165,10 +165,15 @@ class LibSignalChatConnection( chatConnectionFuture.whenComplete( onSuccess = { connection -> CHAT_SERVICE_LOCK.withLock { - chatConnection = connection - connection?.start() - Log.i(TAG, "$name Connected") - state.onNext(WebSocketConnectionState.CONNECTED) + if (state.value == WebSocketConnectionState.CONNECTING) { + chatConnection = connection + connection?.start() + Log.i(TAG, "$name Connected") + state.onNext(WebSocketConnectionState.CONNECTED) + } else { + Log.i(TAG, "$name Dropped successful connection because we are now ${state.value}") + disconnect() + } } }, onFailure = { throwable -> @@ -214,6 +219,18 @@ class LibSignalChatConnection( return } + // This avoids a crash when we get a connection lost event during a connection attempt and try + // to cancel a connection that has not yet been fully established. + // TODO [andrew]: Figure out if this is the right long term behavior. + if (state.value == WebSocketConnectionState.CONNECTING) { + // The right way to do this is to cancel the CompletableFuture returned by connectChat() + // Unfortunately, libsignal's CompletableFuture does not yet support cancellation. + // Instead, we set a flag to disconnect() as soon as the connection completes. + // TODO [andrew]: Add cancellation support to CompletableFuture and use it here + state.onNext(WebSocketConnectionState.DISCONNECTING) + return + } + Log.i(TAG, "$name Disconnecting...") state.onNext(WebSocketConnectionState.DISCONNECTING) chatConnection!!.disconnect() @@ -236,6 +253,14 @@ class LibSignalChatConnection( if (isDead()) { return Single.error(IOException("$name is closed!")) } + + // This avoids a crash loop when we try to send queued messages on app open before the connection + // is fully established. + // TODO [andrew]: Figure out if this is the right long term behavior. + if (state.value == WebSocketConnectionState.CONNECTING) { + return Single.error(IOException("$name is still connecting!")) + } + val single = SingleSubject.create() val internalRequest = request.toLibSignalRequest() chatConnection!!.send(internalRequest) diff --git a/libsignal-service/src/test/java/org/whispersystems/signalservice/internal/websocket/LibSignalChatConnectionTest.kt b/libsignal-service/src/test/java/org/whispersystems/signalservice/internal/websocket/LibSignalChatConnectionTest.kt index bbb0303c25..b7df1dc0a0 100644 --- a/libsignal-service/src/test/java/org/whispersystems/signalservice/internal/websocket/LibSignalChatConnectionTest.kt +++ b/libsignal-service/src/test/java/org/whispersystems/signalservice/internal/websocket/LibSignalChatConnectionTest.kt @@ -375,6 +375,32 @@ class LibSignalChatConnectionTest { assertTrue(connection.readRequestIfAvailable().isEmpty) } + @Test + fun regressionTestDisconnectWhileConnecting() { + every { network.connectUnauthChat(any()) } answers { + chatListener = firstArg() + delay { + // We do not complete the future, so we stay in the CONNECTING state forever. + } + } + + connection.connect() + connection.disconnect() + } + + @Test + fun regressionTestSendWhileConnecting() { + every { network.connectUnauthChat(any()) } answers { + chatListener = firstArg() + delay { + // We do not complete the future, so we stay in the CONNECTING state forever. + } + } + + connection.connect() + connection.sendRequest(WebSocketRequestMessage("GET", "/fake-path")) + } + private fun delay(action: ((CompletableFuture) -> Unit)): CompletableFuture { val future = CompletableFuture() executor.submit {