From d1273475a38b49b9d8281435e8567475b18f448b Mon Sep 17 00:00:00 2001
From: Maksim Ivanov <[email protected]>
Date: Thu, 16 Jul 2026 14:51:14 +0000
Subject: [PATCH] [testing] Do not throw errors from sync point APIs

Just print an stderr message and return false instead of throwing an
error from %WaitUntilBlocked() or %Resume(). This allows regression
tests with highly specific scenarios to not break CQ/Waterfall when the
actual sequence of events differs from the expected one or the execution
is slower than expected.

This also fixes the flakiness of regress-519664497 on debug builds.

Bug: 519664497, 514998642, 532293195
Change-Id: I0fcf7f3015cb22b3236295379b041b502af5e2a4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8091127
Reviewed-by: Marja Hölttä <[email protected]>
Reviewed-by: Arash Kazemi <[email protected]>
Commit-Queue: Maksim Ivanov <[email protected]>
Cr-Commit-Position: refs/heads/main@{#108758}
---

diff --git a/src/common/synchronization-point-support.cc b/src/common/synchronization-point-support.cc
index 34dcaae..89bcc47 100644
--- a/src/common/synchronization-point-support.cc
+++ b/src/common/synchronization-point-support.cc
@@ -45,32 +45,45 @@
     std::string_view synchronization_point) {
   base::MutexGuard lock(&mutex_);
   auto it = states_.find(synchronization_point);
-  if (it == states_.end()) return false;
-  SyncPointState* state = it->second.get();
-  if (!state->block_requested) return false;
-  state->block_requested = false;
-  state->cv.NotifyAll();
-  return true;
+  if (it != states_.end()) {
+    SyncPointState* state = it->second.get();
+    if (state->block_requested) {
+      state->block_requested = false;
+      state->cv.NotifyAll();
+      return true;
+    }
+  }
+  base::OS::PrintError("Synchronization point not armed\n");
+  return false;
 }
 
 bool SynchronizationPointSupport::WaitUntilBlocked(
-    std::string_view synchronization_point, base::TimeDelta timeout,
-    bool& timed_out) {
-  bool success = false;
-  auto wait_loop = [this, synchronization_point, timeout, &timed_out,
-                    &success]() {
+    std::string_view synchronization_point, base::TimeDelta timeout) {
+  // Safe: map elements are never removed, so the pointer is stable.
+  SyncPointState* state;
+  {
     base::MutexGuard lock(&mutex_);
     auto it = states_.find(synchronization_point);
-    if (it == states_.end()) return;
-    SyncPointState* state = it->second.get();
-    if (!state->block_requested && state->blocked_threads == 0) return;
+    if (it == states_.end()) {
+      base::OS::PrintError("Synchronization point not armed\n");
+      return false;
+    }
+    state = it->second.get();
+  }
 
+  bool success = false;
+  auto wait_loop = [this, state, timeout, &success]() {
+    base::MutexGuard lock(&mutex_);
     const base::TimeTicks start = base::TimeTicks::Now();
     while (state->blocked_threads == 0) {
+      if (!state->block_requested) {
+        base::OS::PrintError("Synchronization point not armed\n");
+        return;
+      }
       const base::TimeDelta remaining =
           start + timeout - base::TimeTicks::Now();
       if (remaining <= base::TimeDelta()) {
-        timed_out = true;
+        base::OS::PrintError("Synchronization point wait timed out\n");
         return;
       }
       std::ignore = state->cv.WaitFor(&mutex_, remaining);
diff --git a/src/common/synchronization-point-support.h b/src/common/synchronization-point-support.h
index 4ebd39d..c9830ef 100644
--- a/src/common/synchronization-point-support.h
+++ b/src/common/synchronization-point-support.h
@@ -44,11 +44,10 @@
   bool Resume(std::string_view synchronization_point);
 
   // Waits until the given synchronization point is reached by some thread.
-  // Returns false if a block wasn't requested or on timeout (in which case
-  // `timed_out` is set to true as well). Note: this does not request a block;
-  // it must be requested first.
+  // Returns false if a block wasn't requested or on timeout. Note: this does
+  // not request a block; it must be requested first.
   bool WaitUntilBlocked(std::string_view synchronization_point,
-                        base::TimeDelta timeout, bool& timed_out);
+                        base::TimeDelta timeout);
 
   // Called when the synchronization point is reached; blocks if requested.
   V8_INLINE void BlockIfRequested(std::string_view synchronization_point) {
diff --git a/src/runtime/runtime-test.cc b/src/runtime/runtime-test.cc
index 72a208a..b6884c2 100644
--- a/src/runtime/runtime-test.cc
+++ b/src/runtime/runtime-test.cc
@@ -2903,13 +2903,9 @@
   DCHECK_EQ(1, args.length());
   CHECK_UNLESS_FUZZING(IsString(args[0]));
   DirectHandle<String> phase_name = args.at<String>(0);
-  bool resumed =
+  bool success =
       SynchronizationPointSupport::Get()->Resume(phase_name->ToStdString());
-  if (!resumed) {
-    return isolate->Throw(*isolate->factory()->NewStringFromAsciiChecked(
-        "No thread is currently blocked at this synchronization point"));
-  }
-  return ReadOnlyRoots(isolate).undefined_value();
+  return isolate->heap()->ToBoolean(success);
 }
 
 // Waits until the given synchronization point is reached. Throws an exception
@@ -2926,15 +2922,9 @@
   base::TimeDelta timeout =
       base::TimeDelta::FromMilliseconds(args.smi_value_at(1));
 
-  bool timed_out = false;
-  bool blocked = SynchronizationPointSupport::Get()->WaitUntilBlocked(
-      phase_name->ToStdString(), timeout, timed_out);
-  if (!blocked) {
-    return isolate->Throw(*isolate->factory()->NewStringFromAsciiChecked(
-        timed_out ? "Synchronization point wait timed out"
-                  : "Synchronization point not found or not armed"));
-  }
-  return ReadOnlyRoots(isolate).undefined_value();
+  bool success = SynchronizationPointSupport::Get()->WaitUntilBlocked(
+      phase_name->ToStdString(), timeout);
+  return isolate->heap()->ToBoolean(success);
 }
 
 }  // namespace internal
diff --git a/test/mjsunit/compiler/regress-519664497.js b/test/mjsunit/compiler/regress-519664497.js
index 566a322..57a7321 100644
--- a/test/mjsunit/compiler/regress-519664497.js
+++ b/test/mjsunit/compiler/regress-519664497.js
@@ -26,13 +26,13 @@
 %OptimizeFunctionOnNextCall(f, "concurrent");
 assertEquals(expected, f());
 
-%WaitUntilBlocked('NewConsString', 1000);
+assertTrue(%WaitUntilBlocked('NewConsString', 1000));
 
 // Internalize the string. This will mutate the string on the main thread
 // and turn it into a ThinString, right while the background thread is
 // stopped at NewConsString.
 %InternalizeString(left);
 
-%Resume('NewConsString');
+assertTrue(%Resume('NewConsString'));
 %WaitForBackgroundOptimization();
 assertEquals(expected, f());
diff --git a/test/mjsunit/regress/regress-511263221.js b/test/mjsunit/regress/regress-511263221.js
index 18d63d6..85fad54 100644
--- a/test/mjsunit/regress/regress-511263221.js
+++ b/test/mjsunit/regress/regress-511263221.js
@@ -8,34 +8,28 @@
   onmessage = function(msg) {
     let sab = new Int32Array(msg.data);
 
-    try {
-      %BlockAt("ExternalizeStringExtensionMakeExternalOneByte", 10000);
-      %BlockAt("ConcurrentConcatenateStrings", 10000);
-      postMessage("ready");
+    %BlockAt("ExternalizeStringExtensionMakeExternalOneByte", 10000);
+    %BlockAt("ConcurrentConcatenateStrings", 10000);
+    postMessage("ready");
 
-      %WaitUntilBlocked("ConcurrentConcatenateStrings", 10000);
-      %WaitUntilBlocked("ExternalizeStringExtensionMakeExternalOneByte", 10000);
+    %WaitUntilBlocked("ConcurrentConcatenateStrings", 10000);
+    %WaitUntilBlocked("ExternalizeStringExtensionMakeExternalOneByte", 10000);
 
-      %BlockAt("StringWriteToFlatConsString", 10000);
-      %Resume("ConcurrentConcatenateStrings");
+    %BlockAt("StringWriteToFlatConsString", 10000);
+    %Resume("ConcurrentConcatenateStrings");
 
-      %WaitUntilBlocked("StringWriteToFlatConsString", 10000);
+    %WaitUntilBlocked("StringWriteToFlatConsString", 10000);
 
-      %Resume("ExternalizeStringExtensionMakeExternalOneByte");
+    %Resume("ExternalizeStringExtensionMakeExternalOneByte");
 
-      Atomics.wait(sab, 0, 0);
+    Atomics.wait(sab, 0, 0);
 

diff --git a/test/mjsunit/compiler/regress-519664497.js b/test/mjsunit/compiler/regress-519664497.js
index 566a322..57a7321 100644
--- a/test/mjsunit/compiler/regress-519664497.js
+++ b/test/mjsunit/compiler/regress-519664497.js
@@ -26,13 +26,13 @@
 %OptimizeFunctionOnNextCall(f, "concurrent");
 assertEquals(expected, f());
 
-%WaitUntilBlocked('NewConsString', 1000);
+assertTrue(%WaitUntilBlocked('NewConsString', 1000));
 
 // Internalize the string. This will mutate the string on the main thread
 // and turn it into a ThinString, right while the background thread is
 // stopped at NewConsString.
 %InternalizeString(left);
 
-%Resume('NewConsString');
+assertTrue(%Resume('NewConsString'));
 %WaitForBackgroundOptimization();
 assertEquals(expected, f());
diff --git a/test/mjsunit/regress/regress-511263221.js b/test/mjsunit/regress/regress-511263221.js
index 18d63d6..85fad54 100644
--- a/test/mjsunit/regress/regress-511263221.js
+++ b/test/mjsunit/regress/regress-511263221.js
@@ -8,34 +8,28 @@
   onmessage = function(msg) {
     let sab = new Int32Array(msg.data);
 
-    try {
-      %BlockAt("ExternalizeStringExtensionMakeExternalOneByte", 10000);
-      %BlockAt("ConcurrentConcatenateStrings", 10000);
-      postMessage("ready");
+    %BlockAt("ExternalizeStringExtensionMakeExternalOneByte", 10000);
+    %BlockAt("ConcurrentConcatenateStrings", 10000);
+    postMessage("ready");
 
-      %WaitUntilBlocked("ConcurrentConcatenateStrings", 10000);
-      %WaitUntilBlocked("ExternalizeStringExtensionMakeExternalOneByte", 10000);
+    %WaitUntilBlocked("ConcurrentConcatenateStrings", 10000);
+    %WaitUntilBlocked("ExternalizeStringExtensionMakeExternalOneByte", 10000);
 
-      %BlockAt("StringWriteToFlatConsString", 10000);
-      %Resume("ConcurrentConcatenateStrings");
+    %BlockAt("StringWriteToFlatConsString", 10000);
+    %Resume("ConcurrentConcatenateStrings");
 
-      %WaitUntilBlocked("StringWriteToFlatConsString", 10000);
+    %WaitUntilBlocked("StringWriteToFlatConsString", 10000);
 
-      %Resume("ExternalizeStringExtensionMakeExternalOneByte");
+    %Resume("ExternalizeStringExtensionMakeExternalOneByte");
 
-      Atomics.wait(sab, 0, 0);
+    Atomics.wait(sab, 0, 0);
 
-      %Resume("StringWriteToFlatConsString");
+    %Resume("StringWriteToFlatConsString");
 
-    } catch(e) {
-    } finally {
-      try {
-        %Resume("ExternalizeStringExtensionMakeExternalOneByte");
-      } catch(e) {}
-      try { %Resume("ConcurrentConcatenateStrings"); } catch(e) {}
-      try { %Resume("StringWriteToFlatConsString"); } catch(e) {}
-      close();
-    }
+    %Resume("ExternalizeStringExtensionMakeExternalOneByte");
+    %Resume("ConcurrentConcatenateStrings");
+    %Resume("StringWriteToFlatConsString");
+    close();
   }
 }
diff --git a/test/mjsunit/sandbox/regress/regress-502997649.js b/test/mjsunit/sandbox/regress/regress-502997649.js
index cbf6978..9b4529f 100644
--- a/test/mjsunit/sandbox/regress/regress-502997649.js
+++ b/test/mjsunit/sandbox/regress/regress-502997649.js
@@ -30,7 +30,7 @@
 %OptimizeFunctionOnNextCall(g, 'concurrent');
 g();
 
-%WaitUntilBlocked('TurbofanEarlyGraphTrimming', 10000);
+assertTrue(%WaitUntilBlocked('TurbofanEarlyGraphTrimming', 10000));
 
 const aMod = Sandbox.getAddressOf(mod);
 const aInst = Sandbox.getAddressOf(inst);
@@ -55,6 +55,6 @@
 
 gc(); gc(); gc();
 
-%Resume('TurbofanEarlyGraphTrimming');
+assertTrue(%Resume('TurbofanEarlyGraphTrimming'));
 
 %WaitForBackgroundOptimization();