[atomics] Fix critical section for Atomics.waitAsync

Loading the value at the index for the futex wait should be protected by
the waiterlist mutex for both sync and async waits.

Bug: chromium:1194026
Change-Id: Ie9896cab6828763ebb963f5ad96f264d57c9377f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2796159
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73753}

src/execution/futex-emulation.cc

@@ -517,13 +517,6 @@ FutexWaitListNode::FutexWaitListNode(
       Utils::ToLocal(Handle<Context>::cast(native_context));
   native_context_.Reset(v8_isolate, local_native_context);
   native_context_.SetWeak();
-
-  // Add the Promise into the NativeContext's atomics_waitasync_promises set, so
-  // that the list keeps it alive.
-  Handle<OrderedHashSet> promises(native_context->atomics_waitasync_promises(),
-                                  isolate);
-  promises = OrderedHashSet::Add(isolate, promises, promise).ToHandleChecked();
-  native_context->set_atomics_waitasync_promises(*promises);
 }
 
 template <typename T>
@@ -536,77 +529,108 @@ Object FutexEmulation::WaitAsync(Isolate* isolate,
 
   Factory* factory = isolate->factory();
   Handle<JSObject> result = factory->NewJSObject(isolate->object_function());
+  Handle<JSObject> promise_capability = factory->NewJSPromise();
 
-  std::shared_ptr<BackingStore> backing_store = array_buffer->GetBackingStore();
+  enum { kNotEqual, kTimedOut, kAsync } result_kind;
+  FutexWaitListNode* node = nullptr;
+  {
+    // 16. Perform EnterCriticalSection(WL).
+    NoGarbageCollectionMutexGuard lock_guard(g_mutex.Pointer());
+
+    std::shared_ptr<BackingStore> backing_store =
+        array_buffer->GetBackingStore();
 
-  // 17. Let w be ! AtomicLoad(typedArray, i).
-  std::atomic<T>* p = reinterpret_cast<std::atomic<T>*>(
-      static_cast<int8_t*>(backing_store->buffer_start()) + addr);
-  if (p->load() != value) {
-    // 18. If v is not equal to w, then
-    //   a. Perform LeaveCriticalSection(WL).
-    //   ...
-    //   c. Perform ! CreateDataPropertyOrThrow(resultObject, "async", false).
-    //   d. Perform ! CreateDataPropertyOrThrow(resultObject, "value",
-    //     "not-equal").
-    //   e. Return resultObject.
-    CHECK(
-        JSReceiver::CreateDataProperty(isolate, result, factory->async_string(),
-                                       factory->false_value(), Just(kDontThrow))
-            .FromJust());
-    CHECK(JSReceiver::CreateDataProperty(
-              isolate, result, factory->value_string(),
-              factory->not_equal_string(), Just(kDontThrow))
-              .FromJust());
-    return *result;
-  }
-
-  if (use_timeout && rel_timeout_ns == 0) {
-    // 19. If t is 0 and mode is async, then
-    //   ...
-    //   b. Perform LeaveCriticalSection(WL).
-    //   c. Perform ! CreateDataPropertyOrThrow(resultObject, "async", false).
-    //   d. Perform ! CreateDataPropertyOrThrow(resultObject, "value",
-    //     "timed-out").
-    //   e. Return resultObject.
-    CHECK(
-        JSReceiver::CreateDataProperty(isolate, result, factory->async_string(),
-                                       factory->false_value(), Just(kDontThrow))
-            .FromJust());
-    CHECK(JSReceiver::CreateDataProperty(
-              isolate, result, factory->value_string(),
-              factory->timed_out_string(), Just(kDontThrow))
-              .FromJust());
-    return *result;
+    // 17. Let w be ! AtomicLoad(typedArray, i).
+    std::atomic<T>* p = reinterpret_cast<std::atomic<T>*>(
+        static_cast<int8_t*>(backing_store->buffer_start()) + addr);
+    if (p->load() != value) {
+      result_kind = kNotEqual;
+    } else if (use_timeout && rel_timeout_ns == 0) {
+      result_kind = kTimedOut;
+    } else {
+      result_kind = kAsync;
+      node = new FutexWaitListNode(backing_store, addr, promise_capability,
+                                   isolate);
+      g_wait_list.Pointer()->AddNode(node);
+    }
+
+    // Leaving the block collapses the following steps:
+    // 18.a. Perform LeaveCriticalSection(WL).
+    // 19.b. Perform LeaveCriticalSection(WL).
+    // 24. Perform LeaveCriticalSection(WL).
   }
 
-  Handle<JSObject> promise_capability = factory->NewJSPromise();
-  FutexWaitListNode* node =
-      new FutexWaitListNode(backing_store, addr, promise_capability, isolate);
+  switch (result_kind) {
+    case kNotEqual:
+      // 18. If v is not equal to w, then
+      //   ...
+      //   c. Perform ! CreateDataPropertyOrThrow(resultObject, "async", false).
+      //   d. Perform ! CreateDataPropertyOrThrow(resultObject, "value",
+      //     "not-equal").
+      //   e. Return resultObject.
+      CHECK(JSReceiver::CreateDataProperty(
+                isolate, result, factory->async_string(),
+                factory->false_value(), Just(kDontThrow))
+                .FromJust());
+      CHECK(JSReceiver::CreateDataProperty(
+                isolate, result, factory->value_string(),
+                factory->not_equal_string(), Just(kDontThrow))
+                .FromJust());
+      break;
 
-  {
-    NoGarbageCollectionMutexGuard lock_guard(g_mutex.Pointer());
-    g_wait_list.Pointer()->AddNode(node);
+    case kTimedOut:
+      // 19. If t is 0 and mode is async, then
+      //   ...
+      //   c. Perform ! CreateDataPropertyOrThrow(resultObject, "async", false).
+      //   d. Perform ! CreateDataPropertyOrThrow(resultObject, "value",
+      //     "timed-out").
+      //   e. Return resultObject.
+      CHECK(JSReceiver::CreateDataProperty(
+                isolate, result, factory->async_string(),
+                factory->false_value(), Just(kDontThrow))
+                .FromJust());
+      CHECK(JSReceiver::CreateDataProperty(
+                isolate, result, factory->value_string(),
+                factory->timed_out_string(), Just(kDontThrow))
+                .FromJust());
+      break;
+
+    case kAsync:
+      DCHECK_NOT_NULL(node);
+
+      // Add the Promise into the NativeContext's atomics_waitasync_promises
+      // set, so that the list keeps it alive.
+      Handle<NativeContext> native_context(isolate->native_context());
+      Handle<OrderedHashSet> promises(
+          native_context->atomics_waitasync_promises(), isolate);
+      promises = OrderedHashSet::Add(isolate, promises, promise_capability)
+                     .ToHandleChecked();
+      native_context->set_atomics_waitasync_promises(*promises);
+
+      if (use_timeout) {
+        node->async_timeout_time_ = base::TimeTicks::Now() + rel_timeout;
+        auto task = std::make_unique<AsyncWaiterTimeoutTask>(
+            node->cancelable_task_manager_, node);
+        node->timeout_task_id_ = task->id();
+        node->task_runner_->PostNonNestableDelayedTask(
+            std::move(task), rel_timeout.InSecondsF());
+      }
+
+      // 26. Perform ! CreateDataPropertyOrThrow(resultObject, "async", true).
+      // 27. Perform ! CreateDataPropertyOrThrow(resultObject, "value",
+      // promiseCapability.[[Promise]]).
+      // 28. Return resultObject.
+      CHECK(JSReceiver::CreateDataProperty(
+                isolate, result, factory->async_string(), factory->true_value(),
+                Just(kDontThrow))
+                .FromJust());
+      CHECK(JSReceiver::CreateDataProperty(isolate, result,
+                                           factory->value_string(),
+                                           promise_capability, Just(kDontThrow))
+                .FromJust());
+      break;
   }
-  if (use_timeout) {
-    node->async_timeout_time_ = base::TimeTicks::Now() + rel_timeout;
-    auto task = std::make_unique<AsyncWaiterTimeoutTask>(
-        node->cancelable_task_manager_, node);
-    node->timeout_task_id_ = task->id();
-    node->task_runner_->PostNonNestableDelayedTask(std::move(task),
-                                                   rel_timeout.InSecondsF());
-  }
-
-  // 26. Perform ! CreateDataPropertyOrThrow(resultObject, "async", true).
-  // 27. Perform ! CreateDataPropertyOrThrow(resultObject, "value",
-  // promiseCapability.[[Promise]]).
-  // 28. Return resultObject.
-  CHECK(JSReceiver::CreateDataProperty(isolate, result, factory->async_string(),
-                                       factory->true_value(), Just(kDontThrow))
-            .FromJust());
-  CHECK(JSReceiver::CreateDataProperty(isolate, result, factory->value_string(),
-                                       promise_capability, Just(kDontThrow))
-            .FromJust());
+
   return *result;
 }