Skip to content

Commit 879fdc4

Browse files
bakkotrichardlau
bakkot
authored and
richardlau
committed
deps: V8: backport da20a197a7f9
Original commit message: [builtins] Make Promise resolvers not keep resolved Promises alive Currently a Promise's resolve/reject functions unconditionally hold the Promise, plus a separate bit to track whether the Promise has been resolved. Instead, hold a single field with Promise|Undefined. This allows GC'ing a resolved Promise even if its resolvers are still alive. [email protected] Fixed: 42213031 Change-Id: Ice645dbabb79e63dfcac8ae843cad95439d7a1f1 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7646250 Reviewed-by: Darius Mercadier <[email protected]> Commit-Queue: Kevin Gibbons <[email protected]> Reviewed-by: Olivier Flückiger <[email protected]> Cr-Commit-Position: refs/heads/main@{#108183} Refs: v8/v8@da20a19 Co-authored-by: Kevin Gibbons <[email protected]> PR-URL: #64101 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
1 parent a640543 commit 879fdc4

8 files changed

Lines changed: 83 additions & 52 deletions

File tree

‎common.gypi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141

4242
# Reset this number to 0 on major V8 upgrades.
4343
# Increment by one for each non-official patch applied to deps/v8.
44-
'v8_embedder_string': '-node.23',
44+
'v8_embedder_string': '-node.24',
4545

4646
##### V8 defaults for Node.js #####
4747

‎deps/v8/src/builtins/builtins-promise.h‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,9 @@ namespace internal {
1313
class PromiseBuiltins {
1414
public:
1515
enum PromiseResolvingFunctionContextSlot {
16-
// The promise which resolve/reject callbacks fulfill.
17-
kPromiseSlot = Context::MIN_CONTEXT_SLOTS,
18-
19-
// Whether the callback was already invoked.
20-
kAlreadyResolvedSlot,
16+
// The promise which resolve/reject callbacks fulfill, or Undefined
17+
// if already resolved.
18+
kPromiseIfNotResolvedSlot = Context::MIN_CONTEXT_SLOTS,
2119

2220
// Whether to trigger a debug event or not. Used in catch
2321
// prediction.

‎deps/v8/src/builtins/promise-abstract-operations.tq‎

Lines changed: 38 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,8 @@ const kPromiseCapabilitySize:
265265
type PromiseResolvingFunctionContext extends FunctionContext;
266266
extern enum PromiseResolvingFunctionContextSlot extends intptr
267267
constexpr 'PromiseBuiltins::PromiseResolvingFunctionContextSlot' {
268-
kPromiseSlot: Slot<PromiseResolvingFunctionContext, JSPromise>,
269-
kAlreadyResolvedSlot: Slot<PromiseResolvingFunctionContext, Boolean>,
268+
kPromiseIfNotResolvedSlot:
269+
Slot<PromiseResolvingFunctionContext, JSPromise|Undefined>,
270270
kDebugEventSlot: Slot<PromiseResolvingFunctionContext, Boolean>,
271271
kPromiseContextLength
272272
}
@@ -390,25 +390,29 @@ transitioning builtin NewPromiseCapability(
390390
transitioning javascript builtin PromiseCapabilityDefaultReject(
391391
js-implicit context: Context, receiver: JSAny)(reason: JSAny): JSAny {
392392
const context = %RawDownCast<PromiseResolvingFunctionContext>(context);
393-
// 2. Let promise be F.[[Promise]].
394-
const promise =
395-
*ContextSlot(context, PromiseResolvingFunctionContextSlot::kPromiseSlot);
396-
397-
// 3. Let alreadyResolved be F.[[AlreadyResolved]].
398-
const alreadyResolved = *ContextSlot(
399-
context, PromiseResolvingFunctionContextSlot::kAlreadyResolvedSlot);
400-
401-
// 4. If alreadyResolved.[[Value]] is true, return undefined.
402-
if (alreadyResolved == True) {
403-
return Undefined;
393+
// 2. Let promise be promiseOrEmpty.[[Value]].
394+
const promiseOrEmpty =
395+
*ContextSlot(
396+
context, PromiseResolvingFunctionContextSlot::kPromiseIfNotResolvedSlot);
397+
398+
// 1. If promiseOrEmpty.[[Value]] is ~empty~, return undefined.
399+
let promise: JSPromise;
400+
typeswitch (promiseOrEmpty) {
401+
case (Undefined): {
402+
return Undefined;
403+
}
404+
case (p: JSPromise): {
405+
promise = p;
406+
}
404407
}
405408

406-
// 5. Set alreadyResolved.[[Value]] to true.
409+
// 3. Set promiseOrEmpty.[[Value]] to ~empty~.
407410
*ContextSlot(
408-
context, PromiseResolvingFunctionContextSlot::kAlreadyResolvedSlot) =
409-
True;
411+
context, PromiseResolvingFunctionContextSlot::kPromiseIfNotResolvedSlot) =
412+
Undefined;
410413

411-
// 6. Return RejectPromise(promise, reason).
414+
// 4. Perform RejectPromise(promise, reason).
415+
// 5. Return undefined.
412416
const debugEvent = *ContextSlot(
413417
context, PromiseResolvingFunctionContextSlot::kDebugEventSlot);
414418
return RejectPromise(promise, reason, debugEvent);
@@ -418,23 +422,26 @@ transitioning javascript builtin PromiseCapabilityDefaultReject(
418422
transitioning javascript builtin PromiseCapabilityDefaultResolve(
419423
js-implicit context: Context, receiver: JSAny)(resolution: JSAny): JSAny {
420424
const context = %RawDownCast<PromiseResolvingFunctionContext>(context);
421-
// 2. Let promise be F.[[Promise]].
422-
const promise: JSPromise =
423-
*ContextSlot(context, PromiseResolvingFunctionContextSlot::kPromiseSlot);
424-
425-
// 3. Let alreadyResolved be F.[[AlreadyResolved]].
426-
const alreadyResolved: Boolean = *ContextSlot(
427-
context, PromiseResolvingFunctionContextSlot::kAlreadyResolvedSlot);
428-
429-
// 4. If alreadyResolved.[[Value]] is true, return undefined.
430-
if (alreadyResolved == True) {
431-
return Undefined;
425+
// 2. Let promise be promiseOrEmpty.[[Value]].
426+
const promiseOrEmpty =
427+
*ContextSlot(
428+
context, PromiseResolvingFunctionContextSlot::kPromiseIfNotResolvedSlot);
429+
430+
// 1. If promiseOrEmpty.[[Value]] is ~empty~, return undefined.
431+
let promise: JSPromise;
432+
typeswitch (promiseOrEmpty) {
433+
case (Undefined): {
434+
return Undefined;
435+
}
436+
case (p: JSPromise): {
437+
promise = p;
438+
}
432439
}
433440

434-
// 5. Set alreadyResolved.[[Value]] to true.
441+
// 3. Set promiseOrEmpty.[[Value]] to ~empty~.
435442
*ContextSlot(
436-
context, PromiseResolvingFunctionContextSlot::kAlreadyResolvedSlot) =
437-
True;
443+
context, PromiseResolvingFunctionContextSlot::kPromiseIfNotResolvedSlot) =
444+
Undefined;
438445

439446
// The rest of the logic (and the catch prediction) is
440447
// encapsulated in the dedicated ResolvePromise builtin.

‎deps/v8/src/builtins/promise-all.tq‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,17 +63,14 @@ macro CreatePromiseResolvingFunctionsContext(
6363
nativeContext,
6464
PromiseResolvingFunctionContextSlot::kPromiseContextLength));
6565
InitContextSlot(
66-
resolveContext, PromiseResolvingFunctionContextSlot::kPromiseSlot,
67-
promise);
68-
InitContextSlot(
69-
resolveContext, PromiseResolvingFunctionContextSlot::kAlreadyResolvedSlot,
70-
False);
66+
resolveContext,
67+
PromiseResolvingFunctionContextSlot::kPromiseIfNotResolvedSlot, promise);
7168
InitContextSlot(
7269
resolveContext, PromiseResolvingFunctionContextSlot::kDebugEventSlot,
7370
debugEvent);
7471
static_assert(
7572
PromiseResolvingFunctionContextSlot::kPromiseContextLength ==
76-
ContextSlot::MIN_CONTEXT_SLOTS + 3);
73+
ContextSlot::MIN_CONTEXT_SLOTS + 2);
7774
return resolveContext;
7875
}
7976

‎deps/v8/src/compiler/js-call-reducer.cc‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2543,10 +2543,8 @@ TNode<Object> PromiseBuiltinReducerAssembler::ReducePromiseConstructor(
25432543
// Allocate a promise context for the closures below.
25442544
TNode<Context> promise_context = CreateFunctionContext(
25452545
native_context, context, PromiseBuiltins::kPromiseContextLength);
2546-
StoreContextNoCellSlot(promise_context, PromiseBuiltins::kPromiseSlot,
2547-
promise);
2548-
StoreContextNoCellSlot(promise_context, PromiseBuiltins::kAlreadyResolvedSlot,
2549-
FalseConstant());
2546+
StoreContextNoCellSlot(promise_context,
2547+
PromiseBuiltins::kPromiseIfNotResolvedSlot, promise);
25502548
StoreContextNoCellSlot(promise_context, PromiseBuiltins::kDebugEventSlot,
25512549
TrueConstant());
25522550

‎deps/v8/src/execution/isolate.cc‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,9 +1253,12 @@ void CaptureAsyncStackTrace(Isolate* isolate, DirectHandle<JSPromise> promise,
12531253
DirectHandle<JSFunction> function(
12541254
Cast<JSFunction>(reaction->fulfill_handler()), isolate);
12551255
DirectHandle<Context> context(function->context(), isolate);
1256-
promise = direct_handle(
1257-
Cast<JSPromise>(context->GetNoCell(PromiseBuiltins::kPromiseSlot)),
1258-
isolate);
1256+
Tagged<Object> promise_or_undefined =
1257+
context->GetNoCell(PromiseBuiltins::kPromiseIfNotResolvedSlot);
1258+
if (!TryCast(direct_handle(promise_or_undefined, isolate), &promise)) {
1259+
DCHECK(IsUndefined(promise_or_undefined));
1260+
return;
1261+
}
12591262
} else {
12601263
// We have some generic promise chain here, so try to
12611264
// continue with the chained promise on the reaction

‎deps/v8/test/cctest/test-code-stub-assembler.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3037,7 +3037,8 @@ TEST(CreatePromiseResolvingFunctionsContext) {
30373037
DirectHandle<Context> context_js = Cast<Context>(result);
30383038
CHECK_EQ(isolate->root(RootIndex::kEmptyScopeInfo), context_js->scope_info());
30393039
CHECK_EQ(*isolate->native_context(), context_js->native_context());
3040-
CHECK(IsJSPromise(context_js->GetNoCell(PromiseBuiltins::kPromiseSlot)));
3040+
CHECK(IsJSPromise(
3041+
context_js->GetNoCell(PromiseBuiltins::kPromiseIfNotResolvedSlot)));
30413042
CHECK_EQ(ReadOnlyRoots(isolate).false_value(),
30423043
context_js->GetNoCell(PromiseBuiltins::kDebugEventSlot));
30433044
}
@@ -3246,7 +3247,8 @@ TEST(NewPromiseCapability) {
32463247
CHECK_EQ(*isolate->native_context(), callback_context->native_context());
32473248
CHECK_EQ(PromiseBuiltins::kPromiseContextLength,
32483249
callback_context->length());
3249-
CHECK_EQ(callback_context->GetNoCell(PromiseBuiltins::kPromiseSlot),
3250+
CHECK_EQ(callback_context->GetNoCell(
3251+
PromiseBuiltins::kPromiseIfNotResolvedSlot),
32503252
result->promise());
32513253
}
32523254
}

‎deps/v8/test/mjsunit/regress/regress-crbug-42213031.js‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Copyright 2026 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// Flags: --expose-gc
6+
7+
const pending = new Promise(() => {});
8+
9+
(async function () {
10+
let wr;
11+
12+
await (async function () {
13+
const payload = { };
14+
wr = new WeakRef(payload);
15+
const resolved = Promise.resolve(payload);
16+
// The pending Promise should not prevent GC of the race Promise once the race settles.
17+
await Promise.race([pending, resolved]);
18+
})();
19+
20+
await gc({ type: 'major', execution: 'async' });
21+
22+
assertEquals(undefined, wr.deref());
23+
})().catch((e) => {
24+
console.error(e);
25+
quit(1);
26+
});

0 commit comments

Comments
 (0)