From f9ffd68d8eb3b668056664bf67d0e0c540cd200c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Wed, 19 Apr 2023 10:30:04 +0200 Subject: [PATCH] Migrate PostTask+Wait to BlockingCall to avoid deadlock in DegradedCall. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deadlock happens when the WebRtcCombinedNetworkAndWorkerThread experiment is running because the worker thread doing the PostTask is the same thread as the network thread. When using BlockingCall instead this method will avoid the PostTask and just execute in-line instead if the experiment is running and otherwise do what the old path did. As per webrtc:15099, we do not want to increase uses of rtc::Thread in general, and adding more block-invokes in is also discouraged (webrtc:12649) so instead of adding new methods to TaskQueueBase we simply do a static_cast. When WebRtcCombinedNetworkAndWorkerThread has launched the blocking call can be removed because then we're on a single thread always. Bug: webrtc:15098 Change-Id: I6dcc09bcf6ee0ad12e4beffef3b206989265540b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/301880 Reviewed-by: Tomas Gunnarsson Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/main@{#39894} --- call/BUILD.gn | 1 + call/degraded_call.cc | 13 +++++-------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/call/BUILD.gn b/call/BUILD.gn index b4680de860..85192746c1 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -328,6 +328,7 @@ rtc_library("call") { "../rtc_base:rtc_task_queue", "../rtc_base:safe_minmax", "../rtc_base:stringutils", + "../rtc_base:threading", "../rtc_base:timeutils", "../rtc_base/experiments:field_trial_parser", "../rtc_base/network:sent_packet", diff --git a/call/degraded_call.cc b/call/degraded_call.cc index fc76c7be5c..3f47fcded0 100644 --- a/call/degraded_call.cc +++ b/call/degraded_call.cc @@ -16,7 +16,7 @@ #include "absl/strings/string_view.h" #include "api/sequence_checker.h" #include "modules/rtp_rtcp/source/rtp_util.h" -#include "rtc_base/event.h" +#include "rtc_base/thread.h" namespace webrtc { @@ -172,13 +172,10 @@ DegradedCall::~DegradedCall() { // Otherwise, when the `DegradedCall` object is destroyed but // `SetNotAlive` has not yet been called, // another Closure guarded by `call_alive_` may be called. - rtc::Event event; - call_->network_thread()->PostTask( - [flag = std::move(call_alive_), &event]() mutable { - flag->SetNotAlive(); - event.Set(); - }); - event.Wait(rtc::Event::kForever); + // TODO(https://crbug.com/webrtc/12649): Remove this block-invoke. + static_cast(call_->network_thread()) + ->BlockingCall( + [flag = std::move(call_alive_)]() mutable { flag->SetNotAlive(); }); } AudioSendStream* DegradedCall::CreateAudioSendStream(