[SAB] Fix flake in Atomics.wait

As specified in https://tc39.github.io/ecma262/#sec-atomics-wait, the
critical section must occur before the load and comparison.

This slightly changes the `AtomicsWaitCallback` API, but in a
direction that arguably makes it more consistent.

As a drive-by fix, reset `node->waiting_` in case there
was an exception from the first callback.

Refs: https://chromium-review.googlesource.com/c/v8/v8/+/1095814
Bug: v8:7836
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I577cdf76cedfe39bc61f783203b543c7c68fc238
Reviewed-on: https://chromium-review.googlesource.com/1096236Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Ben Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53673}

include/v8.h

@@ -7626,7 +7626,9 @@ class V8_EXPORT Isolate {
     /** `Atomics.wait()` was interrupted through |TerminateExecution()|. */
     kTerminatedExecution,
     /** `Atomics.wait()` was stopped through |AtomicsWaitWakeHandle|. */
-    kAPIStopped
+    kAPIStopped,
+    /** `Atomics.wait()` did not wait, as the initial condition was not met. */
+    kNotEqual
   };
 
   /**
@@ -7674,10 +7676,6 @@ class V8_EXPORT Isolate {
    *
    * This callback may schedule exceptions, *unless* |event| is equal to
    * |kTerminatedExecution|.
-   *
-   * This callback is not called if |value| did not match the expected value
-   * inside the SharedArrayBuffer and `Atomics.wait()` returns immediately
-   * because of that.
    */
   typedef void (*AtomicsWaitCallback)(AtomicsWaitEvent event,
                                       Local<SharedArrayBuffer> array_buffer,

src/futex-emulation.cc

@@ -87,12 +87,7 @@ Object* FutexEmulation::Wait(Isolate* isolate,
   int32_t* p =
       reinterpret_cast<int32_t*>(static_cast<int8_t*>(backing_store) + addr);
 
-  if (*p != value) {
-    return isolate->heap()->not_equal();
-  }
-
   FutexWaitListNode* node = isolate->futex_wait_list_node();
-
   node->backing_store_ = backing_store;
   node->wait_addr_ = addr;
   node->waiting_ = true;
@@ -116,24 +111,36 @@ Object* FutexEmulation::Wait(Isolate* isolate,
     }
   }
 
-  base::TimeTicks start_time = base::TimeTicks::Now();
-  base::TimeTicks timeout_time = start_time + rel_timeout;
-  base::TimeTicks current_time = start_time;
-
   AtomicsWaitWakeHandle stop_handle(isolate);
 
   isolate->RunAtomicsWaitCallback(AtomicsWaitEvent::kStartWait, array_buffer,
                                   addr, value, rel_timeout_ms, &stop_handle);
 
   if (isolate->has_scheduled_exception()) {
+    node->waiting_ = false;
     return isolate->PromoteScheduledException();
   }
 
   Object* result;
   AtomicsWaitEvent callback_result = AtomicsWaitEvent::kWokenUp;
 
-  {
+  do {  // Not really a loop, just makes it easier to break out early.
     base::LockGuard<base::Mutex> lock_guard(mutex_.Pointer());
+
+    if (*p != value) {
+      result = isolate->heap()->not_equal();
+      callback_result = AtomicsWaitEvent::kNotEqual;
+      break;
+    }
+
+    base::TimeTicks timeout_time;
+    base::TimeTicks current_time;
+
+    if (use_timeout) {
+      current_time = base::TimeTicks::Now();
+      timeout_time = current_time + rel_timeout;
+    }
+
     wait_list_.Pointer()->AddNode(node);
 
     while (true) {
@@ -205,13 +212,13 @@ Object* FutexEmulation::Wait(Isolate* isolate,
     }
 
     wait_list_.Pointer()->RemoveNode(node);
-  }
+  } while (0);
+
+  node->waiting_ = false;
 
   isolate->RunAtomicsWaitCallback(callback_result, array_buffer, addr, value,
                                   rel_timeout_ms, nullptr);
 
-  node->waiting_ = false;
-
   if (isolate->has_scheduled_exception()) {
     CHECK_NE(callback_result, AtomicsWaitEvent::kTerminatedExecution);
     result = isolate->PromoteScheduledException();

test/cctest/test-api.cc

@@ -28175,6 +28175,12 @@ TEST(AtomicsWaitCallback) {
 
   {
     v8::TryCatch try_catch(isolate);
+    info.expected_offset = 8;
+    info.expected_timeout = std::numeric_limits<double>::infinity();
+    info.expected_value = 1;
+    info.expected_event = v8::Isolate::AtomicsWaitEvent::kNotEqual;
+    info.action = AtomicsWaitCallbackAction::KeepWaiting;
+    info.ncalls = 0;
     CompileRun("Atomics.wait(int32arr, 1, 1);");  // real value is 0 != 1
     CHECK_EQ(info.ncalls, 2);
     CHECK(!try_catch.HasCaught());