Clarify thread/TaskQueue requirements for internal::CallStats
Bug: webrtc:11581
Change-Id: Idec96b14e61d9f9c53dd81fa4325b5ed63da448e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/197424
Reviewed-by: Tommi <tommi@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32835}
diff --git a/video/call_stats2.cc b/video/call_stats2.cc
index faf08d6..fbbe2de 100644
--- a/video/call_stats2.cc
+++ b/video/call_stats2.cc
@@ -76,7 +76,7 @@
time_of_first_rtt_ms_(-1),
task_queue_(task_queue) {
RTC_DCHECK(task_queue_);
- process_thread_checker_.Detach();
+ RTC_DCHECK_RUN_ON(task_queue_);
repeating_task_ =
RepeatingTaskHandle::DelayedStart(task_queue_, kUpdateInterval, [this]() {
UpdateAndReport();
@@ -85,7 +85,7 @@
}
CallStats::~CallStats() {
- RTC_DCHECK_RUN_ON(&construction_thread_checker_);
+ RTC_DCHECK_RUN_ON(task_queue_);
RTC_DCHECK(observers_.empty());
repeating_task_.Stop();
@@ -94,7 +94,7 @@
}
void CallStats::UpdateAndReport() {
- RTC_DCHECK_RUN_ON(&construction_thread_checker_);
+ RTC_DCHECK_RUN_ON(task_queue_);
RemoveOldReports(clock_->CurrentTime().ms(), &reports_);
max_rtt_ms_ = GetMaxRttMs(reports_);
@@ -112,18 +112,18 @@
}
void CallStats::RegisterStatsObserver(CallStatsObserver* observer) {
- RTC_DCHECK_RUN_ON(&construction_thread_checker_);
+ RTC_DCHECK_RUN_ON(task_queue_);
if (!absl::c_linear_search(observers_, observer))
observers_.push_back(observer);
}
void CallStats::DeregisterStatsObserver(CallStatsObserver* observer) {
- RTC_DCHECK_RUN_ON(&construction_thread_checker_);
+ RTC_DCHECK_RUN_ON(task_queue_);
observers_.remove(observer);
}
int64_t CallStats::LastProcessedRtt() const {
- RTC_DCHECK_RUN_ON(&construction_thread_checker_);
+ RTC_DCHECK_RUN_ON(task_queue_);
// No need for locking since we're on the construction thread.
return avg_rtt_ms_;
}
@@ -134,7 +134,7 @@
// on the correct TQ.
int64_t now_ms = clock_->TimeInMilliseconds();
auto update = [this, rtt, now_ms]() {
- RTC_DCHECK_RUN_ON(&construction_thread_checker_);
+ RTC_DCHECK_RUN_ON(task_queue_);
reports_.push_back(RttTime(rtt, now_ms));
if (time_of_first_rtt_ms_ == -1)
time_of_first_rtt_ms_ = now_ms;
@@ -149,7 +149,7 @@
}
void CallStats::UpdateHistograms() {
- RTC_DCHECK_RUN_ON(&construction_thread_checker_);
+ RTC_DCHECK_RUN_ON(task_queue_);
if (time_of_first_rtt_ms_ == -1 || num_avg_rtt_ < 1)
return;
diff --git a/video/call_stats2.h b/video/call_stats2.h
index 30e8263..5932fad 100644
--- a/video/call_stats2.h
+++ b/video/call_stats2.h
@@ -18,8 +18,6 @@
#include "modules/include/module_common_types.h"
#include "modules/rtp_rtcp/include/rtp_rtcp_defines.h"
#include "rtc_base/constructor_magic.h"
-#include "rtc_base/synchronization/sequence_checker.h"
-#include "rtc_base/system/no_unique_address.h"
#include "rtc_base/task_queue.h"
#include "rtc_base/task_utils/pending_task_safety_flag.h"
#include "rtc_base/task_utils/repeating_task.h"
@@ -33,6 +31,7 @@
// Time interval for updating the observers.
static constexpr TimeDelta kUpdateInterval = TimeDelta::Millis(1000);
+ // Must be created and destroyed on the same task_queue.
CallStats(Clock* clock, TaskQueueBase* task_queue);
~CallStats();
@@ -50,11 +49,6 @@
// Expose |LastProcessedRtt()| from RtcpRttStats to the public interface, as
// it is the part of the API that is needed by direct users of CallStats.
- // TODO(tommi): Threading or lifetime guarantees are not explicit in how
- // CallStats is used as RtcpRttStats or how pointers are cached in a
- // few different places (distributed via Call). It would be good to clarify
- // from what thread/TQ calls to OnRttUpdate and LastProcessedRtt need to be
- // allowed.
int64_t LastProcessedRtt() const;
// Exposed for tests to test histogram support.
@@ -106,35 +100,24 @@
Clock* const clock_;
// Used to regularly call UpdateAndReport().
- RepeatingTaskHandle repeating_task_
- RTC_GUARDED_BY(construction_thread_checker_);
+ RepeatingTaskHandle repeating_task_ RTC_GUARDED_BY(task_queue_);
// The last RTT in the statistics update (zero if there is no valid estimate).
- int64_t max_rtt_ms_ RTC_GUARDED_BY(construction_thread_checker_);
+ int64_t max_rtt_ms_ RTC_GUARDED_BY(task_queue_);
// Last reported average RTT value.
- int64_t avg_rtt_ms_ RTC_GUARDED_BY(construction_thread_checker_);
+ int64_t avg_rtt_ms_ RTC_GUARDED_BY(task_queue_);
- // |sum_avg_rtt_ms_|, |num_avg_rtt_| and |time_of_first_rtt_ms_| are only used
- // on the ProcessThread when running. When the Process Thread is not running,
- // (and only then) they can be used in UpdateHistograms(), usually called from
- // the dtor.
- int64_t sum_avg_rtt_ms_ RTC_GUARDED_BY(construction_thread_checker_);
- int64_t num_avg_rtt_ RTC_GUARDED_BY(construction_thread_checker_);
- int64_t time_of_first_rtt_ms_ RTC_GUARDED_BY(construction_thread_checker_);
+ int64_t sum_avg_rtt_ms_ RTC_GUARDED_BY(task_queue_);
+ int64_t num_avg_rtt_ RTC_GUARDED_BY(task_queue_);
+ int64_t time_of_first_rtt_ms_ RTC_GUARDED_BY(task_queue_);
// All Rtt reports within valid time interval, oldest first.
- std::list<RttTime> reports_ RTC_GUARDED_BY(construction_thread_checker_);
+ std::list<RttTime> reports_ RTC_GUARDED_BY(task_queue_);
// Observers getting stats reports.
- // When attached to ProcessThread, this is read-only. In order to allow
- // modification, we detach from the process thread while the observer
- // list is updated, to avoid races. This allows us to not require a lock
- // for the observers_ list, which makes the most common case lock free.
- std::list<CallStatsObserver*> observers_;
+ std::list<CallStatsObserver*> observers_ RTC_GUARDED_BY(task_queue_);
- RTC_NO_UNIQUE_ADDRESS SequenceChecker construction_thread_checker_;
- RTC_NO_UNIQUE_ADDRESS SequenceChecker process_thread_checker_;
TaskQueueBase* const task_queue_;
// Used to signal destruction to potentially pending tasks.
