TaskScheduler: Prevent single-threaded CONTINUE_ON_SHUTDOWN from blocking shutdown.

Previously, tasks posted to a shared SingleThreadTaskRunner with the
same traits were scheduled on the same thread. If single-threaded task A
(BACKGROUND, CONTINUE_ON_SHUTDOWN) was scheduled on the thread and
single-threaded task B (BACKGROUND, BLOCK_SHUTDOWN) was posted
afterwards, shutdown could not complete until task A finished and
allowed task B to run on the thread.

This CL fixes the issue by scheduling shared single-threaded tasks
that are CONTINUE_ON_SHUTDOWN/non-CONTINUE_ON_SHUTDOWN on different
threads.

Bug: 829786
Change-Id: Ia0b556971895c2a5799157f8dcf1a1ee6ce650c9
Reviewed-on: https://chromium-review.googlesource.com/1005422
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550216}

base/task_scheduler/scheduler_single_thread_task_runner_manager.cc

@@ -386,6 +386,10 @@ SchedulerSingleThreadTaskRunnerManager::SchedulerSingleThreadTaskRunnerManager(
                     arraysize(shared_scheduler_workers_),
                 "The size of |shared_com_scheduler_workers_| must match "
                 "|shared_scheduler_workers_|");
+  static_assert(arraysize(shared_com_scheduler_workers_[0]) ==
+                    arraysize(shared_scheduler_workers_[0]),
+                "The size of |shared_com_scheduler_workers_| must match "
+                "|shared_scheduler_workers_|");
 #endif  // defined(OS_WIN)
   DCHECK(!g_manager_is_alive);
   g_manager_is_alive = true;
@@ -431,6 +435,15 @@ SchedulerSingleThreadTaskRunnerManager::CreateCOMSTATaskRunnerWithTraits(
 }
 #endif  // defined(OS_WIN)
 
+// static
+SchedulerSingleThreadTaskRunnerManager::ContinueOnShutdown
+SchedulerSingleThreadTaskRunnerManager::TraitsToContinueOnShutdown(
+    const TaskTraits& traits) {
+  if (traits.shutdown_behavior() == TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN)
+    return IS_CONTINUE_ON_SHUTDOWN;
+  return IS_NOT_CONTINUE_ON_SHUTDOWN;
+}
+
 template <typename DelegateType>
 scoped_refptr<
     SchedulerSingleThreadTaskRunnerManager::SchedulerSingleThreadTaskRunner>
@@ -541,7 +554,8 @@ template <>
 SchedulerWorker*&
 SchedulerSingleThreadTaskRunnerManager::GetSharedSchedulerWorkerForTraits<
     SchedulerWorkerDelegate>(const TaskTraits& traits) {
-  return shared_scheduler_workers_[GetEnvironmentIndexForTraits(traits)];
+  return shared_scheduler_workers_[GetEnvironmentIndexForTraits(traits)]
+                                  [TraitsToContinueOnShutdown(traits)];
 }
 
 #if defined(OS_WIN)
@@ -549,7 +563,8 @@ template <>
 SchedulerWorker*&
 SchedulerSingleThreadTaskRunnerManager::GetSharedSchedulerWorkerForTraits<
     SchedulerWorkerCOMDelegate>(const TaskTraits& traits) {
-  return shared_com_scheduler_workers_[GetEnvironmentIndexForTraits(traits)];
+  return shared_com_scheduler_workers_[GetEnvironmentIndexForTraits(traits)]
+                                      [TraitsToContinueOnShutdown(traits)];
 }
 #endif  // defined(OS_WIN)
 
@@ -585,23 +600,28 @@ void SchedulerSingleThreadTaskRunnerManager::ReleaseSharedSchedulerWorkers() {
   {
     AutoSchedulerLock auto_lock(lock_);
     for (size_t i = 0; i < arraysize(shared_scheduler_workers_); ++i) {
-      local_shared_scheduler_workers[i] = shared_scheduler_workers_[i];
-      shared_scheduler_workers_[i] = nullptr;
+      for (size_t j = 0; j < arraysize(shared_scheduler_workers_[i]); ++j) {
+        local_shared_scheduler_workers[i][j] = shared_scheduler_workers_[i][j];
+        shared_scheduler_workers_[i][j] = nullptr;
 #if defined(OS_WIN)
-      local_shared_com_scheduler_workers[i] = shared_com_scheduler_workers_[i];
-      shared_com_scheduler_workers_[i] = nullptr;
+        local_shared_com_scheduler_workers[i][j] =
+            shared_com_scheduler_workers_[i][j];
+        shared_com_scheduler_workers_[i][j] = nullptr;
 #endif
     }
+    }
   }
 
   for (size_t i = 0; i < arraysize(local_shared_scheduler_workers); ++i) {
-    if (local_shared_scheduler_workers[i])
-      UnregisterSchedulerWorker(local_shared_scheduler_workers[i]);
+    for (size_t j = 0; j < arraysize(local_shared_scheduler_workers[i]); ++j) {
+      if (local_shared_scheduler_workers[i][j])
+        UnregisterSchedulerWorker(local_shared_scheduler_workers[i][j]);
 #if defined(OS_WIN)
-    if (local_shared_com_scheduler_workers[i])
-      UnregisterSchedulerWorker(local_shared_com_scheduler_workers[i]);
+      if (local_shared_com_scheduler_workers[i][j])
+        UnregisterSchedulerWorker(local_shared_com_scheduler_workers[i][j]);
 #endif
   }
+  }
 }
 
 }  // namespace internal

