Revert "Migrate test/time_controller to webrtc::Mutex." This reverts commit 52fd96fb731d9a911bd064a9718d96fef0bd5b24. Reason for revert: previously undetected lock recursions happening in downstream project. Original change's description: > Migrate test/time_controller to webrtc::Mutex. > > Bug: webrtc:11567 > Change-Id: I26fb07bf84ed197ce667290aa0bf4816bc9c5c06 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178818 > Reviewed-by: Sebastian Jansson <srte@webrtc.org> > Commit-Queue: Markus Handell <handellm@webrtc.org> > Cr-Commit-Position: refs/heads/master@{#31660} TBR=srte@webrtc.org,handellm@webrtc.org Change-Id: Icccfa32ac21412bc46f75ac7aca76641f5593096 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:11567 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178872 Reviewed-by: Markus Handell <handellm@webrtc.org> Commit-Queue: Markus Handell <handellm@webrtc.org> Cr-Commit-Position: refs/heads/master@{#31668}
diff --git a/test/time_controller/BUILD.gn b/test/time_controller/BUILD.gn index ac396b9..c3d5dc9 100644 --- a/test/time_controller/BUILD.gn +++ b/test/time_controller/BUILD.gn
@@ -37,7 +37,6 @@ "../../rtc_base:checks", "../../rtc_base:rtc_base_tests_utils", "../../rtc_base:rtc_event", - "../../rtc_base/synchronization:mutex", "../../rtc_base/synchronization:sequence_checker", "../../rtc_base/synchronization:yield_policy", "../../rtc_base/task_utils:to_queued_task",
diff --git a/test/time_controller/simulated_process_thread.cc b/test/time_controller/simulated_process_thread.cc index e001841..df90f54 100644 --- a/test/time_controller/simulated_process_thread.cc +++ b/test/time_controller/simulated_process_thread.cc
@@ -39,7 +39,7 @@ void SimulatedProcessThread::RunReady(Timestamp at_time) { CurrentTaskQueueSetter set_current(this); - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); std::vector<Module*> ready_modules; for (auto it = delayed_modules_.begin(); it != delayed_modules_.end() && it->first <= at_time; @@ -63,10 +63,10 @@ while (!queue_.empty()) { std::unique_ptr<QueuedTask> task = std::move(queue_.front()); queue_.pop_front(); - lock_.Unlock(); + lock_.Leave(); bool should_delete = task->Run(); RTC_CHECK(should_delete); - lock_.Lock(); + lock_.Enter(); } RTC_DCHECK(queue_.empty()); if (!delayed_modules_.empty()) { @@ -81,7 +81,7 @@ void SimulatedProcessThread::Start() { std::vector<Module*> starting; { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); if (process_thread_running_) return; process_thread_running_ = true; @@ -91,7 +91,7 @@ module->ProcessThreadAttached(this); Timestamp at_time = handler_->CurrentTime(); - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); for (auto& module : starting) delayed_modules_[GetNextTime(module, at_time)].push_back(module); @@ -107,7 +107,7 @@ void SimulatedProcessThread::Stop() { std::vector<Module*> stopping; { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); process_thread_running_ = false; for (auto& delayed : delayed_modules_) { @@ -123,7 +123,7 @@ } void SimulatedProcessThread::WakeUp(Module* module) { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); for (auto it = delayed_modules_.begin(); it != delayed_modules_.end(); ++it) { if (RemoveByValue(&it->second, module)) break; @@ -136,7 +136,7 @@ void SimulatedProcessThread::RegisterModule(Module* module, const rtc::Location& from) { module->ProcessThreadAttached(this); - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); if (!process_thread_running_) { stopped_modules_.push_back(module); } else { @@ -149,7 +149,7 @@ void SimulatedProcessThread::DeRegisterModule(Module* module) { bool modules_running; { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); if (!process_thread_running_) { RemoveByValue(&stopped_modules_, module); } else { @@ -165,14 +165,14 @@ } void SimulatedProcessThread::PostTask(std::unique_ptr<QueuedTask> task) { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); queue_.emplace_back(std::move(task)); next_run_time_ = Timestamp::MinusInfinity(); } void SimulatedProcessThread::PostDelayedTask(std::unique_ptr<QueuedTask> task, uint32_t milliseconds) { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); Timestamp target_time = handler_->CurrentTime() + TimeDelta::Millis(milliseconds); delayed_tasks_[target_time].push_back(std::move(task));
diff --git a/test/time_controller/simulated_process_thread.h b/test/time_controller/simulated_process_thread.h index 54d5db7..6026826 100644 --- a/test/time_controller/simulated_process_thread.h +++ b/test/time_controller/simulated_process_thread.h
@@ -16,7 +16,6 @@ #include <memory> #include <vector> -#include "rtc_base/synchronization/mutex.h" #include "test/time_controller/simulated_time_controller.h" namespace webrtc { @@ -30,7 +29,7 @@ void RunReady(Timestamp at_time) override; Timestamp GetNextRunTime() const override { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); return next_run_time_; } @@ -56,7 +55,7 @@ sim_time_impl::SimulatedTimeControllerImpl* const handler_; // Using char* to be debugger friendly. char* name_; - mutable Mutex lock_; + rtc::CriticalSection lock_; Timestamp next_run_time_ RTC_GUARDED_BY(lock_) = Timestamp::PlusInfinity(); std::deque<std::unique_ptr<QueuedTask>> queue_;
diff --git a/test/time_controller/simulated_task_queue.cc b/test/time_controller/simulated_task_queue.cc index 721fcba..6bc96c73 100644 --- a/test/time_controller/simulated_task_queue.cc +++ b/test/time_controller/simulated_task_queue.cc
@@ -28,7 +28,7 @@ void SimulatedTaskQueue::Delete() { { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); ready_tasks_.clear(); delayed_tasks_.clear(); } @@ -36,7 +36,7 @@ } void SimulatedTaskQueue::RunReady(Timestamp at_time) { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); for (auto it = delayed_tasks_.begin(); it != delayed_tasks_.end() && it->first <= at_time; it = delayed_tasks_.erase(it)) { @@ -48,14 +48,14 @@ while (!ready_tasks_.empty()) { std::unique_ptr<QueuedTask> ready = std::move(ready_tasks_.front()); ready_tasks_.pop_front(); - lock_.Unlock(); + lock_.Leave(); bool delete_task = ready->Run(); if (delete_task) { ready.reset(); } else { ready.release(); } - lock_.Lock(); + lock_.Enter(); } if (!delayed_tasks_.empty()) { next_run_time_ = delayed_tasks_.begin()->first; @@ -65,14 +65,14 @@ } void SimulatedTaskQueue::PostTask(std::unique_ptr<QueuedTask> task) { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); ready_tasks_.emplace_back(std::move(task)); next_run_time_ = Timestamp::MinusInfinity(); } void SimulatedTaskQueue::PostDelayedTask(std::unique_ptr<QueuedTask> task, uint32_t milliseconds) { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); Timestamp target_time = handler_->CurrentTime() + TimeDelta::Millis(milliseconds); delayed_tasks_[target_time].push_back(std::move(task));
diff --git a/test/time_controller/simulated_task_queue.h b/test/time_controller/simulated_task_queue.h index 5035f79..940117c 100644 --- a/test/time_controller/simulated_task_queue.h +++ b/test/time_controller/simulated_task_queue.h
@@ -15,7 +15,6 @@ #include <memory> #include <vector> -#include "rtc_base/synchronization/mutex.h" #include "test/time_controller/simulated_time_controller.h" namespace webrtc { @@ -31,7 +30,7 @@ void RunReady(Timestamp at_time) override; Timestamp GetNextRunTime() const override { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); return next_run_time_; } TaskQueueBase* GetAsTaskQueue() override { return this; } @@ -47,7 +46,7 @@ // Using char* to be debugger friendly. char* name_; - mutable Mutex lock_; + rtc::CriticalSection lock_; std::deque<std::unique_ptr<QueuedTask>> ready_tasks_ RTC_GUARDED_BY(lock_); std::map<Timestamp, std::vector<std::unique_ptr<QueuedTask>>> delayed_tasks_
diff --git a/test/time_controller/simulated_thread.cc b/test/time_controller/simulated_thread.cc index 8071264..8d1637c 100644 --- a/test/time_controller/simulated_thread.cc +++ b/test/time_controller/simulated_thread.cc
@@ -59,7 +59,7 @@ CurrentThreadSetter set_current(this); ProcessMessages(0); int delay_ms = GetDelay(); - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); if (delay_ms == kForever) { next_run_time_ = Timestamp::PlusInfinity(); } else { @@ -95,7 +95,7 @@ rtc::MessageData* pdata, bool time_sensitive) { rtc::Thread::Post(posted_from, phandler, id, pdata, time_sensitive); - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); next_run_time_ = Timestamp::MinusInfinity(); } @@ -105,7 +105,7 @@ uint32_t id, rtc::MessageData* pdata) { rtc::Thread::PostDelayed(posted_from, delay_ms, phandler, id, pdata); - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); next_run_time_ = std::min(next_run_time_, Timestamp::Millis(rtc::TimeMillis() + delay_ms)); } @@ -116,7 +116,7 @@ uint32_t id, rtc::MessageData* pdata) { rtc::Thread::PostAt(posted_from, target_time_ms, phandler, id, pdata); - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); next_run_time_ = std::min(next_run_time_, Timestamp::Millis(target_time_ms)); }
diff --git a/test/time_controller/simulated_thread.h b/test/time_controller/simulated_thread.h index b6c1e6e..fd39696 100644 --- a/test/time_controller/simulated_thread.h +++ b/test/time_controller/simulated_thread.h
@@ -12,7 +12,6 @@ #include <memory> -#include "rtc_base/synchronization/mutex.h" #include "test/time_controller/simulated_time_controller.h" namespace webrtc { @@ -29,7 +28,7 @@ void RunReady(Timestamp at_time) override; Timestamp GetNextRunTime() const override { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); return next_run_time_; } @@ -62,7 +61,7 @@ sim_time_impl::SimulatedTimeControllerImpl* const handler_; // Using char* to be debugger friendly. char* name_; - mutable Mutex lock_; + rtc::CriticalSection lock_; Timestamp next_run_time_ RTC_GUARDED_BY(lock_) = Timestamp::PlusInfinity(); };
diff --git a/test/time_controller/simulated_time_controller.cc b/test/time_controller/simulated_time_controller.cc index aba8c66..769be3f 100644 --- a/test/time_controller/simulated_time_controller.cc +++ b/test/time_controller/simulated_time_controller.cc
@@ -95,7 +95,7 @@ // Using a dummy thread rather than nullptr to avoid implicit thread creation // by Thread::Current(). SimulatedThread::CurrentThreadSetter set_current(dummy_thread_.get()); - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); RTC_DCHECK_EQ(rtc::CurrentThreadId(), thread_id_); Timestamp current_time = CurrentTime(); // Clearing |ready_runners_| in case this is a recursive call: @@ -116,25 +116,25 @@ while (!ready_runners_.empty()) { auto* runner = ready_runners_.front(); ready_runners_.pop_front(); - lock_.Unlock(); + lock_.Leave(); // Note that the RunReady function might indirectly cause a call to // Unregister() which will grab |lock_| again to remove items from // |ready_runners_|. runner->RunReady(current_time); - lock_.Lock(); + lock_.Enter(); } } } Timestamp SimulatedTimeControllerImpl::CurrentTime() const { - MutexLock lock(&time_lock_); + rtc::CritScope lock(&time_lock_); return current_time_; } Timestamp SimulatedTimeControllerImpl::NextRunTime() const { Timestamp current_time = CurrentTime(); Timestamp next_time = Timestamp::PlusInfinity(); - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); for (auto* runner : runners_) { Timestamp next_run_time = runner->GetNextRunTime(); if (next_run_time <= current_time) @@ -145,18 +145,18 @@ } void SimulatedTimeControllerImpl::AdvanceTime(Timestamp target_time) { - MutexLock time_lock(&time_lock_); + rtc::CritScope time_lock(&time_lock_); RTC_DCHECK_GE(target_time, current_time_); current_time_ = target_time; } void SimulatedTimeControllerImpl::Register(SimulatedSequenceRunner* runner) { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); runners_.push_back(runner); } void SimulatedTimeControllerImpl::Unregister(SimulatedSequenceRunner* runner) { - MutexLock lock(&lock_); + rtc::CritScope lock(&lock_); bool removed = RemoveByValue(&runners_, runner); RTC_CHECK(removed); RemoveByValue(&ready_runners_, runner);
diff --git a/test/time_controller/simulated_time_controller.h b/test/time_controller/simulated_time_controller.h index 6c6dbfa..48112b3 100644 --- a/test/time_controller/simulated_time_controller.h +++ b/test/time_controller/simulated_time_controller.h
@@ -21,9 +21,9 @@ #include "api/units/timestamp.h" #include "modules/include/module.h" #include "modules/utility/include/process_thread.h" +#include "rtc_base/critical_section.h" #include "rtc_base/fake_clock.h" #include "rtc_base/platform_thread_types.h" -#include "rtc_base/synchronization/mutex.h" #include "rtc_base/synchronization/yield_policy.h" #include "rtc_base/thread_checker.h" @@ -89,9 +89,9 @@ private: const rtc::PlatformThreadId thread_id_; const std::unique_ptr<rtc::Thread> dummy_thread_ = rtc::Thread::Create(); - mutable Mutex time_lock_; + rtc::CriticalSection time_lock_; Timestamp current_time_ RTC_GUARDED_BY(time_lock_); - mutable Mutex lock_; + rtc::CriticalSection lock_; std::vector<SimulatedSequenceRunner*> runners_ RTC_GUARDED_BY(lock_); // Used in RunReadyRunners() to keep track of ready runners that are to be // processed in a round robin fashion. the reason it's a member is so that