Prevent concurrent access to AudioSendStream's configuration.
By design:
* OnPacketAdded() is meant to be called on pacer thread.
* Reconfigure() is meant to be called on worker thread.
Thus we guard against race condition on config_ member.
Possible downside: packet filtering based on ssrc might be slowed down.
Bug: webrtc:9849
Change-Id: I734bb9b34b01db160705897adb1b58e866e12639
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/146980
Commit-Queue: Yves Gerey <yvesg@google.com>
Reviewed-by: Oskar Sundbom <ossu@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28691}
diff --git a/audio/audio_send_stream_unittest.cc b/audio/audio_send_stream_unittest.cc
index dcd3667..94bc34c 100644
--- a/audio/audio_send_stream_unittest.cc
+++ b/audio/audio_send_stream_unittest.cc
@@ -11,6 +11,7 @@
#include "audio/audio_send_stream.h"
#include <string>
+#include <thread>
#include <utility>
#include <vector>
@@ -679,6 +680,29 @@
send_stream->Reconfigure(helper.config());
}
+// Allow to check for race conditions under tsan.
+// This mimicks the situation where 'ModuleProcessThread' (pacer thread) is
+// launched by webrtc::RtpTransportControllerSend::RtpTransportControllerSend().
+TEST(AudioSendStreamTest, RaceFree) {
+ ConfigHelper helper(false, false);
+ // Sanity checks: copy-pasted from DontRecreateEncoder test.
+ EXPECT_CALL(*helper.channel_send(), SetEncoderForMock(_, _))
+ .WillOnce(Return());
+
+ EXPECT_CALL(*helper.channel_send(), RegisterCngPayloadType(105, 8000));
+
+ helper.config().send_codec_spec =
+ AudioSendStream::Config::SendCodecSpec(9, kG722Format);
+ helper.config().send_codec_spec->cng_payload_type = 105;
+ auto send_stream = helper.CreateAudioSendStream();
+ std::thread pacer([&]() {
+ send_stream->OnPacketAdded(/*ssrc*/ 0xcafe,
+ /*seq_num*/ 0xf00d);
+ });
+ send_stream->Reconfigure(helper.config());
+ pacer.join();
+}
+
TEST(AudioSendStreamTest, ReconfigureTransportCcResetsFirst) {
ScopedFieldTrials field_trials("WebRTC-Audio-SendSideBwe/Enabled/");
ConfigHelper helper(false, true);