Fix sequence_manager_fuzzer race

In the Edge fuzzer runs, we're seeing crashes in sequence_manager_fuzzer
via ThreadManager::PostDelayedTask. The root cause appears to be the
fact that a cross-thread post can execute on one thread while the
thread where the target ThreadManager lives is executing a shutdown
task. The TestTaskQueue pointer that is held by
ThreadManager::PostDelayedTask can be deleted when the
TaskQueueWithVoters is erased from the task_queues_ vector. Keeping a
scoped_refptr on the stack allows safe use of the TestTaskQueue pointer
for the duration of the function, and the task runner infrastructure
gracefully handles posting to a TaskQueue that has gone away.

That change alone fixes the fuzzer crash, but there's still a race -
since only GetTaskQueueFor is protected by the lock (same lock is
acquired before erasing the vector entry), it's possible for the
TaskQueueWithVoters to be destroyed along with its queue before we
take the extra reference when assigning the stack scoped_refptr.
To fix this, I've converted TaskQueueWithVoters to a thread-safe
refcounted type and GetTaskQueueFor now returns a
scoped_refptr<TaskQueueWithVoters> instead of a raw pointer.

Bug: 977527

Change-Id: I9f49531bd7d8c76bae36a65ed150c32714039d45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1835216Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Commit-Queue: Daniel Libby <dlibby@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#703781}

third_party/blink/renderer/platform/scheduler/test/fuzzer/task_queue_with_voters.h

@@ -13,7 +13,8 @@
 namespace base {
 namespace sequence_manager {
 
-struct PLATFORM_EXPORT TaskQueueWithVoters {
+struct PLATFORM_EXPORT TaskQueueWithVoters
+    : public RefCountedThreadSafe<TaskQueueWithVoters> {
   explicit TaskQueueWithVoters(scoped_refptr<TestTaskQueue> task_queue)
       : queue(std::move(task_queue)) {}
 

third_party/blink/renderer/platform/scheduler/test/fuzzer/thread_manager.cc

@@ -58,7 +58,7 @@ ThreadManager::ThreadManager(base::TimeTicks initial_time,
                                      test_task_runner_->GetMockTickClock());
 
   TaskQueue::Spec spec = TaskQueue::Spec("default_task_queue");
-  task_queues_.emplace_back(std::make_unique<TaskQueueWithVoters>(
+  task_queues_.emplace_back(MakeRefCounted<TaskQueueWithVoters>(
       manager_->CreateTaskQueueWithType<TestTaskQueue>(spec)));
 }
 
@@ -157,7 +157,7 @@ void ThreadManager::ExecuteCreateTaskQueueAction(
   TestTaskQueue* chosen_task_queue;
   {
     AutoLock lock(lock_);
-    task_queues_.emplace_back(std::make_unique<TaskQueueWithVoters>(
+    task_queues_.emplace_back(MakeRefCounted<TaskQueueWithVoters>(
         manager_->CreateTaskQueueWithType<TestTaskQueue>(spec)));
     chosen_task_queue = task_queues_.back()->queue.get();
   }
@@ -197,7 +197,10 @@ void ThreadManager::PostDelayedTask(
     uint64_t task_queue_id,
     uint32_t delay_ms,
     const SequenceManagerTestDescription::Task& task) {
-  TestTaskQueue* chosen_task_queue =
+  // PostDelayedTask could be called cross-thread - therefore we need a
+  // refptr to the TestTaskQueue which could potentially be deleted by the
+  // thread on which ThreadManager lives.
+  scoped_refptr<TestTaskQueue> chosen_task_queue =
       GetTaskQueueFor(task_queue_id)->queue.get();
 
   std::unique_ptr<Task> pending_task = std::make_unique<Task>(this);
@@ -240,7 +243,7 @@ void ThreadManager::ExecuteSetQueueEnabledAction(
                                   ActionForTest::ActionType::kSetQueueEnabled,
                                   NowTicks());
 
-  TaskQueueWithVoters* chosen_task_queue =
+  scoped_refptr<TaskQueueWithVoters> chosen_task_queue =
       GetTaskQueueFor(action.task_queue_id());
 
   if (chosen_task_queue->voters.IsEmpty()) {
@@ -261,7 +264,7 @@ void ThreadManager::ExecuteCreateQueueVoterAction(
                                   ActionForTest::ActionType::kCreateQueueVoter,
                                   NowTicks());
 
-  TaskQueueWithVoters* chosen_task_queue =
+  scoped_refptr<TaskQueueWithVoters> chosen_task_queue =
       GetTaskQueueFor(action.task_queue_id());
   chosen_task_queue->voters.push_back(
       chosen_task_queue->queue.get()->CreateQueueEnabledVoter());
@@ -326,7 +329,7 @@ void ThreadManager::ExecuteInsertFenceAction(
                                   ActionForTest::ActionType::kInsertFence,
                                   NowTicks());
 
-  TestTaskQueue* chosen_task_queue =
+  scoped_refptr<TestTaskQueue> chosen_task_queue =
       GetTaskQueueFor(action.task_queue_id())->queue.get();
 
   if (action.position() ==
@@ -347,7 +350,7 @@ void ThreadManager::ExecuteRemoveFenceAction(
                                   ActionForTest::ActionType::kRemoveFence,
                                   NowTicks());
 
-  TestTaskQueue* chosen_task_queue =
+  scoped_refptr<TestTaskQueue> chosen_task_queue =
       GetTaskQueueFor(action.task_queue_id())->queue.get();
   chosen_task_queue->RemoveFence();
 }
@@ -391,7 +394,8 @@ void ThreadManager::DeleteTask(Task* task) {
     pending_tasks_.erase(pending_tasks_.begin() + i);
 }
 
-TaskQueueWithVoters* ThreadManager::GetTaskQueueFor(uint64_t task_queue_id) {
+scoped_refptr<TaskQueueWithVoters> ThreadManager::GetTaskQueueFor(
+    uint64_t task_queue_id) {
   AutoLock lock(lock_);
   DCHECK(!task_queues_.IsEmpty());
   return task_queues_[task_queue_id % task_queues_.size()].get();

third_party/blink/renderer/platform/scheduler/test/fuzzer/thread_manager.h

@@ -127,7 +127,7 @@ class PLATFORM_EXPORT ThreadManager {
   // Used to delete |task| from |pending_tasks_|.
   void DeleteTask(Task* task);
 
-  TaskQueueWithVoters* GetTaskQueueFor(uint64_t task_queue_id);
+  scoped_refptr<TaskQueueWithVoters> GetTaskQueueFor(uint64_t task_queue_id);
 
   // Used to protect |task_queues_| and |pending_tasks_|.
   Lock lock_;
@@ -140,7 +140,7 @@ class PLATFORM_EXPORT ThreadManager {
 
   // For testing purposes, this should follow the order in which queues
   // were created on the thread in which |this| was instantiated.
-  Vector<std::unique_ptr<TaskQueueWithVoters>> task_queues_;
+  Vector<scoped_refptr<TaskQueueWithVoters>> task_queues_;
 
   // Used to be able to cancel pending tasks from the sequence manager. For
   // testing purposes, this should follow the order in which the tasks were