From 05a9e5abd3a36e3e2e54bb854455d8f687f69f1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Fri, 13 Aug 2021 14:00:44 +0200 Subject: [PATCH] Fix race in CallPerfTest.Bitrate_Kbps_PadsToMinTransmitBitrate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task posted by OnSendRtp might be scheduled after `send_stream_` is destroyed. Fix by using a PendingTaskSafetyFlag, killed from the OnStreamsStopped callback. Bug: webrtc:12726 Change-Id: I935917a3d80e82c3536261d72059448fb7aac00d Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228643 Reviewed-by: Erik Språng Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#34754} --- call/BUILD.gn | 2 +- call/call_perf_tests.cc | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/call/BUILD.gn b/call/BUILD.gn index d542f4b5b4..fc8fd34022 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -535,9 +535,9 @@ if (rtc_include_tests) { "../rtc_base:checks", "../rtc_base:rtc_base_approved", "../rtc_base:task_queue_for_test", - "../rtc_base:task_queue_for_test", "../rtc_base:threading", "../rtc_base/synchronization:mutex", + "../rtc_base/task_utils:pending_task_safety_flag", "../rtc_base/task_utils:repeating_task", "../system_wrappers", "../system_wrappers:metrics", diff --git a/call/call_perf_tests.cc b/call/call_perf_tests.cc index 71bd490851..9596323859 100644 --- a/call/call_perf_tests.cc +++ b/call/call_perf_tests.cc @@ -31,6 +31,7 @@ #include "rtc_base/checks.h" #include "rtc_base/synchronization/mutex.h" #include "rtc_base/task_queue_for_test.h" +#include "rtc_base/task_utils/pending_task_safety_flag.h" #include "rtc_base/thread.h" #include "rtc_base/thread_annotations.h" #include "system_wrappers/include/metrics.h" @@ -668,12 +669,13 @@ void CallPerfTest::TestMinTransmitBitrate(bool pad_to_min_bitrate) { : (kMaxEncodeBitrateKbps + kAcceptableBitrateErrorMargin / 2)), num_bitrate_observations_in_range_(0), - task_queue_(task_queue) {} + task_queue_(task_queue), + task_safety_flag_(PendingTaskSafetyFlag::CreateDetached()) {} private: // TODO(holmer): Run this with a timer instead of once per packet. Action OnSendRtp(const uint8_t* packet, size_t length) override { - task_queue_->PostTask(ToQueuedTask([this]() { + task_queue_->PostTask(ToQueuedTask(task_safety_flag_, [this]() { VideoSendStream::Stats stats = send_stream_->GetStats(); if (!stats.substreams.empty()) { @@ -701,6 +703,8 @@ void CallPerfTest::TestMinTransmitBitrate(bool pad_to_min_bitrate) { send_stream_ = send_stream; } + void OnStreamsStopped() override { task_safety_flag_->SetNotAlive(); } + void ModifyVideoConfigs( VideoSendStream::Config* send_config, std::vector* receive_configs, @@ -729,6 +733,7 @@ void CallPerfTest::TestMinTransmitBitrate(bool pad_to_min_bitrate) { int num_bitrate_observations_in_range_; std::vector bitrate_kbps_list_; TaskQueueBase* task_queue_; + rtc::scoped_refptr task_safety_flag_; } test(pad_to_min_bitrate, task_queue()); fake_encoder_max_bitrate_ = kMaxEncodeBitrateKbps;