commit 1921708141e25f8c15a66bf39fc8c5dad05fca3e
author Markus Handell <handellm@webrtc.org> Thu Jan 14 13:00:40 2021
committer Commit Bot <commit-bot@chromium.org> Thu Jan 14 14:29:56 2021
tree 4f16c71f15503506b1cc14705e6e04210a9b65c6
parent c12f6259384d5d7fa313296f5cf7f7998458984a

SetNegotiatedHeaderExtensions_w: Set list synchronously.

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 <hbos@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32977}

pc/channel.cc

diff --git a/pc/channel.cc b/pc/channel.cc
index 661f0e9..69b2ca1 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::SetNegotiatedHeaderExtensions_w(
     const RtpHeaderExtensions& extensions) {
   TRACE_EVENT0("webrtc", __func__);
-  RtpHeaderExtensions extensions_copy = extensions;
-  invoker_.AsyncInvoke<void>(
-      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_;
 }
 

pc/channel.h

diff --git a/pc/channel.h b/pc/channel.h
index 5e2bd7e..bbb95d7 100644
--- a/pc/channel.h
+++ b/pc/channel.h
@@ -381,8 +381,13 @@
   // 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,