Remove rtc::TaskQueue in AudioDeviceBuffer
Instead stop/delete TaskQueueBase in destructor explicitly and explain potential race.
Bug: webrtc:14169
Change-Id: Ica7a78f149be11ba1a82cbf79d4244c918aa9d0a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/335360
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41810}
diff --git a/modules/audio_device/BUILD.gn b/modules/audio_device/BUILD.gn
index 2088e74..e5f8463 100644
--- a/modules/audio_device/BUILD.gn
+++ b/modules/audio_device/BUILD.gn
@@ -80,7 +80,6 @@
"../../rtc_base:event_tracer",
"../../rtc_base:logging",
"../../rtc_base:macromagic",
- "../../rtc_base:rtc_task_queue",
"../../rtc_base:safe_conversions",
"../../rtc_base:timestamp_aligner",
"../../rtc_base:timeutils",
diff --git a/modules/audio_device/audio_device_buffer.cc b/modules/audio_device/audio_device_buffer.cc
index f1bd8e8..38ed633 100644
--- a/modules/audio_device/audio_device_buffer.cc
+++ b/modules/audio_device/audio_device_buffer.cc
@@ -78,6 +78,17 @@
RTC_DCHECK(!playing_);
RTC_DCHECK(!recording_);
RTC_LOG(LS_INFO) << "AudioDeviceBuffer::~dtor";
+
+ // Delete and and thus stop task queue before deleting other members to avoid
+ // race with running tasks. Even though !playing_ and !recording_ called
+ // StopPeriodicLogging, such stop is asynchronous and may race with the
+ // AudioDeviceBuffer destructor. In particular there might be regular LogStats
+ // that attempts to repost task to the task_queue_.
+ // Thus task_queue_ should be deleted before pointer to it is invalidated.
+ // std::unique_ptr destructor does the same two operations in reverse order as
+ // it doesn't expect member would be used after its destruction has started.
+ task_queue_.get_deleter()(task_queue_.get());
+ task_queue_.release();
}
int32_t AudioDeviceBuffer::RegisterAudioCallback(
@@ -102,7 +113,7 @@
}
RTC_DLOG(LS_INFO) << __FUNCTION__;
// Clear members tracking playout stats and do it on the task queue.
- task_queue_.PostTask([this] { ResetPlayStats(); });
+ task_queue_->PostTask([this] { ResetPlayStats(); });
// Start a periodic timer based on task queue if not already done by the
// recording side.
if (!recording_) {
@@ -121,7 +132,7 @@
}
RTC_DLOG(LS_INFO) << __FUNCTION__;
// Clear members tracking recording stats and do it on the task queue.
- task_queue_.PostTask([this] { ResetRecStats(); });
+ task_queue_->PostTask([this] { ResetRecStats(); });
// Start a periodic timer based on task queue if not already done by the
// playout side.
if (!playing_) {
@@ -388,15 +399,15 @@
}
void AudioDeviceBuffer::StartPeriodicLogging() {
- task_queue_.PostTask([this] { LogStats(AudioDeviceBuffer::LOG_START); });
+ task_queue_->PostTask([this] { LogStats(AudioDeviceBuffer::LOG_START); });
}
void AudioDeviceBuffer::StopPeriodicLogging() {
- task_queue_.PostTask([this] { LogStats(AudioDeviceBuffer::LOG_STOP); });
+ task_queue_->PostTask([this] { LogStats(AudioDeviceBuffer::LOG_STOP); });
}
void AudioDeviceBuffer::LogStats(LogState state) {
- RTC_DCHECK_RUN_ON(&task_queue_);
+ RTC_DCHECK_RUN_ON(task_queue_.get());
int64_t now_time = rtc::TimeMillis();
if (state == AudioDeviceBuffer::LOG_START) {
@@ -497,20 +508,20 @@
RTC_DCHECK_GT(time_to_wait_ms, 0) << "Invalid timer interval";
// Keep posting new (delayed) tasks until state is changed to kLogStop.
- task_queue_.PostDelayedTask(
+ task_queue_->PostDelayedTask(
[this] { AudioDeviceBuffer::LogStats(AudioDeviceBuffer::LOG_ACTIVE); },
TimeDelta::Millis(time_to_wait_ms));
}
void AudioDeviceBuffer::ResetRecStats() {
- RTC_DCHECK_RUN_ON(&task_queue_);
+ RTC_DCHECK_RUN_ON(task_queue_.get());
last_stats_.ResetRecStats();
MutexLock lock(&lock_);
stats_.ResetRecStats();
}
void AudioDeviceBuffer::ResetPlayStats() {
- RTC_DCHECK_RUN_ON(&task_queue_);
+ RTC_DCHECK_RUN_ON(task_queue_.get());
last_stats_.ResetPlayStats();
MutexLock lock(&lock_);
stats_.ResetPlayStats();
diff --git a/modules/audio_device/audio_device_buffer.h b/modules/audio_device/audio_device_buffer.h
index 1260a24..b96696e 100644
--- a/modules/audio_device/audio_device_buffer.h
+++ b/modules/audio_device/audio_device_buffer.h
@@ -15,13 +15,14 @@
#include <stdint.h>
#include <atomic>
+#include <memory>
#include "api/sequence_checker.h"
+#include "api/task_queue/task_queue_base.h"
#include "api/task_queue/task_queue_factory.h"
#include "modules/audio_device/include/audio_device_defines.h"
#include "rtc_base/buffer.h"
#include "rtc_base/synchronization/mutex.h"
-#include "rtc_base/task_queue.h"
#include "rtc_base/thread_annotations.h"
#include "rtc_base/timestamp_aligner.h"
@@ -158,7 +159,7 @@
// Task queue used to invoke LogStats() periodically. Tasks are executed on a
// worker thread but it does not necessarily have to be the same thread for
// each task.
- rtc::TaskQueue task_queue_;
+ std::unique_ptr<TaskQueueBase, TaskQueueDeleter> task_queue_;
// Raw pointer to AudioTransport instance. Supplied to RegisterAudioCallback()
// and it must outlive this object. It is not possible to change this member
