admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove rtc::TaskQueue in AudioDeviceBuffer

Browse Source 
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}
This commit is contained in:
Danil Chapovalov 2024-01-22 10:55:50 +01:00 committed by WebRTC LUCI CQ
parent 3f7566abda
commit 5261619ad2
3 changed files with 22 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
modules/audio_device/BUILD.gn
View File

@ -80,7 +80,6 @@ rtc_library("audio_device_buffer") {
"../../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",

27
modules/audio_device/audio_device_buffer.cc
View File

@ -78,6 +78,17 @@ AudioDeviceBuffer::~AudioDeviceBuffer() {
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 @@ void AudioDeviceBuffer::StartPlayout() {
}
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 @@ void AudioDeviceBuffer::StartRecording() {
}
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 @@ int32_t AudioDeviceBuffer::GetPlayoutData(void* audio_buffer) {
}
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 @@ void AudioDeviceBuffer::LogStats(LogState state) {
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();

5
modules/audio_device/audio_device_buffer.h
View File

@ -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 @@ class AudioDeviceBuffer {
// 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