Bug 1654112 - Destroy Call instances as their associated PeerConnections close. r=ng,bwc

Prior to this patch, WebRtcCallWrapper would never clear the call instance,
meaning that it went away as the classes having references to WebRtcCallWrapper
also went away. These are mainly TransceiverImpl and PeerConnectionMedia. There
are other classes too, but these are the ones that have a strong-ref to
WebRtcCallWrapper *and* are cycle collected.

With the worker threads in libwebrtc now using our own TaskQueues on top of
our SharedThreadPool instances, the ownership model of the Call instances
becomes problematic.

On shutdown, xpcom-shutdown-threads happens before the cycle collector is shut
down.

xpcom-shutdown-threads will however, block shutdown until all SharedThreadPools
have been shut down.

If a TransceiverImpl sits around until shutdown it **will** only be destroyed
during cycle collector shutdown. And, alas, we reach a deadlock as we're stuck
in xpcom-shutdown-threads until that TransceiverImpl gets destroyed.

This patch decouples the Call instance lifetime from the cycle collector, and
avoids the shutdown hang.

Differential Revision: https://phabricator.services.mozilla.com/D103496
This commit is contained in:
Andreas Pehrson 2021-01-28 11:15:40 +01:00
Родитель 9d5176e633
Коммит 9939950d10
4 изменённых файлов: 55 добавлений и 8 удалений

Просмотреть файл

@ -614,6 +614,14 @@ void PeerConnectionMedia::SelfDestruct() {
mQueuedIceCtxOperations.clear();
if (mCall) {
// Clear any resources held by libwebrtc
// mMainThread because that's the Call worker thread for now
mMainThread->Dispatch(NS_NewRunnableFunction(
"PeerConnectionMedia::SelfDestruct(mCall)",
[call = std::move(mCall)]() { call->Destroy(); }));
}
// Shutdown the transport (async)
RUN_ON_THREAD(
mSTSThread,

Просмотреть файл

@ -965,6 +965,10 @@ MediaConduitErrorCode WebrtcAudioConduit::DeliverPacket(const void* data,
auto syncPromise =
InvokeAsync(GetMainThreadSerialEventTarget(), __func__, [&] {
auto current = TaskQueueWrapper::MainAsCurrent();
if (!mCall->Call()) {
return PacketPromise::CreateAndResolve(
webrtc::PacketReceiver::DELIVERY_PACKET_ERROR, __func__);
}
// Bug 1499796 - we need to get passed the time the
// packet was received
webrtc::PacketReceiver::DeliveryStatus status =

Просмотреть файл

@ -281,7 +281,10 @@ class WebRtcCallWrapper : public RefCounted<WebRtcCallWrapper> {
WebRtcCallWrapper(const WebRtcCallWrapper&) = delete;
void operator=(const WebRtcCallWrapper&) = delete;
webrtc::Call* Call() const { return mCall.get(); }
webrtc::Call* Call() const {
MOZ_ASSERT(NS_IsMainThread());
return mCall.get();
}
virtual ~WebRtcCallWrapper() = default;
@ -310,6 +313,27 @@ class WebRtcCallWrapper : public RefCounted<WebRtcCallWrapper> {
DOMHighResTimeStamp GetNow() const { return mTimestampMaker.GetNow(); }
// Allow destroying the Call instance on the Call worker thread.
//
// This CallWrapper is designed to be sharable, and is held by several objects
// that are cycle-collectable. TaskQueueWrapper that the Call instances use
// for worker threads are based off SharedThreadPools, and will block
// xpcom-shutdown-threads until destroyed. The Call instance however will hold
// on to its worker threads until destroyed itself.
//
// If the last ref to this CallWrapper is held to cycle collector shutdown we
// end up in a deadlock where cycle collector shutdown is required to destroy
// the SharedThreadPool that is blocking xpcom-shutdown-threads from finishing
// and triggering cycle collector shutdown.
//
// It would be nice to have the invariant where this class is immutable to the
// degree that mCall is const, but given the above that is not possible.
void Destroy() {
MOZ_ASSERT(NS_IsMainThread());
auto current = TaskQueueWrapper::MainAsCurrent();
mCall = nullptr;
}
const dom::RTCStatsTimestampMaker& GetTimestampMaker() const {
return mTimestampMaker;
}
@ -342,7 +366,10 @@ class WebRtcCallWrapper : public RefCounted<WebRtcCallWrapper> {
const RefPtr<webrtc::AudioDecoderFactory> mAudioDecoderFactory;
const UniquePtr<webrtc::RtcEventLog> mEventLog;
const UniquePtr<webrtc::TaskQueueFactory> mTaskQueueFactory;
const UniquePtr<webrtc::Call> mCall;
private:
// Main thread only, as it's the Call worker thread.
UniquePtr<webrtc::Call> mCall;
};
// Abstract base classes for external encoder/decoder.

Просмотреть файл

@ -263,11 +263,11 @@ class TestAgent {
webrtc::AudioDecoderFactory* aAudioDecoderFactory,
webrtc::WebRtcKeyValueConfig* aTrials)
: audio_config_(109, "opus", 48000, 2, false),
call_(WebRtcCallWrapper::Create(dom::RTCStatsTimestampMaker(),
aModuleThread, aAudioStateConfig,
aAudioDecoderFactory, aTrials)),
audio_conduit_(mozilla::AudioSessionConduit::Create(
WebRtcCallWrapper::Create(dom::RTCStatsTimestampMaker(),
aModuleThread, aAudioStateConfig,
aAudioDecoderFactory, aTrials),
test_utils->sts_target())),
call_, test_utils->sts_target())),
audio_pipeline_(),
transport_(new LoopbackTransport) {}
@ -301,8 +301,15 @@ class TestAgent {
void Shutdown_s() { transport_->Shutdown(); }
void Shutdown() {
if (audio_pipeline_) audio_pipeline_->Shutdown_m();
if (audio_conduit_) audio_conduit_->DeleteStreams();
if (audio_pipeline_) {
audio_pipeline_->Shutdown_m();
}
if (audio_conduit_) {
audio_conduit_->DeleteStreams();
}
if (call_) {
call_->Destroy();
}
test_utils->sts_target()->Dispatch(
WrapRunnable(this, &TestAgent::Shutdown_s),
@ -335,6 +342,7 @@ class TestAgent {
protected:
mozilla::AudioCodecConfig audio_config_;
RefPtr<WebRtcCallWrapper> call_;
RefPtr<mozilla::MediaSessionConduit> audio_conduit_;
RefPtr<FakeAudioTrack> audio_track_;
// TODO(bcampen@mozilla.com): Right now this does not let us test RTCP in