[CompilerDispatcher] Fix unittest flake to avoid posting an idle task after aborted.

There was a small race where an idle task could be posted after the compiler dispatcher
had aborted and the CancellableTaskRunner had been cancelled. This was causing flakyness
on the bots. This fixes this by moving the idle task posting into the same lock block as
the notification to the main thread that the background task has completed.

BUG=v8:8041

Change-Id: I43ca4cea807bfdfeb13f6d1c4a67a4d8a4f6291f
Reviewed-on: https://chromium-review.googlesource.com/c/1278494Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56607}

src/compiler-dispatcher/compiler-dispatcher.cc

@@ -136,16 +136,13 @@ void CompilerDispatcher::RegisterSharedFunctionInfo(
   Job* job = job_it->second.get();
   shared_to_unoptimized_job_id_.Set(function_handle, job_id);
 
-  // Schedule an idle task to finalize job if it is ready.
-  bool is_ready_to_finalize;
   {
     base::LockGuard<base::Mutex> lock(&mutex_);
     job->function = function_handle;
-    is_ready_to_finalize = job->IsReadyToFinalize(lock);
-  }
-  // TODO(rmcilroy): Move idle task posting into locked block above.
-  if (is_ready_to_finalize) {
-    ScheduleIdleTaskIfNeeded();
+    if (job->IsReadyToFinalize(lock)) {
+      // Schedule an idle task to finalize job if it is ready.
+      ScheduleIdleTaskFromAnyThread(lock);
+    }
   }
 }
 
@@ -303,23 +300,17 @@ CompilerDispatcher::JobMap::const_iterator CompilerDispatcher::GetJobFor(
   return job;
 }
 
-void CompilerDispatcher::ScheduleIdleTaskFromAnyThread() {
+void CompilerDispatcher::ScheduleIdleTaskFromAnyThread(
+    const base::LockGuard<base::Mutex>&) {
   if (!taskrunner_->IdleTasksEnabled()) return;
-  {
-    base::LockGuard<base::Mutex> lock(&mutex_);
-    if (idle_task_scheduled_ || abort_) return;
-    idle_task_scheduled_ = true;
-  }
+  if (idle_task_scheduled_ || abort_) return;
+
+  idle_task_scheduled_ = true;
   taskrunner_->PostIdleTask(MakeCancelableIdleLambdaTask(
       task_manager_.get(),
       [this](double deadline_in_seconds) { DoIdleWork(deadline_in_seconds); }));
 }
 
-void CompilerDispatcher::ScheduleIdleTaskIfNeeded() {
-  if (jobs_.empty()) return;
-  ScheduleIdleTaskFromAnyThread();
-}
-
 void CompilerDispatcher::ScheduleAbortTask() {
   taskrunner_->PostTask(MakeCancelableLambdaTask(
       task_manager_.get(), [this] { AbortInactiveJobs(); }));
@@ -367,25 +358,22 @@ void CompilerDispatcher::DoBackgroundWork() {
 
     job->task->Run();
 
-    bool is_ready_to_finalize;
     {
       base::LockGuard<base::Mutex> lock(&mutex_);
       running_background_jobs_.erase(job);
 
       job->has_run = true;
-      is_ready_to_finalize = job->IsReadyToFinalize(lock);
+      if (job->IsReadyToFinalize(lock)) {
+        // Schedule an idle task to finalize the compilation on the main thread
+        // if the job has a shared function info registered.
+        ScheduleIdleTaskFromAnyThread(lock);
+      }
+
       if (main_thread_blocking_on_job_ == job) {
         main_thread_blocking_on_job_ = nullptr;
         main_thread_blocking_signal_.NotifyOne();
       }
     }
-
-    // Schedule an idle task to finalize the compilation on the main thread if
-    // the job has a shared function info registered.
-    // TODO(rmcilroy) Move this into the locked section above.
-    if (is_ready_to_finalize) {
-      ScheduleIdleTaskFromAnyThread();
-    }
   }
 
   {
@@ -450,7 +438,10 @@ void CompilerDispatcher::DoIdleWork(double deadline_in_seconds) {
   }
 
   // We didn't return above so there still might be jobs to finalize.
-  ScheduleIdleTaskIfNeeded();
+  {
+    base::LockGuard<base::Mutex> lock(&mutex_);
+    ScheduleIdleTaskFromAnyThread(lock);
+  }
 }
 
 CompilerDispatcher::JobMap::const_iterator CompilerDispatcher::InsertJob(

src/compiler-dispatcher/compiler-dispatcher.h

@@ -150,8 +150,7 @@ class V8_EXPORT_PRIVATE CompilerDispatcher {
   void AbortInactiveJobs();
   JobMap::const_iterator GetJobFor(Handle<SharedFunctionInfo> shared) const;
   void ScheduleMoreWorkerTasksIfNeeded();
-  void ScheduleIdleTaskFromAnyThread();
-  void ScheduleIdleTaskIfNeeded();
+  void ScheduleIdleTaskFromAnyThread(const base::LockGuard<base::Mutex>&);
   void ScheduleAbortTask();
   void DoBackgroundWork();
   void DoIdleWork(double deadline_in_seconds);