From 5d91cd2961f8eb284edf3972b38501997fd60025 Mon Sep 17 00:00:00 2001
From: Igor Sheludko <[email protected]>
Date: Tue, 19 May 2026 00:35:02 +0200
Subject: [PATCH] [promise] Fix double-settlement issue in v8::Promise::Resolver

This CL adds JSPromise::is_native_resolver_invoked flag as an anti
double-settlement measure for native promises. This flag is never
used for JavaScript promises.

This is a simpler version of "promiseOrEmpty.[[Value]] is EMPTY"
related steps from the spec:
  https://tc39.es/ecma262/#sec-createresolvingfunctions
Native promises do not require the full `promiseOrEmpty` capturing
machinery because their Api is limited and does not allow providing
custom executor for native promises.

Drive-by:
 - cleanup definition of JSPromise flags,
 - cleanup Api tests by moving GetData/MakeData helpers to test-api.h.

Fixes: 513289241

TAG=agy
CONV=d40e1b59-f716-4800-9f72-429924a60d7f

Change-Id: Icdff40c58aa7dfdc330e53b6ff8e2e2e1f6619d9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7857889
Reviewed-by: Leszek Swirski <[email protected]>
Commit-Queue: Igor Sheludko <[email protected]>
Cr-Commit-Position: refs/heads/main@{#107410}
---

diff --git a/src/api/api.cc b/src/api/api.cc
index 337a4a4..7bb0535 100644
--- a/src/api/api.cc
+++ b/src/api/api.cc
@@ -8643,9 +8643,18 @@
   auto self = Utils::OpenDirectHandle(this);
   auto promise = i::Cast<i::JSPromise>(self);
 
-  if (promise->status() != Promise::kPending) {
+  // Protect against double settlement. This is a simpler version of
+  // "promiseOrEmpty.[[Value]] is EMPTY" related steps from the spec:
+  // https://tc39.es/ecma262/#sec-createresolvingfunctions
+  // Native promises do not require the full `promiseOrEmpty` capturing
+  // dance because Api is limited and does not allow providing custom
+  // executor for native promises.
+  DCHECK_IMPLIES(promise->status() != Promise::kPending,
+                 promise->is_native_resolver_invoked());
+  if (promise->is_native_resolver_invoked()) {
     return Just(true);
   }
+  promise->set_is_native_resolver_invoked(true);
 
   // TODO(434637746): Drop this CHECK when the bug is fixed.
   CHECK(!value.IsEmpty());
@@ -8665,9 +8674,18 @@
   auto self = Utils::OpenDirectHandle(this);
   auto promise = i::Cast<i::JSPromise>(self);
 
-  if (promise->status() != Promise::kPending) {
+  // Protect against double settlement. This is a simpler version of
+  // "promiseOrEmpty.[[Value]] is EMPTY" related steps from the spec:
+  // https://tc39.es/ecma262/#sec-createresolvingfunctions
+  // Native promises do not require the full `promiseOrEmpty` capturing
+  // dance because Api is limited and does not allow providing custom
+  // executor for native promises.
+  DCHECK_IMPLIES(promise->status() != Promise::kPending,
+                 promise->is_native_resolver_invoked());
+  if (promise->is_native_resolver_invoked()) {
     return Just(true);
   }
+  promise->set_is_native_resolver_invoked(true);
 
   if (i::JSPromise::Reject(promise, Utils::OpenDirectHandle(*value))
           .is_null()) {
diff --git a/src/builtins/promise-misc.tq b/src/builtins/promise-misc.tq
index f83ee77..f82a7c2 100644
--- a/src/builtins/promise-misc.tq
+++ b/src/builtins/promise-misc.tq
@@ -49,6 +49,7 @@
   promise.reactions_or_result = kZero;
   promise.flags = SmiTag(JSPromiseFlags{
     status: PromiseState::kPending,
+    is_native_resolver_invoked: false,
     has_handler: false,
     is_silent: false,
     async_task_id: kInvalidAsyncTaskId
@@ -68,6 +69,7 @@
   promise.reactions_or_result = kZero;
   promise.flags = SmiTag(JSPromiseFlags{
     status: PromiseState::kPending,
+    is_native_resolver_invoked: false,
     has_handler: false,
     is_silent: false,
     async_task_id: kInvalidAsyncTaskId
diff --git a/src/objects/js-promise-inl.h b/src/objects/js-promise-inl.h
index 21dfbff..a15f688 100644
--- a/src/objects/js-promise-inl.h
+++ b/src/objects/js-promise-inl.h
@@ -33,6 +33,8 @@
   flags_.store(this, Smi::From31BitPattern(value));
 }
 
+BOOL_ACCESSORS(JSPromise, flags, is_native_resolver_invoked,
+               IsNativeResolverInvokedBit::kShift)
 BOOL_ACCESSORS(JSPromise, flags, has_handler, HasHandlerBit::kShift)
 BOOL_ACCESSORS(JSPromise, flags, is_silent, IsSilentBit::kShift)
 
diff --git a/src/objects/js-promise.h b/src/objects/js-promise.h
index 19e3f89..9e3f79f 100644
--- a/src/objects/js-promise.h
+++ b/src/objects/js-promise.h
@@ -10,6 +10,7 @@
 #include "src/handles/handles.h"
 #include "src/objects/js-objects.h"
 #include "src/objects/promise.h"
+#include "torque-generated/bit-fields.h"
 
 // Has to be the last include (doesn't have include guards):
 #include "src/objects/object-macros.h"
@@ -65,6 +66,14 @@
   V8_EXPORT_PRIVATE Promise::PromiseState status() const;
   void set_status(Promise::PromiseState status);
 
+  // [is_native_resolver_invoked]: This is a part of simpler anti double
+  // settlement mechanism for native promises unlike the "promiseOrEmpty"
+  // machinery required by the spec for JavaScript promises.
+  // https://tc39.es/ecma262/#sec-createresolvingfunctions
+  // Returns true if v8::Resolver::Resolve() or Reject() was called for this
+  // promise.
+  DECL_BOOLEAN_ACCESSORS(is_native_resolver_invoked)
+
   // https://tc39.es/ecma262/#sec-fulfillpromise
   V8_EXPORT_PRIVATE static Handle<Object> Fulfill(
       DirectHandle<JSPromise> promise, DirectHandle<Object> value);
@@ -92,10 +101,7 @@
   DECL_VERIFIER(JSPromise)
 
   // Flags layout.
-  using StatusBits = base::BitField<Promise::PromiseState, 0, 2>;
-  using HasHandlerBit = base::BitField<bool, 2, 1>;
-  using IsSilentBit = base::BitField<bool, 3, 1>;
-  using AsyncTaskIdBits = base::BitField<uint32_t, 4, 27>;
+  DEFINE_TORQUE_GENERATED_JS_PROMISE_FLAGS()
 
   static_assert(v8::Promise::kPending == 0);
   static_assert(v8::Promise::kFulfilled == 1);
diff --git a/src/objects/js-promise.tq b/src/objects/js-promise.tq
index 11c4aff..6ea2059e 100644
--- a/src/objects/js-promise.tq
+++ b/src/objects/js-promise.tq
@@ -4,9 +4,10 @@
 
 bitfield struct JSPromiseFlags extends uint31 {
   status: PromiseState: 2 bit;
+  is_native_resolver_invoked: bool: 1 bit;
   has_handler: bool: 1 bit;
   is_silent: bool: 1 bit;
-  async_task_id: uint32: 27 bit;
+  async_task_id: uint32: 26 bit;
 }
 
 @cppObjectLayoutDefinition
diff --git a/test/cctest/test-api-interceptors.cc b/test/cctest/test-api-interceptors.cc
index 46e05bf..bd69f2b 100644
--- a/test/cctest/test-api-interceptors.cc
+++ b/test/cctest/test-api-interceptors.cc
@@ -32,20 +32,6 @@
 
 namespace {
 
-// This tag value has been picked arbitrarily between 0 and
-// V8_EXTERNAL_POINTER_TAG_COUNT.
-constexpr v8::ExternalPointerTypeTag kTestConfigTag = 14;
-
-template <typename T, typename U>
-T* GetData(const v8::PropertyCallbackInfo<U>& info) {
-  return reinterpret_cast<T*>(
-      v8::External::Cast(*info.Data())->Value(kTestConfigTag));
-}
-
-v8::Local<v8::External> MakeData(v8::Isolate* isolate, void* pointer) {
-  return v8::External::New(isolate, pointer, kTestConfigTag);
-}
-
 constexpr v8::EmbedderDataTypeTag kApiInterceptorTag = 1;
 
 void Returns42(const v8::FunctionCallbackInfo<v8::Value>& info) {
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index e202818..d255ae9 100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -23513,6 +23513,117 @@
                   .FromJust());
 }
 
+TEST(PromiseResolverIsNativeResolverInvokedTest) {
+  LocalContext context;
+  v8::Isolate* isolate = context.isolate();