zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Destroy channel objects synchronously.

Browse Source 
This reverts the async operation introduced here:
https://webrtc-review.googlesource.com/c/src/+/248170

The race that happened was that the "flush" operation in the dtor
of ChannelManager, could run _after_ PeerConnection::Close() which
is where the Call object gets deleted. Inside the dtor of Call, there
are DCHECKs that could hit when the pending deletions hadn't run.
In most cases the Invoke() that is used to delete the Call object
would run after the pending tasks, but there's still one code path
that I'm looking for that could trigger the deletion of a channel
after Call is destructed.

Bug: webrtc:11992, webrtc:13540, chromium:1291383
Change-Id: I160742907cc0c097a4b2bb1b7c3da03b4e8cd8d8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249780
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35822}
This commit is contained in:
Tommi
2022-01-28 08:53:00 +01:00
committed by WebRTC LUCI CQ
parent 5d9ae8635c
commit e8d854eca1
1 changed files with 9 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
pc/channel_manager.cc
View File

@ -61,18 +61,14 @@ ChannelManager::ChannelManager(
ChannelManager::~ChannelManager() {
RTC_DCHECK_RUN_ON(signaling_thread_);
// While `media_engine_` is const throughout the ChannelManager's lifetime,
// it requires destruction to happen on the worker thread. Instead of
// marking the pointer as non-const, we live with this const_cast<> in the
// destructor.
// NOTE: Before removing this Invoke(), consider that it has a dual purpose.
// Besides resetting the media engine pointer, it also ensures that any
// potentially outstanding calls to `DestroyChannel` on the worker, will be
// completed.
worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
RTC_DCHECK_RUN_ON(worker_thread_);
RTC_DCHECK(voice_channels_.empty());
RTC_DCHECK(video_channels_.empty());
// While `media_engine_` is const throughout the ChannelManager's lifetime,
// it requires destruction to happen on the worker thread. Instead of
// marking the pointer as non-const, we live with this const_cast<> in the
// destructor.
const_cast<std::unique_ptr<MediaEngineInterface>&>(media_engine_).reset();
});
}
@ -251,12 +247,11 @@ void ChannelManager::DestroyChannel(ChannelInterface* channel) {
RTC_DCHECK(channel);
if (!worker_thread_->IsCurrent()) {
// Delete the channel asynchronously on the worker thread.
// NOTE: This is made safe by a call to `worker_thread_->Invoke()` from
// the destructor, which acts as a 'flush' for any pending calls to
// DestroyChannel. If that Invoke() gets removed, we'll need to make
// adjustments here.
worker_thread_->PostTask([this, channel] { DestroyChannel(channel); });
// TODO(tommi): Do this asynchronously when we have a way to make sure that
// the call to DestroyChannel runs before ~Call() runs, which today happens
// inside an Invoke from the signaling thread in PeerConnectin::Close().
worker_thread_->Invoke<void>(RTC_FROM_HERE,
[&] { DestroyChannel(channel); });
return;
}