[platform] Fix data race on DefaultJobState::priority_

The {priority_} field is being updated in {DefaultJobState::Join}, under
{mutex_}. In other places though, it is read unprotected (without
holding the mutex), leading to data races.
This CL fixes that by reading the field while holding the mutex and
using the read priority after releasing the mutex.

Note that the {priority_} field is documented to be protected by
{mutex_}, so the unprotected read was a bug.

R=ulan@chromium.org
CC=etiennep@chromium.org

Bug: v8:10822
Change-Id: I80079f3cb6689e26116ffeb33755c6938c4a2cf1
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_linux64_tsan_isolates_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2377685Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69571}

src/libplatform/default-job.cc

@@ -43,6 +43,7 @@ void DefaultJobState::NotifyConcurrencyIncrease() {
   if (is_canceled_.load(std::memory_order_relaxed)) return;
 
   size_t num_tasks_to_post = 0;
+  TaskPriority priority;
   {
     base::MutexGuard guard(&mutex_);
     const size_t max_concurrency = CappedMaxConcurrency(active_workers_);
@@ -51,11 +52,12 @@ void DefaultJobState::NotifyConcurrencyIncrease() {
       num_tasks_to_post = max_concurrency - active_workers_ - pending_tasks_;
       pending_tasks_ += num_tasks_to_post;
     }
+    priority = priority_;
   }
   // Post additional worker tasks to reach |max_concurrency|.
   for (size_t i = 0; i < num_tasks_to_post; ++i) {
-    CallOnWorkerThread(std::make_unique<DefaultJobWorker>(shared_from_this(),
-                                                          job_task_.get()));
+    CallOnWorkerThread(priority, std::make_unique<DefaultJobWorker>(
+                                     shared_from_this(), job_task_.get()));
   }
 }
 
@@ -136,6 +138,7 @@ bool DefaultJobState::CanRunFirstTask() {
 
 bool DefaultJobState::DidRunTask() {
   size_t num_tasks_to_post = 0;
+  TaskPriority priority;
   {
     base::MutexGuard guard(&mutex_);
     const size_t max_concurrency = CappedMaxConcurrency(active_workers_ - 1);
@@ -151,6 +154,7 @@ bool DefaultJobState::DidRunTask() {
       num_tasks_to_post = max_concurrency - active_workers_ - pending_tasks_;
       pending_tasks_ += num_tasks_to_post;
     }
+    priority = priority_;
   }
   // Post additional worker tasks to reach |max_concurrency| in the case that
   // max concurrency increased. This is not strictly necessary, since
@@ -158,8 +162,8 @@ bool DefaultJobState::DidRunTask() {
   // users of PostJob() batch work and tend to call NotifyConcurrencyIncrease()
   // late. Posting here allows us to spawn new workers sooner.
   for (size_t i = 0; i < num_tasks_to_post; ++i) {
-    CallOnWorkerThread(std::make_unique<DefaultJobWorker>(shared_from_this(),
-                                                          job_task_.get()));
+    CallOnWorkerThread(priority, std::make_unique<DefaultJobWorker>(
+                                     shared_from_this(), job_task_.get()));
   }
   return true;
 }
@@ -183,8 +187,9 @@ size_t DefaultJobState::CappedMaxConcurrency(size_t worker_count) const {
                   num_worker_threads_);
 }
 
-void DefaultJobState::CallOnWorkerThread(std::unique_ptr<Task> task) {
-  switch (priority_) {
+void DefaultJobState::CallOnWorkerThread(TaskPriority priority,
+                                         std::unique_ptr<Task> task) {
+  switch (priority) {
     case TaskPriority::kBestEffort:
       return platform_->CallLowPriorityTaskOnWorkerThread(std::move(task));
     case TaskPriority::kUserVisible:

src/libplatform/default-job.h

@@ -73,7 +73,7 @@ class V8_PLATFORM_EXPORT DefaultJobState
   // job.
   size_t CappedMaxConcurrency(size_t worker_count) const;
 
-  void CallOnWorkerThread(std::unique_ptr<Task> task);
+  void CallOnWorkerThread(TaskPriority priority, std::unique_ptr<Task> task);
 
   Platform* const platform_;
   std::unique_ptr<JobTask> job_task_;