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

Reply via email to