From 43eb36e17aed64841571fa0920cc2bb743f69ceb Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Fri, 14 May 2021 22:36:13 -0500 Subject: [PATCH] Simplify and improve graceful disconnect logic --- src/Connection.c | 8 ------ src/ControlStream.c | 24 +++++++---------- src/Limelight-internal.h | 1 - src/Misc.c | 56 +++++++++++++++++++++++----------------- 4 files changed, 42 insertions(+), 47 deletions(-) diff --git a/src/Connection.c b/src/Connection.c index fcdfd41..d4999e3 100644 --- a/src/Connection.c +++ b/src/Connection.c @@ -24,7 +24,6 @@ OPUS_MULTISTREAM_CONFIGURATION HighQualityOpusConfig; int OriginalVideoBitrate; int AudioPacketDuration; bool AudioEncryptionEnabled; -bool UserRequestedTermination; // Connection stages static const char* stageNames[STAGE_MAX] = { @@ -56,12 +55,6 @@ void LiInterruptConnection(void) { // Stop the connection by undoing the step at the current stage and those before it void LiStopConnection(void) { - // If this was a fully complete connection and we haven't started any termination - // logic prior to this point, this termination is user requested. - if (stage == STAGE_MAX - 1 && !alreadyTerminated) { - UserRequestedTermination = true; - } - // Disable termination callbacks now alreadyTerminated = true; @@ -200,7 +193,6 @@ int LiStartConnection(PSERVER_INFORMATION serverInfo, PSTREAM_CONFIGURATION stre alreadyTerminated = false; ConnectionInterrupted = false; - UserRequestedTermination = false; // Validate the audio configuration if (MAGIC_BYTE_FROM_AUDIO_CONFIG(StreamConfig.audioConfiguration) != 0xCA || diff --git a/src/ControlStream.c b/src/ControlStream.c index 5b43d19..c1fe3fd 100644 --- a/src/ControlStream.c +++ b/src/ControlStream.c @@ -834,7 +834,13 @@ static void controlReceiveThreadFunc(void* context) { // GFE 3.20.3.63 we don't get one for 10 seconds after we first get // this termination message. The termination message should be reliable // enough to end the stream now, rather than waiting for an explicit - // disconnect. + // disconnect. The server will also not acknowledge our disconnect + // message once it sends this message, so we mark the peer as fully + // disconnected now to avoid delays waiting for an ack that will + // never arrive. + PltLockMutex(&enetMutex); + enet_peer_disconnect_now(peer, 0); + PltUnlockMutex(&enetMutex); ListenerCallbacks.connectionTerminated((int)terminationErrorCode); free(ctlHdr); return; @@ -1054,19 +1060,9 @@ int stopControlStream(void) { } if (peer != NULL) { - if (UserRequestedTermination) { - // Gracefully disconnect to ensure the remote host receives all of our final - // outbound traffic, including any key up events that might be sent. - gracefullyDisconnectEnetPeer(client, peer, CONTROL_STREAM_LINGER_TIMEOUT_SEC * 1000); - enet_peer_reset(peer); - } - else { - // This termination was requested by the remote host or caused due to a - // connection problem, so just do a quick abortive disconnect. The peer - // may be gone by this point. - enet_peer_disconnect_now(peer, 0); - } - + // Gracefully disconnect to ensure the remote host receives all of our final + // outbound traffic, including any key up events that might be sent. + gracefullyDisconnectEnetPeer(client, peer, CONTROL_STREAM_LINGER_TIMEOUT_SEC * 1000); peer = NULL; } if (client != NULL) { diff --git a/src/Limelight-internal.h b/src/Limelight-internal.h index 7577c9e..14f1116 100644 --- a/src/Limelight-internal.h +++ b/src/Limelight-internal.h @@ -31,7 +31,6 @@ extern OPUS_MULTISTREAM_CONFIGURATION HighQualityOpusConfig; extern int OriginalVideoBitrate; extern int AudioPacketDuration; extern bool AudioEncryptionEnabled; -extern bool UserRequestedTermination; #ifndef UINT24_MAX #define UINT24_MAX 0xFFFFFF diff --git a/src/Misc.c b/src/Misc.c index 55eb826..4c43af0 100644 --- a/src/Misc.c +++ b/src/Misc.c @@ -38,37 +38,45 @@ int serviceEnetHost(ENetHost* client, ENetEvent* event, enet_uint32 timeoutMs) { // This function performs a graceful disconnect, including lingering until outbound // traffic is acked (up until the linger timeout elapses). int gracefullyDisconnectEnetPeer(ENetHost* host, ENetPeer* peer, enet_uint32 lingerTimeoutMs) { - ENetEvent event; - int err; + // Check if this peer is currently alive. We won't get another ENET_EVENT_TYPE_DISCONNECT + // event from ENet if the peer is dead. In that case, we'll do an abortive disconnect. + if (peer->state == ENET_PEER_STATE_CONNECTED) { + ENetEvent event; + int err; - // Begin the disconnection process. If this peer is already a zombie, we'll get an - // immediate disconnect event that will complete the process. If it's still alive, - // we'll get the disconnect event after all outgoing messages have been acked. - enet_peer_disconnect_later(peer, 0); + // Begin the disconnection process. We'll get ENET_EVENT_TYPE_DISCONNECT once + // the peer acks all outstanding reliable sends. + enet_peer_disconnect_later(peer, 0); - // We must use the internal function which lets us ignore pending interrupts. - while ((err = serviceEnetHostInternal(host, &event, lingerTimeoutMs, true)) > 0) { - switch (event.type) { - case ENET_EVENT_TYPE_RECEIVE: - enet_packet_destroy(event.packet); - break; - case ENET_EVENT_TYPE_DISCONNECT: - Limelog("ENet peer disconnection is complete\n"); - return 0; - default: - LC_ASSERT(false); - break; + // We must use the internal function which lets us ignore pending interrupts. + while ((err = serviceEnetHostInternal(host, &event, lingerTimeoutMs, true)) > 0) { + switch (event.type) { + case ENET_EVENT_TYPE_RECEIVE: + enet_packet_destroy(event.packet); + break; + case ENET_EVENT_TYPE_DISCONNECT: + Limelog("ENet peer acknowledged disconnection\n"); + return 0; + default: + LC_ASSERT(false); + break; + } } - } - if (err == 0) { - Limelog("Timed out waiting for ENet peer to acknowledge disconnection\n"); + if (err == 0) { + Limelog("Timed out waiting for ENet peer to acknowledge disconnection\n"); + } + else { + Limelog("Failed to receive ENet peer disconnection acknowledgement\n"); + } + + return -1; } else { - Limelog("Failed to receive ENet peer disconnection acknowledgement\n"); + Limelog("ENet peer is already disconnected\n"); + enet_peer_disconnect_now(peer, 0); + return 0; } - - return -1; } int extractVersionQuadFromString(const char* string, int* quad) {