Add thread annotations and docs in ProcessThreadImpl.

Bug: webrtc:11567
Change-Id: Ib6b635f658aeecd43cf4ea66e517b7f2caa14022
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/206465
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33312}
This commit is contained in:
Niels Möller
2021-02-22 10:36:29 +01:00
committed by Commit Bot
parent bc9dc5a0b0
commit 2bfddf78d2
2 changed files with 26 additions and 12 deletions

View File

@ -69,7 +69,8 @@ void ProcessThreadImpl::Delete() {
delete this;
}
void ProcessThreadImpl::Start() {
// Doesn't need locking, because the contending thread isn't running.
void ProcessThreadImpl::Start() RTC_NO_THREAD_SAFETY_ANALYSIS {
RTC_DCHECK(thread_checker_.IsCurrent());
RTC_DCHECK(!thread_.get());
if (thread_.get())
@ -91,6 +92,7 @@ void ProcessThreadImpl::Stop() {
return;
{
// Need to take lock, for synchronization with `thread_`.
rtc::CritScope lock(&lock_);
stop_ = true;
}
@ -98,9 +100,17 @@ void ProcessThreadImpl::Stop() {
wake_up_.Set();
thread_->Stop();
thread_.reset();
StopNoLocks();
}
// No locking needed, since this is called after the contending thread is
// stopped.
void ProcessThreadImpl::StopNoLocks() RTC_NO_THREAD_SAFETY_ANALYSIS {
RTC_DCHECK(!thread_);
stop_ = false;
thread_.reset();
for (ModuleCallback& m : modules_)
m.module->ProcessThreadAttached(nullptr);
}

View File

@ -85,25 +85,29 @@ class ProcessThreadImpl : public ProcessThread {
typedef std::list<ModuleCallback> ModuleList;
void Delete() override;
// The part of Stop processing that doesn't need any locking.
void StopNoLocks();
// Warning: For some reason, if |lock_| comes immediately before |modules_|
// with the current class layout, we will start to have mysterious crashes
// on Mac 10.9 debug. I (Tommi) suspect we're hitting some obscure alignemnt
// issues, but I haven't figured out what they are, if there are alignment
// requirements for mutexes on Mac or if there's something else to it.
// So be careful with changing the layout.
rtc::RecursiveCriticalSection
lock_; // Used to guard modules_, tasks_ and stop_.
// Members protected by this mutex are accessed on the constructor thread and
// on the spawned process thread, and locking is needed only while the process
// thread is running.
rtc::RecursiveCriticalSection lock_;
SequenceChecker thread_checker_;
rtc::Event wake_up_;
// TODO(pbos): Remove unique_ptr and stop recreating the thread.
std::unique_ptr<rtc::PlatformThread> thread_;
ModuleList modules_;
ModuleList modules_ RTC_GUARDED_BY(lock_);
std::queue<QueuedTask*> queue_;
std::priority_queue<DelayedTask> delayed_tasks_ RTC_GUARDED_BY(lock_);
bool stop_;
// The `stop_` flag is modified only by the construction thread, protected by
// `thread_checker_`. It is read also by the spawned `thread_`. The latter
// thread must take `lock_` before access, and for thread safety, the
// constructor thread needs to take `lock_` when it modifies `stop_` and
// `thread_` is running. Annotations like RTC_GUARDED_BY doesn't support this
// usage pattern.
bool stop_ RTC_GUARDED_BY(lock_);
const char* thread_name_;
};