diff options
Diffstat (limited to 'indra/llwebrtc/llwebrtc.cpp')
| -rw-r--r-- | indra/llwebrtc/llwebrtc.cpp | 154 |
1 files changed, 108 insertions, 46 deletions
diff --git a/indra/llwebrtc/llwebrtc.cpp b/indra/llwebrtc/llwebrtc.cpp index 8b77a39541..ce5f487b32 100644 --- a/indra/llwebrtc/llwebrtc.cpp +++ b/indra/llwebrtc/llwebrtc.cpp @@ -412,7 +412,7 @@ void LLWebRTCImpl::init() } -void LLWebRTCImpl::terminate() +bool LLWebRTCImpl::terminate() { // Run all blocking WebRTC shutdown calls on a separate thread so that a // hung BlockingCall cannot block the viewer shutdown indefinitely. @@ -426,16 +426,57 @@ void LLWebRTCImpl::terminate() std::vector<webrtc::scoped_refptr<LLWebRTCPeerConnectionImpl>> connections; connections.swap(mPeerConnections); + // Explicitely unregister observers before shutting down the threads. + // shutdown_thread, if detached, can outlive observer. + mWorkerThread->BlockingCall([this]() + { +#if !CM_WEBRTC + if (mDeviceModule) + { + mDeviceModule->SetObserver(nullptr); + } +#endif + }); + mVoiceDevicesObserverList.clear(); + + // shutdown_thread can be detached, then LLWebRTCImpl will be nulled out. + // Capture what's needed in lambda, don't rely on [this]. std::thread shutdown_thread( - [this, connections = std::move(connections), done_promise]() mutable + [networkThread = std::move(mNetworkThread), + workerThread = std::move(mWorkerThread), + signalingThread = std::move(mSignalingThread), + deviceModule = std::move(mDeviceModule), + factory = std::move(mPeerConnectionFactory), + connections = std::move(connections), + done_promise]() mutable { - mWorkerThread->BlockingCall( - [this]() + // Stop the capture/render devices alongside the connection teardown + // below rather than ahead of it. Both of these calls end in a + // WaitForSingleObject on a WASAPI thread with a 2s timeout apiece + // (AudioDeviceWindowsCore::StopRecording / StopPlayout), so blocking on + // them here can spend most of the shutdown budget before the + // connections have been touched at all -- and after an OS sleep they + // tend to hit the full timeout. + // + // This work has to stay on the worker thread: the device module was + // created there and its AudioDeviceBuffer is guarded by a sequence + // checker bound to that thread. Posting instead of blocking lets the + // signaling close below get on with its network-thread work (data + // channel close, transport teardown) while the device stop is still + // waiting on WASAPI. + // + // No explicit join is needed: Thread's task queue is FIFO and + // BlockingCall posts through it, so the ForceTerminate call at the end + // of this lambda can't run until this task has finished. Any + // worker-thread work the signaling close does is likewise ordered + // after it, so nothing sees the device module half torn down. + workerThread->PostTask( + [&deviceModule]() { - if (mDeviceModule) + if (deviceModule) { - mDeviceModule->ForceStopRecording(); - mDeviceModule->StopPlayout(); + deviceModule->ForceStopRecording(); + deviceModule->StopPlayout(); } }); @@ -450,7 +491,7 @@ void LLWebRTCImpl::terminate() // callback inline, and that callback calls back into the viewer's // signaling observers. Those observers are only valid until // llwebrtc::terminate() returns. - mSignalingThread->BlockingCall( + signalingThread->BlockingCall( [&connections]() { for (auto& connection : connections) @@ -463,22 +504,29 @@ void LLWebRTCImpl::terminate() }); // Drain anything the closes posted before dropping the factory. - mSignalingThread->BlockingCall([]() {}); + signalingThread->BlockingCall([]() {}); - mSignalingThread->BlockingCall([this]() { - mPeerConnectionFactory = nullptr; + signalingThread->BlockingCall([&factory]() { + factory = nullptr; }); - mWorkerThread->BlockingCall( - [this]() + workerThread->BlockingCall( + [&deviceModule]() { - if (mDeviceModule) + if (deviceModule) { - mDeviceModule->ForceTerminate(); + deviceModule->ForceTerminate(); } - mDeviceModule = nullptr; + deviceModule = nullptr; }); + // Explicitly clean WebRTC threads in dependency order before signalling completion. + // The connections were closed and destroyed on the signaling thread, so it's safe + // to clean. + signalingThread.reset(); + workerThread.reset(); + networkThread.reset(); + done_promise->set_value(); }); @@ -490,29 +538,29 @@ void LLWebRTCImpl::terminate() " Detaching — some WebRTC resources will be leaked."; shutdown_thread.detach(); - // Release the unique_ptrs WITHOUT joining/deleting: the detached thread - // may still be using these thread objects. - // The raw pointers are intentionally leaked — the process is exiting anyway - // and our priority is saving cache and personal data. - (void)mNetworkThread.release(); - (void)mWorkerThread.release(); - (void)mSignalingThread.release(); - + // Leave every member exactly as it is. The detached thread is still + // running the lambda above, which reads mSignalingThread, mWorkerThread, + // mDeviceModule and mPeerConnectionFactory through `this` -- clearing or + // releasing them here would pull them out from under it mid-shutdown + // (a null mSignalingThread is an immediate segfault at the next + // BlockingCall). Instead we report the failure so the caller leaks this + // object rather than deleting it; the process is exiting anyway and our + // priority is saving cache and personal data. + // // mPeerConnections is already empty -- the detached thread owns the // connections now and must be left to finish with them. + // + // The log sink is unhooked here (and deliberately not deleted, since the + // detached thread may still log) because the viewer-side log callback + // behind it doesn't outlive this call. webrtc::LogMessage::RemoveLogToStream(mLogSink); - return; + return false; } shutdown_thread.join(); - // The connections were closed and destroyed on the signaling thread before - // the shutdown thread finished, so it's safe to drop the threads now. - mNetworkThread = nullptr; - mWorkerThread = nullptr; - mSignalingThread = nullptr; - webrtc::LogMessage::RemoveLogToStream(mLogSink); + return true; } @@ -862,7 +910,9 @@ void LLWebRTCImpl::updateDevices() void LLWebRTCImpl::OnDevicesUpdated() { - updateDevices(); + // OnDevicesUpdated() is called on macOS CoreAudio's device-change callback + // thread. Calling updateDevices() on that thread causes a deadlock. + mWorkerThread->PostTask([this] { updateDevices(); }); } @@ -1140,17 +1190,23 @@ void LLWebRTCPeerConnectionImpl::closeOnSignalingThread() mLocalStream = nullptr; } mPeerConnection = nullptr; + } - for (auto &observer : mSignalingObserverList) - { - observer->OnPeerConnectionClosed(); - } + // Notify unconditionally, even if there was no peer connection to close -- + // a connection can be shut down before it ever finished initializing, and + // the caller is still waiting to hear that the close is done. Withholding + // this leaves the viewer's connection state machine parked in + // VOICE_STATE_WAIT_FOR_CLOSE, which has no timeout of its own. + for (auto &observer : mSignalingObserverList) + { + observer->OnPeerConnectionClosed(); } - // Nothing may call back into the viewer past this point. On shutdown the - // viewer's connection objects are torn down as soon as llwebrtc::terminate() - // returns and they deliberately don't unset themselves as observers, so any - // late callback would be reaching into freed memory. + // Nothing may call back into the viewer past this point. Connections + // closed while the viewer is still running unset themselves as observers + // when they're destroyed, but any that are left for llwebrtc::terminate() + // to close deliberately don't -- they're torn down as soon as it returns, + // so a late callback would be reaching into freed memory. mSignalingObserverList.clear(); mDataObserverList.clear(); } @@ -1775,20 +1831,20 @@ void LLWebRTCPeerConnectionImpl::OnStateChange() switch (mDataChannel->state()) { case webrtc::DataChannelInterface::kOpen: - RTC_LOG(LS_INFO) << __FUNCTION__ << " Data Channel State Open"; + RTC_LOG(LS_VERBOSE) << __FUNCTION__ << " Data Channel State Open"; for (auto &observer : mSignalingObserverList) { observer->OnDataChannelReady(this); } break; case webrtc::DataChannelInterface::kConnecting: - RTC_LOG(LS_INFO) << __FUNCTION__ << " Data Channel State Connecting"; + RTC_LOG(LS_VERBOSE) << __FUNCTION__ << " Data Channel State Connecting"; break; case webrtc::DataChannelInterface::kClosing: - RTC_LOG(LS_INFO) << __FUNCTION__ << " Data Channel State closing"; + RTC_LOG(LS_VERBOSE) << __FUNCTION__ << " Data Channel State closing"; break; case webrtc::DataChannelInterface::kClosed: - RTC_LOG(LS_INFO) << __FUNCTION__ << " Data Channel State closed"; + RTC_LOG(LS_VERBOSE) << __FUNCTION__ << " Data Channel State closed"; break; default: break; @@ -1947,8 +2003,14 @@ void terminate() { if (gWebRTCImpl) { - gWebRTCImpl->terminate(); - delete gWebRTCImpl; + if (gWebRTCImpl->terminate()) + { + delete gWebRTCImpl; + } + // Otherwise shutdown timed out and was left to a detached thread that is + // still using this object -- and the webrtc threads it owns -- so it's + // intentionally leaked. Deleting it would hand that thread a freed + // object to finish shutting down with. gWebRTCImpl = nullptr; } } |