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 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 114 insertions(+), 10 deletions(-) (limited to 'indra/newview/llvoicewebrtc.cpp') 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; } -- cgit v1.3