commit ef8fc080fe87abf3a3f18ac62ca2980503681224
author Yuri Wiitala <miu@chromium.org> Wed Sep 25 17:21:23 2019 -0700
committer Commit Bot <commit-bot@chromium.org> Thu Sep 26 00:30:46 2019 +0000
tree 0abeed973606f3139ea76e9739eb28efbbaa5e91
parent 2fdf9668312b500ba424a87a3b06a9dcc952cbd9

Fix for indefinite blockage in TaskRunnerImpl.

Change-Id: Id11a097e2376e42093d9c721ffd0a9e1a22885c2
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/1825871
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Max Yakimakha <yakimakha@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>

platform/impl/task_runner.cc

diff --git a/platform/impl/task_runner.cc b/platform/impl/task_runner.cc
index 7323c47..9bacacf 100644
--- a/platform/impl/task_runner.cc
+++ b/platform/impl/task_runner.cc
@@ -19,7 +19,10 @@
       task_waiter_(event_waiter),
       waiter_timeout_(waiter_timeout) {}
 
-TaskRunnerImpl::~TaskRunnerImpl() = default;
+TaskRunnerImpl::~TaskRunnerImpl() {
+  // Ensure no thread is currently executing inside RunUntilStopped().
+  OSP_DCHECK_EQ(task_runner_thread_id_, std::thread::id());
+}
 
 void TaskRunnerImpl::PostPackagedTask(Task task) {
   std::lock_guard<std::mutex> lock(task_mutex_);
@@ -48,7 +51,7 @@
 }
 
 void TaskRunnerImpl::RunUntilStopped() {
-  OSP_CHECK(!is_running_);
+  OSP_DCHECK(!is_running_);
   task_runner_thread_id_ = std::this_thread::get_id();
   is_running_ = true;
 
@@ -104,19 +107,6 @@
   delayed_tasks_.erase(delayed_tasks_.begin(), end_of_range);
 }
 
-bool TaskRunnerImpl::ShouldWakeUpRunLoop() {
-  if (!is_running_) {
-    return true;
-  }
-
-  if (!tasks_.empty()) {
-    return true;
-  }
-
-  return !delayed_tasks_.empty() &&
-         (delayed_tasks_.begin()->first <= now_function_());
-}
-
 bool TaskRunnerImpl::GrabMoreRunnableTasks() {
   OSP_DCHECK(running_tasks_.empty());
 
@@ -131,36 +121,25 @@
   }
 
   if (task_waiter_) {
-    do {
-      Clock::duration timeout = waiter_timeout_;
-      if (!delayed_tasks_.empty()) {
-        Clock::duration next_task_delta =
-            delayed_tasks_.begin()->first - now_function_();
-        if (next_task_delta < timeout) {
-          timeout = next_task_delta;
-        }
-      }
-      lock.unlock();
-      task_waiter_->WaitForTaskToBePosted(timeout);
-      lock.lock();
-    } while (!ShouldWakeUpRunLoop());
-  } else {
-    // Pass a wait predicate to avoid lost or spurious wakeups.
-    const auto wait_predicate = [this] { return ShouldWakeUpRunLoop(); };
+    Clock::duration timeout = waiter_timeout_;
     if (!delayed_tasks_.empty()) {
-      // We don't have any work to do currently, but have some in the
-      // pipe.
-      OSP_DVLOG << "TaskRunner waiting for lock until delayed task ready...";
-      run_loop_wakeup_.wait_for(lock,
-                                delayed_tasks_.begin()->first - now_function_(),
-                                wait_predicate);
-    } else {
-      // We don't have any work queued.
-      OSP_DVLOG << "TaskRunnerImpl waiting for lock...";
-      run_loop_wakeup_.wait(lock, wait_predicate);
+      Clock::duration next_task_delta =
+          delayed_tasks_.begin()->first - now_function_();
+      if (next_task_delta < timeout) {
+        timeout = next_task_delta;
+      }
     }
+    lock.unlock();
+    task_waiter_->WaitForTaskToBePosted(timeout);
+    return false;
   }
 
+  if (delayed_tasks_.empty()) {
+    run_loop_wakeup_.wait(lock);
+  } else {
+    run_loop_wakeup_.wait_for(lock,
+                              delayed_tasks_.begin()->first - now_function_());
+  }
   return false;
 }