base/task_scheduler/scheduler_single_thread_task_runner_manager.h

@@ -80,6 +80,15 @@ class BASE_EXPORT SchedulerSingleThreadTaskRunnerManager final {
  private:
   class SchedulerSingleThreadTaskRunner;
 
+  enum ContinueOnShutdown {
+    IS_CONTINUE_ON_SHUTDOWN,
+    IS_NOT_CONTINUE_ON_SHUTDOWN,
+    CONTINUE_ON_SHUTDOWN_COUNT,
+  };
+
+  static ContinueOnShutdown TraitsToContinueOnShutdown(
+      const TaskTraits& traits);
+
   template <typename DelegateType>
   scoped_refptr<SchedulerSingleThreadTaskRunner> CreateTaskRunnerWithTraitsImpl(
       const TaskTraits& traits,
@@ -110,10 +119,17 @@ class BASE_EXPORT SchedulerSingleThreadTaskRunnerManager final {
   std::vector<scoped_refptr<SchedulerWorker>> workers_;
   int next_worker_id_ = 0;
 
-  SchedulerWorker* shared_scheduler_workers_[ENVIRONMENT_COUNT] = {};
-
+  // Workers for SingleThreadTaskRunnerThreadMode::SHARED tasks. It is
+  // important to have separate threads for CONTINUE_ON_SHUTDOWN and non-
+  // CONTINUE_ON_SHUTDOWN to avoid being in a situation where a
+  // CONTINUE_ON_SHUTDOWN task effectively blocks shutdown by preventing a
+  // BLOCK_SHUTDOWN task to be scheduled. https://crbug.com/829786
+  SchedulerWorker* shared_scheduler_workers_[ENVIRONMENT_COUNT]
+                                            [CONTINUE_ON_SHUTDOWN_COUNT] = {};
 #if defined(OS_WIN)
-  SchedulerWorker* shared_com_scheduler_workers_[ENVIRONMENT_COUNT] = {};
+  SchedulerWorker* shared_com_scheduler_workers_[ENVIRONMENT_COUNT]
+                                                [CONTINUE_ON_SHUTDOWN_COUNT] =
+                                                    {};
 #endif  // defined(OS_WIN)
 
   // Set to true when Start() is called.

base/task_scheduler/scheduler_single_thread_task_runner_manager_unittest.cc

@@ -20,6 +20,7 @@
 #include "base/threading/platform_thread.h"
 #include "base/threading/simple_thread.h"
 #include "base/threading/thread.h"
+#include "base/threading/thread_restrictions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 #if defined(OS_WIN)
@@ -194,6 +195,51 @@ TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest,
   });
 }
 
+// Regression test for https://crbug.com/829786
+TEST_F(TaskSchedulerSingleThreadTaskRunnerManagerTest,
+       ContinueOnShutdownDoesNotBlockBlockShutdown) {
+  WaitableEvent task_has_started(WaitableEvent::ResetPolicy::MANUAL,
+                                 WaitableEvent::InitialState::NOT_SIGNALED);
+  WaitableEvent task_can_continue(WaitableEvent::ResetPolicy::MANUAL,
+                                  WaitableEvent::InitialState::NOT_SIGNALED);
+
+  // Post a CONTINUE_ON_SHUTDOWN task that waits on
+  // |task_can_continue| to a shared SingleThreadTaskRunner.
+  single_thread_task_runner_manager_
+      ->CreateSingleThreadTaskRunnerWithTraits(
+          {TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
+          SingleThreadTaskRunnerThreadMode::SHARED)
+      ->PostTask(FROM_HERE, base::BindOnce(
+                                [](WaitableEvent* task_has_started,
+                                   WaitableEvent* task_can_continue) {
+                                  task_has_started->Signal();
+                                  ScopedAllowBaseSyncPrimitivesForTesting
+                                      allow_base_sync_primitives;
+                                  task_can_continue->Wait();
+                                },
+                                Unretained(&task_has_started),
+                                Unretained(&task_can_continue)));
+
+  task_has_started.Wait();
+
+  // Post a BLOCK_SHUTDOWN task to a shared SingleThreadTaskRunner.
+  single_thread_task_runner_manager_
+      ->CreateSingleThreadTaskRunnerWithTraits(
+          {TaskShutdownBehavior::BLOCK_SHUTDOWN},
+          SingleThreadTaskRunnerThreadMode::SHARED)
+      ->PostTask(FROM_HERE, DoNothing());
+
+  // Shutdown should not hang even though the first task hasn't finished.
+  task_tracker_.Shutdown();
+
+  // Let the first task finish.
+  task_can_continue.Signal();
+
+  // Tear down from the test body to prevent accesses to |task_can_continue|
+  // after it goes out of scope.
+  TearDownSingleThreadTaskRunnerManager();
+}
+
 namespace {
 
 class TaskSchedulerSingleThreadTaskRunnerManagerCommonTest