Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 4c00d7bcee9551888f474a7f1b1841312a71cc38
https://github.com/WebKit/WebKit/commit/4c00d7bcee9551888f474a7f1b1841312a71cc38
Author: Vassili Bykov <[email protected]>
Date: 2026-04-28 (Tue, 28 Apr 2026)
Changed paths:
A JSTests/wasm/stress/jspi-gc-abandoned-suspensions.js
M Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
M Source/JavaScriptCore/Sources.txt
M Source/JavaScriptCore/heap/Heap.cpp
M Source/JavaScriptCore/heap/RootMarkReason.h
M Source/JavaScriptCore/runtime/PinballCompletion.cpp
M Source/JavaScriptCore/runtime/PinballCompletion.h
A Source/JavaScriptCore/runtime/PinballHandlerContext.cpp
M Source/JavaScriptCore/runtime/PinballHandlerContext.h
M Source/JavaScriptCore/runtime/TemporalPlainYearMonth.cpp
M Source/JavaScriptCore/runtime/TemporalPlainYearMonth.h
M Source/JavaScriptCore/runtime/VM.cpp
M Source/JavaScriptCore/runtime/VM.h
Log Message:
-----------
[JSC][JSPI] Scan PinballCompletion conservative roots via a GC marking
constraint
https://bugs.webkit.org/show_bug.cgi?id=307564
rdar://170156450
Reviewed by Keith Miller.
In the existing JSPI implementation, evacuated Wasm stack slices and callee
saves are
registered with the VM and scanned as conservative roots. This creates a
potential memory
leak. Here is the reference structure:
Promise --> PinballCompletion --> EvacuatedStacks, Callee Saves --> ...
^
|
VM
The Promise is the promise that caused JSPI suspension, and it owns
(transitively via
resolution handlers) the PinballCompletion with the suspended computation.
Normally, when
the promise is fulfilled or rejected, PinballCompletion unregisters the
evacuated stacks
and callee saves it owns, and all these objects become garbage-collectable.
Also, if the
promise is forgotten without being resolved and becomes unmarked together with
PinballCompletion, PinballCompletion destructor unregisters the evacuated
stacks and
callee saves, so everything is collected. However, if there happens to be a
reference
chain starting in evacuated stacks or callee saves and leading back to the
promise, this
transitive strong reference from the VM conservative roots will keep the
promise and all
the other objects involved alive forever.
This patch prevents such a scenario. It stops treating evacuated stacks and
callee saves
as VM conservative roots. Instead, a new GC constraint is introduced for
PinballCompletion
objects, to scan the evacuated stacks and callee saves owned by a completion for
conservative roots only when the completion itself is marked.
This change requires some care in the implantation of evacuated stack slices.
In the
original implementation the top slice is removed from a PinballCompletion and
stored in
PinballHandlerContext by pinballHandlerInitContext(). Control would then return
to
assembly code which would prepare the stack space for the slice and then call
pinballHandlerImplantSlice() to copy the slice data onto the stack. This
creates a time
window when a stack slice has been taken out of its owner PinballCompletion,
but its
contents have not yet been copied onto the stack. If GC were to run in this
window, the
slice would not be scanned for conservative roots. This is not an immediate
problem
because currently we do nothing in that time window that could start a GC, but
it is a
fragility in case the code is changed in the future.
To avoid this fragility, we change the logic so that instead of storing the
slice to
implant in PinballHandlerContext, we store the pinball object itself, and the
slice is
only taken out of a PinballCompletion immediately before being implanted. We
also refactor
the initialization code to make it an initializer of the PinballHandlerContext
struct
instead of a static helper function.
Unified sources fixes
---------------------
The change in unified sources order with this patch, combined with the revision
of JSC
header includes by https://bugs.webkit.org/show_bug.cgi?id=312585 caused build
breakage.
The patch includes these additional changes unrelated to its core logic to fix
the
breakage:
- TemporalPlainYearMonth.h / .cpp: Moved the addDurationToYearMonth template
definition
from the header into the .cpp file and added explicit template instantiations
for
AddOrSubtract::Add and AddOrSubtract::Subtract. The header only had a forward
declaration of JSGlobalObject, so the globalObject->vm() call on line 91
failed with an
incomplete type error.
- WeakMapImplInlines.h: Added #include "Symbol.h". The canBeHeldWeakly function
calls
asSymbol() which is declared in Symbol.h, but the header only had a forward
declaration
of Symbol via the include chain.
- PinballHandlerContext.cpp: Added #include "StackAlignment.h". The constructor
asserts
!(sliceByteSize % stackAlignmentBytes()), but stackAlignmentBytes() is
declared in
StackAlignment.h which wasn't included directly.
Testing
-------
- Added `jspi-gc-abandoned-suspensions.js` to reproduce the memory leak
scenario described
above.
- Refactoring changes are covered by existing tests.
Canonical link: https://commits.webkit.org/312253@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications