From 1921708141e25f8c15a66bf39fc8c5dad05fca3e Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Thu, 14 Jan 2021 14:00:40 +0100 Subject: [PATCH] SetNegotiatedHeaderExtensions_w: Set list synchronously. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SetNegotiatedHeaderExtensions_w queued a task to update the list of negotiated header extensions on the signal thread from the worker thread, in belief that a later call to GetNegotiatedHeaderExtensions() would happen on the WebRTC proxies, leading to the update happening before the readout. In downstream project, this is not always the case. Fix this by synchronously updating the list of negotiated header extensions. Bug: chromium:1051821 Change-Id: I3266292e7508bb7a22a3f7d871e82c12f60cfc83 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/201728 Reviewed-by: Henrik Boström Commit-Queue: Markus Handell Cr-Commit-Position: refs/heads/master@{#32977} --- pc/channel.cc | 12 +++++------- pc/channel.h | 7 ++++++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pc/channel.cc b/pc/channel.cc index 661f0e908c..69b2ca1676 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -30,6 +30,7 @@ #include "rtc_base/logging.h" #include "rtc_base/network_route.h" #include "rtc_base/strings/string_builder.h" +#include "rtc_base/synchronization/mutex.h" #include "rtc_base/synchronization/sequence_checker.h" #include "rtc_base/trace_event.h" @@ -839,17 +840,14 @@ void BaseChannel::SignalSentPacket_n(const rtc::SentPacket& sent_packet) { void BaseChannel::SetNegotiatedHeaderExtensions_w( const RtpHeaderExtensions& extensions) { TRACE_EVENT0("webrtc", __func__); - RtpHeaderExtensions extensions_copy = extensions; - invoker_.AsyncInvoke( - RTC_FROM_HERE, signaling_thread(), - [this, extensions_copy = std::move(extensions_copy)] { - RTC_DCHECK_RUN_ON(signaling_thread()); - negotiated_header_extensions_ = std::move(extensions_copy); - }); + RTC_DCHECK_RUN_ON(worker_thread()); + webrtc::MutexLock lock(&negotiated_header_extensions_lock_); + negotiated_header_extensions_ = extensions; } RtpHeaderExtensions BaseChannel::GetNegotiatedRtpHeaderExtensions() const { RTC_DCHECK_RUN_ON(signaling_thread()); + webrtc::MutexLock lock(&negotiated_header_extensions_lock_); return negotiated_header_extensions_; } diff --git a/pc/channel.h b/pc/channel.h index 5e2bd7e651..bbb95d7ea1 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -381,8 +381,13 @@ class BaseChannel : public ChannelInterface, // This object is not owned by the channel so it must outlive it. rtc::UniqueRandomIdGenerator* const ssrc_generator_; + // |negotiated_header_extensions_| is read on the signaling thread, but + // written on the worker thread while being sync-invoked from the signal + // thread in SdpOfferAnswerHandler::PushdownMediaDescription(). Hence the lock + // isn't strictly needed, but it's anyway placed here for future safeness. + mutable webrtc::Mutex negotiated_header_extensions_lock_; RtpHeaderExtensions negotiated_header_extensions_ - RTC_GUARDED_BY(signaling_thread()); + RTC_GUARDED_BY(negotiated_header_extensions_lock_); }; // VoiceChannel is a specialization that adds support for early media, DTMF,