From d4efc4f2aef0065f4238cc9b224eae807588ab62 Mon Sep 17 00:00:00 2001 From: Roxie Linden Date: Wed, 12 Aug 2026 14:29:29 -0700 Subject: p#682 Fix shutdown crash and cut WebRTC teardown time terminate() released the thread unique_ptrs on the timeout path while the detached shutdown thread was still running the lambda that reads them, so a slow shutdown became a null deref at the next BlockingCall. Leave the members alone and report the timeout instead, so the caller leaks this object rather than deleting it out from under the detached thread. Reduce how often that timeout is reached at all: - Post the audio device stop instead of blocking on it. StopRecording and StopPlayout each wait up to 2s on a WASAPI thread, spending much of the 10s budget before the connections were touched. The device stop has to stay on the worker thread (AudioDeviceBuffer is guarded by a sequence checker bound to it), but the queue is FIFO, so ForceTerminate still runs after it while the signaling close gets on with its network-thread work. - Drain the peer connections in LLWebRTCVoiceClient::terminate() before handing off to llwebrtc::terminate(), bounded to 3s. An estate session can hold ten live connections -- the current region plus up to eight neighbours -- and closing them inline and serially under the shutdown budget is what made it tight. Draining also quiets the stats poll, so PeerConnection::Close() no longer blocks in WaitForPendingRequest() on an in-flight GetStats. Connections reaped by the drain are destroyed while webrtc is still fully alive, so they must unregister and release their peer connections normally; key the destructor's early-out off a new sWebRTCTerminated instead of isShuttingDown(). Notify signaling observers even when there was no peer connection to close, or a connection shut down before it finished initializing parks in VOICE_STATE_WAIT_FOR_CLOSE, which has no timeout of its own. Co-Authored-By: Claude Opus 5 (1M context) --- indra/newview/llvoicewebrtc.cpp | 124 ++++++++++++++++++++++++++++++++++++---- indra/newview/llvoicewebrtc.h | 20 +++++++ 2 files changed, 134 insertions(+), 10 deletions(-) (limited to 'indra/newview') diff --git a/indra/newview/llvoicewebrtc.cpp b/indra/newview/llvoicewebrtc.cpp index 126d22924b..476ab18338 100644 --- a/indra/newview/llvoicewebrtc.cpp +++ b/indra/newview/llvoicewebrtc.cpp @@ -208,6 +208,7 @@ LLSD LLVoiceWebRTCStats::read() /////////////////////////////////////////////////////////////////////////////////////////////// bool LLWebRTCVoiceClient::sShuttingDown = false; +bool LLWebRTCVoiceClient::sWebRTCTerminated = false; LLWebRTCVoiceClient::LLWebRTCVoiceClient() : mHidden(false), @@ -233,6 +234,7 @@ LLWebRTCVoiceClient::LLWebRTCVoiceClient() : mWebRTCDeviceInterface(nullptr) { sShuttingDown = false; + sWebRTCTerminated = false; mSpeakerVolume = 0.0; @@ -301,11 +303,81 @@ void LLWebRTCVoiceClient::terminate() mVoiceEnabled = false; sShuttingDown = true; // so that coroutines won't post more work. + + drainConnections(); + + sWebRTCTerminated = true; llwebrtc::terminate(); mWebRTCDeviceInterface = nullptr; } +// Close the live peer connections before handing control to +// llwebrtc::terminate(). +// +// Anything still open when terminate() runs gets closed inline and serially on +// the signaling thread, under a single 10s budget that is also paying for the +// audio device shutdown. An estate session can hold ten live connections -- +// the current region plus up to eight neighbours, plus any group or ad-hoc +// session -- each needing a full DTLS/SCTP teardown, so that budget is not +// generous. Closing them here lets the teardown proceed asynchronously on the +// signaling thread while this thread keeps pumping. +// +// It also quiets the per-connection stats poll before terminate() runs. A +// GetStats request left in flight makes PeerConnection::Close() block in +// RTCStatsCollector::WaitForPendingRequest(), which waits on the network thread +// with no timeout at all. +// +// Best effort: whatever hasn't closed by the deadline is left to +// llwebrtc::terminate(), exactly as before. +void LLWebRTCVoiceClient::drainConnections() +{ + // Marks every session and connection as shutting down. This also stops + // estateSessionState::processConnectionStates() from spinning up + // replacement connections to neighbouring regions while we drain. + sessionState::for_each(boost::bind(predShutdownSession, _1)); + + // Long enough for a local close to complete, short enough that a wedged + // connection doesn't noticeably delay quitting. The remaining budget in + // llwebrtc::terminate() is the real backstop. + constexpr F32 DRAIN_TIMEOUT_SECONDS = 3.0f; + constexpr U32 DRAIN_POLL_MS = 10; + + // Wait on the peer connections being closed rather than on the sessions + // being reaped. A connection that still has an HTTP coroutine in flight + // holds its session alive until mOutstandingRequests unwinds, and those + // coroutines don't run from here -- but its peer connection has already + // been closed by then, which is all terminate() cares about. + LLTimer timer; + while (!sessionState::allSessionsClosed() && timer.getElapsedTimeF32() < DRAIN_TIMEOUT_SECONDS) + { + // OnPeerConnectionClosed comes back through the main queue, so it has + // to be pumped or the state machines never see connections finish. + if (auto main_queue = mMainQueue.lock()) + { + main_queue->runFor(std::chrono::milliseconds(DRAIN_POLL_MS)); + } + sessionState::processSessionStates(); + + if (!sessionState::allSessionsClosed()) + { + ms_sleep(DRAIN_POLL_MS); + } + } + + if (!sessionState::allSessionsClosed()) + { + LL_WARNS("Voice") << "Timed out draining voice connections after " + << DRAIN_TIMEOUT_SECONDS + << "s; leaving the rest to llwebrtc::terminate()." << LL_ENDL; + } + else + { + LL_INFOS("Voice") << "Voice connections drained in " + << timer.getElapsedTimeF32() << "s." << LL_ENDL; + } +} + //--------------------------------------------------- void LLWebRTCVoiceClient::cleanUp() @@ -1890,6 +1962,7 @@ void LLWebRTCVoiceClient::userAuthorized(const std::string& user_id, const LLUUI if (sShuttingDown) { sShuttingDown = false; // was terminated, restart + sWebRTCTerminated = false; initWebRTC(); } } @@ -2173,6 +2246,30 @@ void LLWebRTCVoiceClient::sessionState::processSessionStates() } } +bool LLWebRTCVoiceClient::sessionState::allConnectionsClosed() const +{ + for (const auto &connection : mWebRTCConnections) + { + if (!connection->isClosed()) + { + return false; + } + } + return true; +} + +bool LLWebRTCVoiceClient::sessionState::allSessionsClosed() +{ + for (const auto &session : sSessions) + { + if (session.second && !session.second->allConnectionsClosed()) + { + return false; + } + } + return true; +} + // process the states on each connection associated with a session. bool LLWebRTCVoiceClient::sessionState::processConnectionStates() { @@ -2425,12 +2522,18 @@ LLVoiceWebRTCConnection::LLVoiceWebRTCConnection(const LLUUID ®ionID, const s LLVoiceWebRTCConnection::~LLVoiceWebRTCConnection() { - if (LLWebRTCVoiceClient::isShuttingDown()) + if (LLWebRTCVoiceClient::isWebRTCTerminated()) { - // peer connection and observers will be cleaned up - // by llwebrtc::terminate() on shutdown. + // peer connection and observers have already been cleaned up + // by llwebrtc::terminate(). return; } + // Note this is deliberately keyed off isWebRTCTerminated() rather than + // isShuttingDown(): connections drained by drainConnections() are destroyed + // while the webrtc library is still fully alive, and must unregister + // themselves and release the peer connection like any other close. Leaving + // a freed observer registered would hand llwebrtc::terminate() a dangling + // pointer to call OnPeerConnectionClosed() on. mWebRTCPeerConnectionInterface->unsetSignalingObserver(this); llwebrtc::freePeerConnection(mWebRTCPeerConnectionInterface); } @@ -3092,8 +3195,10 @@ bool LLVoiceWebRTCConnection::connectionStateMachine() } else { - // llwebrtc::terminate() is already shuting down the connection. - setVoiceConnectionState(VOICE_STATE_WAIT_FOR_CLOSE); + // Shutting down: skip the courtesy logout to the sim (the HTTP + // round trip would just delay quitting) and go straight to + // dropping the webrtc connection. + setVoiceConnectionState(VOICE_STATE_SESSION_EXIT); } break; @@ -3104,11 +3209,10 @@ bool LLVoiceWebRTCConnection::connectionStateMachine() { setVoiceConnectionState(VOICE_STATE_WAIT_FOR_CLOSE); mOutstandingRequests++; - if (!LLWebRTCVoiceClient::isShuttingDown()) - { - mWebRTCPeerConnectionInterface->shutdownConnection(); - } - // else was already posted by llwebrtc::terminate(). + // Always drop the connection ourselves, including during shutdown: + // drainConnections() runs before llwebrtc::terminate(), so nothing + // else has posted the close yet. + mWebRTCPeerConnectionInterface->shutdownConnection(); break; } diff --git a/indra/newview/llvoicewebrtc.h b/indra/newview/llvoicewebrtc.h index 818bce5bc3..ec01b8c08c 100644 --- a/indra/newview/llvoicewebrtc.h +++ b/indra/newview/llvoicewebrtc.h @@ -79,6 +79,12 @@ public: static bool isShuttingDown() { return sShuttingDown; } + // True once llwebrtc::terminate() has been entered. Between + // isShuttingDown() and this, the webrtc library is still fully alive and + // connections must still release their peer connections normally -- see + // drainConnections() and ~LLVoiceWebRTCConnection(). + static bool isWebRTCTerminated() { return sWebRTCTerminated; } + const LLVoiceVersionInfo& getVersion() override; void updateVersion(); @@ -306,6 +312,9 @@ public: bool isEmpty() { return mWebRTCConnections.empty(); } + bool allConnectionsClosed() const; + static bool allSessionsClosed(); + virtual bool isSpatial() = 0; virtual bool isEstate() = 0; virtual bool isCallbackPossible() = 0; @@ -455,6 +464,10 @@ private: /// Clean up objects created during a voice session. void cleanUp(); + /// Close the live peer connections before handing off to + /// llwebrtc::terminate(). Bounded and best effort. + void drainConnections(); + LL::WorkQueue::weak_t mMainQueue; F32 mTuningMicGain; @@ -539,6 +552,7 @@ private: // These variables can last longer than WebRTC in coroutines so we need them as static static bool sShuttingDown; + static bool sWebRTCTerminated; LLEventMailDrop mWebRTCPump; @@ -643,6 +657,12 @@ class LLVoiceWebRTCConnection : return mShutDown; } + // True once the webrtc peer connection has finished closing. The + // connection object can outlive this while it waits for outstanding + // requests to unwind, so this -- not reaping -- is what drainConnections() + // waits on. + bool isClosed() const { return mVoiceConnectionState == VOICE_STATE_CLOSED; } + void OnVoiceConnectionRequestSuccess(const LLSD &body); void resetConnectionStats(); -- cgit v1.3