From 8e998f17e0ce24d68e9d68efa78c1f390bf3af47 Mon Sep 17 00:00:00 2001
From: Sebastian Jansson <srte@webrtc.org>
Date: Thu, 30 Jan 2020 16:35:25 +0100
Subject: [PATCH] Fixes stall in SimulatedProcessThread

A previous refactoring introduced an issues in SimulatedProcessThread
causing stalls when task are posted. This CL fixes this and cleans up
the code to make it easier to see that it's correct.

Bug: webrtc:11255
Change-Id: I33d7daa993ad2a4cfe2b63f674692455c2e09d05
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/167380
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Ali Tofigh <alito@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30429}
---
 .../simulated_process_thread.cc               | 42 +++++++------------
 .../simulated_process_thread.h                |  1 -
 2 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/test/time_controller/simulated_process_thread.cc b/test/time_controller/simulated_process_thread.cc
index f34470608f..540e061fa4 100644
--- a/test/time_controller/simulated_process_thread.cc
+++ b/test/time_controller/simulated_process_thread.cc
@@ -40,23 +40,18 @@ SimulatedProcessThread::~SimulatedProcessThread() {
 void SimulatedProcessThread::RunReady(Timestamp at_time) {
   TokenTaskQueue::CurrentTaskQueueSetter set_current(this);
   rtc::CritScope lock(&lock_);
+  std::vector<Module*> ready_modules;
   for (auto it = delayed_modules_.begin();
        it != delayed_modules_.end() && it->first <= at_time;
        it = delayed_modules_.erase(it)) {
     for (auto module : it->second) {
-      ready_modules_.push_back(module);
+      ready_modules.push_back(module);
     }
   }
-  if (!ready_modules_.empty()) {
-    for (auto* module : ready_modules_) {
-      module->Process();
-      delayed_modules_[GetNextTime(module, at_time)].push_back(module);
-    }
-    next_run_time_ = delayed_modules_.begin()->first;
-  } else {
-    next_run_time_ = Timestamp::PlusInfinity();
+  for (auto* module : ready_modules) {
+    module->Process();
+    delayed_modules_[GetNextTime(module, at_time)].push_back(module);
   }
-  ready_modules_.clear();
 
   while (!queue_.empty()) {
     std::unique_ptr<QueuedTask> task = std::move(queue_.front());
@@ -66,6 +61,12 @@ void SimulatedProcessThread::RunReady(Timestamp at_time) {
     RTC_CHECK(should_delete);
     lock_.Enter();
   }
+  RTC_DCHECK(queue_.empty());
+  if (!delayed_modules_.empty()) {
+    next_run_time_ = delayed_modules_.begin()->first;
+  } else {
+    next_run_time_ = Timestamp::PlusInfinity();
+  }
 }
 void SimulatedProcessThread::Start() {
   std::vector<Module*> starting;
@@ -84,7 +85,7 @@ void SimulatedProcessThread::Start() {
   for (auto& module : starting)
     delayed_modules_[GetNextTime(module, at_time)].push_back(module);
 
-  if (!ready_modules_.empty() || !queue_.empty()) {
+  if (!queue_.empty()) {
     next_run_time_ = Timestamp::MinusInfinity();
   } else if (!delayed_modules_.empty()) {
     next_run_time_ = delayed_modules_.begin()->first;
@@ -99,10 +100,6 @@ void SimulatedProcessThread::Stop() {
     rtc::CritScope lock(&lock_);
     process_thread_running_ = false;
 
-    for (auto* ready : ready_modules_)
-      stopped_modules_.push_back(ready);
-    ready_modules_.clear();
-
     for (auto& delayed : delayed_modules_) {
       for (auto mod : delayed.second)
         stopped_modules_.push_back(mod);
@@ -117,12 +114,6 @@ void SimulatedProcessThread::Stop() {
 
 void SimulatedProcessThread::WakeUp(Module* module) {
   rtc::CritScope lock(&lock_);
-  // If we already are planning to run this module as soon as possible, we don't
-  // need to do anything.
-  for (auto mod : ready_modules_)
-    if (mod == module)
-      return;
-
   for (auto it = delayed_modules_.begin(); it != delayed_modules_.end(); ++it) {
     if (RemoveByValue(&it->second, module))
       break;
@@ -152,12 +143,9 @@ void SimulatedProcessThread::DeRegisterModule(Module* module) {
     if (!process_thread_running_) {
       RemoveByValue(&stopped_modules_, module);
     } else {
-      bool removed = RemoveByValue(&ready_modules_, module);
-      if (!removed) {
-        for (auto& pair : delayed_modules_) {
-          if (RemoveByValue(&pair.second, module))
-            break;
-        }
+      for (auto& pair : delayed_modules_) {
+        if (RemoveByValue(&pair.second, module))
+          break;
       }
     }
     modules_running = process_thread_running_;
diff --git a/test/time_controller/simulated_process_thread.h b/test/time_controller/simulated_process_thread.h
index db4c8c2559..d6b132552a 100644
--- a/test/time_controller/simulated_process_thread.h
+++ b/test/time_controller/simulated_process_thread.h
@@ -57,7 +57,6 @@ class SimulatedProcessThread : public TokenTaskQueue,
 
   bool process_thread_running_ RTC_GUARDED_BY(lock_) = false;
   std::vector<Module*> stopped_modules_ RTC_GUARDED_BY(lock_);
-  std::vector<Module*> ready_modules_ RTC_GUARDED_BY(lock_);
   std::map<Timestamp, std::list<Module*>> delayed_modules_
       RTC_GUARDED_BY(lock_);
 };