Skip to content

JSC Slotvisitor::drain crash #1622

Open
Open
JSC Slotvisitor::drain crash#1622
Assignees
aoikonomopoulosmagomez
Labels
wpe-2.38

Description

@modeveci
modeveci
opened on Feb 24, 2026

Sometimes with 2.38, we are observing crashes around this call stack, to be able to address we need to know whether changes around this call stack between versions 2.46-2.38 can improve these occurrences.

JSC::SlotVisitor::drainFromShared
WTF::ParallelHelperClient::runTask
WTF::ParallelHelperPool::Thread::work
call
WTF::Thread::entryPoint
wtfThreadEntryPoint

It is not easy to reproduce but we want to understand whether these changes on GC can improve and prevent this crash. In particular, whether there is difference regarding a race condition in GC between marking and sweeping.

wpe-2.38

WPEWebKit/Source/JavaScriptCore/heap/MarkedBlock.cpp

Line 196 in a78b2a8

void MarkedBlock::aboutToMarkSlow(HeapVersion markingVersion)

wpe-2.46

WPEWebKit/Source/JavaScriptCore/heap/MarkedBlock.cpp

Line 251 in 99499dc

void MarkedBlock::aboutToMarkSlow(HeapVersion markingVersion, HeapCell* cell)

Activity

  1. modeveci
    added
    wpe-2.38
    on Feb 24, 2026
  2. modeveci
    assigned
    magomez
    on Feb 24, 2026

  3. modeveci commented on Feb 24, 2026

    @modeveci
    modeveci
    on Feb 24, 2026
    Author

    @magomez , can Justin look at this ticket? Thank you!

  4. magomez commented on Feb 24, 2026

    @magomez
    magomez
    on Feb 24, 2026

    @magomez , can Justin look at this ticket? Thank you!

    Sure, I'll ask him

  5. modeveci
    assigned
    aoikonomopoulos
    on Feb 25, 2026

  6. aoikonomopoulos commented on Feb 26, 2026

    @aoikonomopoulos
    aoikonomopoulos
    on Feb 26, 2026

    @modeveci The differences in aboutToMarkSlow come from https://commits.webkit.org/284376@main, later modified by https://commits.webkit.org/286689@main.

    Both of those changes add code to dump the state of a MarkedBlock when a handle is unexpectedly NULL. If that's where you've been observing the crash in 2.38 then these changes won't fix it; they can only produce some extra diagnostic output that may help track down the source.

    That said, if the problem you're running into is the NULL handle, collecting the diagnostic output in the rare occurrences when this does happen, would help track down this bug.

  7. GlebNovodranPE commented on Feb 26, 2026

    @GlebNovodranPE
    GlebNovodranPE
    on Feb 26, 2026

    @aoikonomopoulos
    FYI. Here is the more detailed stack trace:

    JSC::MarkedBlock::aboutToMarkSlow
    /usr/src/debug/lib32-wpe-webkit/2.38.8+gitAUTOINC+680d0afd32-r7/build/../git/Source/JavaScriptCore/heap/MarkedBlock.h:469

    JSC::Structure::visitChildren
    /usr/src/debug/lib32-wpe-webkit/2.38.8+gitAUTOINC+680d0afd32-r7/build/../git/Source/JavaScriptCore/heap/MarkedBlock.h:576

    operator
    /usr/src/debug/lib32-wpe-webkit/2.38.8+gitAUTOINC+680d0afd32-r7/build/../git/Source/JavaScriptCore/runtime/ClassInfo.h:118

    JSC::SlotVisitor::drain
    /usr/src/debug/lib32-wpe-webkit/2.38.8+gitAUTOINC+680d0afd32-r7/build/../git/Source/JavaScriptCore/heap/SlotVisitorInlines.h:184

    JSC::SlotVisitor::drainFromShared
    /usr/src/debug/lib32-wpe-webkit/2.38.8+gitAUTOINC+680d0afd32-r7/build/../git/Source/JavaScriptCore/heap/SlotVisitor.cpp:694

    operator
    /usr/src/debug/lib32-wpe-webkit/2.38.8+gitAUTOINC+680d0afd32-r7/build/../git/Source/JavaScriptCore/heap/Heap.cpp:1393

    WTF::ParallelHelperClient::runTask
    /usr/src/debug/lib32-wpe-webkit/2.38.8+gitAUTOINC+680d0afd32-r7/build/../git/Source/WTF/wtf/ParallelHelperPool.cpp:110

    WTF::ParallelHelperPool::Thread::work
    /usr/src/debug/lib32-wpe-webkit/2.38.8+gitAUTOINC+680d0afd32-r7/build/../git/Source/WTF/wtf/ParallelHelperPool.cpp:201

    call
    /usr/src/debug/lib32-wpe-webkit/2.38.8+gitAUTOINC+680d0afd32-r7/build/../git/Source/WTF/wtf/AutomaticThread.cpp:229

    WTF::Thread::entryPoint
    /usr/src/debug/lib32-wpe-webkit/2.38.8+gitAUTOINC+680d0afd32-r7/build/../git/Source/WTF/wtf/Function.h:82

    wtfThreadEntryPoint
    /usr/src/debug/lib32-wpe-webkit/2.38.8+gitAUTOINC+680d0afd32-r7/build/../git/Source/WTF/wtf/posix/ThreadingPOSIX.cpp:242

    start_thread
    /usr/src/debug/lib32-glibc/2.35-r0/git/nptl/pthread_create.c:442

    clone
    :

  8. GlebNovodranPE commented on Feb 26, 2026

    @GlebNovodranPE
    GlebNovodranPE
    on Feb 26, 2026

    It looks like it crashes upon dereferencing a null pointer:
    BlockDirectory* directory = handle().directory();

    WPEWebKit/Source/JavaScriptCore/heap/MarkedBlock.h

    Line 469 in 7609a21

    return m_directory;

    Looks like the wpe-2.46 adds the check and dump of the additional info.

    WPEWebKit/Source/JavaScriptCore/heap/MarkedBlock.cpp

    Line 262 in 9e67942

    if (UNLIKELY(!handle))

    This probably means that the problem per se was not solved, but acknowledged in the upstream.

  9. aoikonomopoulos commented on Feb 27, 2026

    @aoikonomopoulos
    aoikonomopoulos
    on Feb 27, 2026

    @GlebNovodranPE Thanks. That is exactly the issue that the new instrumentation code is trying to catch.

  10. modeveci commented on Feb 27, 2026

    @modeveci
    modeveci
    on Feb 27, 2026
    Author

    Thank you both @aoikonomopoulos and @GlebNovodranPE.

    @aoikonomopoulos do you think it is useful to backport those changes to 2/38 and try to catch crashes with this new instrumental impl.?

  11. aoikonomopoulos commented on Feb 27, 2026

    @aoikonomopoulos
    aoikonomopoulos
    on Feb 27, 2026

    Yes, but for it to be useful we need to make sure the diagnostic output gets picked up by your telemetry. Currently, nothing is logged on non-Cocoa platforms.

  12. justinmichaud commented on Mar 3, 2026

    @justinmichaud
    justinmichaud
    on Mar 3, 2026

    This looks like it is going to be hard to debug.

    Just confirming:

    1. This reproduces on 2.38
    2. This reproduces on 2.46, albeit with a slightly different signature

    Can we focus on testing on 2.46?

    BlockDirectory* directory = handle().directory();

    I will propose some release asserts to add to rule out some things.

    Then:

    1. Let's start by testing on 2.46 and turning on this logging for all platforms (@aoikonomopoulos has already started a thread about that).

    2. Do you have a history of the crash rate? Was there a 2.38 release that did not have this issue? Maybe a recent patch introduced this, and we can bisect.

    3. I will try to reproduce this on my end. This will probably be hard / take a while, I will privately ping you for more details.

    Next, what happens to the repro rate under the following situations (on 2.46)? I can test these myself if I can reproduce it.

    First, let me confirm we are on the latest 2.46 with no additional JSC options being set. Please let me know if any default options have been changed.

    1. collectContinuously=1 (might make the crash happen much sooner)
    2. useZombieMode=1 (this will tell us if we have a race or a uaf)
    3. useConcurrentGC=0
    4. useGenerationalGC=0
    5. useConcurrentJIT=0
    6. forceCodeBlockLiveness=1 (this will tell us if we have a code lifecycle issue)
    7. arm64 wpe (this will improve testing rate, since I can test on my workstation)
    8. if it reproduces on arm64 wpe, then I can try to repro on arm64 ToT. That would rule out missing backported patches

    The more of these options we can turn on while still crashing, the easier it will be to debug.

  13. justinmichaud commented on Mar 3, 2026

    @justinmichaud
    justinmichaud
    on Mar 3, 2026

    The line numbers in your symbolicated stack trace don't match up with the symbol names. For example, JSC::Structure::visitChildren is not MarkedBlock.h:576.

    Nothing quite lines up (maybe due to optimization), but I have made a few guesses to make things make sense. We need to double check the back trace though.

    We know (based on what you said above):

    MarkedBlock::Handle* handle = header().handlePointerForNullCheck();

    handle is 0 (please confirm that the trap value is 0, not just unmapped)

    The path to get here:

    SlotVisitor::drain
forEachMarkStack callback body, m_collectorStack
visitChildren()
SlotVisitor::visitChildren
(via cell->methodTable()->visitChildren) MethodTable::visitChildren
(via a pointer in the method table) JSC::Structure::visitChildren
<speculation>
visitor.append, appendUnbarriered (it would be so freaking helpful to know which one)
aboutToMark
</speculation>
MarkedBlock::aboutToMarkSlow

    This cell was added to the collector stack by:

    some visit children function || 
appendSlow | appendHiddenSlow ||
SlotVisitor::appendHiddenSlowImpl || SlotVisitor::append(ConservativeRoots)
setMarkedAndAppendToMarkStack || appendJSCellOrAuxiliary
SlotVisitor::appendToMarkStack

    Then:

    1. header() is garbage

    cell is garbage, interpreted as a structure. Then, cell->markedBlock() (which is just masking off some bits of cell) is garbage.

    ENABLE_GC_VALIDATION might be able to help us trace down where/why.

    If we can collect many crashes, but every time we see a different visitChildren method being called, this case seems much more likely.

    1. header() is valid, header().m_handle is 0

    Then either:

    a) Handle::didRemoveFromDirectory was called

    Then BlockDirectory::removeBlock(WillDeleteBlock::No) was called, so Handle::removeFromDirectory was called, so tryAllocateWithoutCollecting (with Options::stealEmptyBlocksFromOtherAllocators()) was called).

    This gives us a super easy test: disable JSC_stealEmptyBlocksFromOtherAllocators!

    b) Handle::didAddToDirectory got null

    There is already a RELEASE_ASSERT that would fail if that was the case, but maybe it got compiled out. I'll add more asserts here just in case, but this seems unlikely.

    1. Do we only ever see Structure::visitChildren?

    2. Does this crash reproduce with JSC_stealEmptyBlocksFromOtherAllocators=0?

    The answer to this can already rule out a bunch of options today, without needing a new build.

  14. GlebNovodranPE commented on Mar 4, 2026

    @GlebNovodranPE
    GlebNovodranPE
    on Mar 4, 2026
    1. Let's start by testing on 2.46 and turning on this logging for all platforms ([@aoikonomopoulos](https://github.com/aoikonomopoulos) has already started a thread about that).

    @justinmichaud , @modeveci ,
    We do not have 2.46 based devices in the field at this point.

    2. Do you have a history of the crash rate? Was there a 2.38 release that did not have this issue? Maybe a recent patch introduced this, and we can bisect.

    The crash is a long standing issue it was present even before 2.38 I think, with a somewhat different stack trace.

    3. I will try to reproduce this on my end. This will probably be hard / take a while, I will privately ping you for more details.

    I am not sure if attempting to reproduce it locally is not going to be a waste of time. Would you please talk to @modeveci regarding the reproduction approaches discussed with him.

    Next, what happens to the repro rate under the following situations (on 2.46)? I can test these myself if I can reproduce it.

    As I mentioned previously it is impossible to conclude anything about reproducibility in the field with 2.46.

    First, let me confirm we are on the latest 2.46 with no additional JSC options being set. Please let me know if any default options have been changed.

    1. collectContinuously=1 (might make the crash happen much sooner)

2. useZombieMode=1 (this will tell us if we have a race or a uaf)

3. useConcurrentGC=0

4. useGenerationalGC=0

5. useConcurrentJIT=0

6. forceCodeBlockLiveness=1 (this will tell us if we have a code lifecycle issue)

7. arm64 wpe (this will improve testing rate, since I can test on my workstation)

8. if it reproduces on arm64 wpe, then I can try to repro on arm64 ToT. That would rule out missing backported patches

    I don't think anything above was tweaked, but let me clarify this with the colleagues.

  15. GlebNovodranPE commented on Mar 4, 2026

    @GlebNovodranPE
    GlebNovodranPE
    on Mar 4, 2026

    @justinmichaud ,

    handle is 0 (please confirm that the trap value is 0, not just unmapped

    The crash information we have comes from Breakpad minidumps and the binaries upon which the dumps were created are stripped, so it is difficult to conclude about the local variable values.

    Do we only ever see Structure::visitChildren?

    I would say it constitutes a bigger part of SEGV crashes in JSC::MarkedBlock::aboutToMarkSlow we observe. I am attaching sample staktraces for the cases when it dos not originate from Structure::visitChildren.

    JSFinalObject.txt
    UnlinkedCodeBlock.txt
    visitAggregate.txt

    Does this crash reproduce with JSC_stealEmptyBlocksFromOtherAllocators=0?

    We don't know at the moment and it would be hard to conclude if it helps, since it is difficult to reproduce it in a lab even without that tweak. All the data we have come from the filed where it is enabled by default.

  16. GlebNovodranPE commented on Mar 4, 2026

    @GlebNovodranPE
    GlebNovodranPE
    on Mar 4, 2026

    First, let me confirm we are on the latest 2.46 with no additional JSC options being set. Please let me know if any default options have been changed.

    1. collectContinuously=1 (might make the crash happen much sooner)

2. useZombieMode=1 (this will tell us if we have a race or a uaf)

3. useConcurrentGC=0

4. useGenerationalGC=0

5. useConcurrentJIT=0

6. forceCodeBlockLiveness=1 (this will tell us if we have a code lifecycle issue)

7. arm64 wpe (this will improve testing rate, since I can test on my workstation)

8. if it reproduces on arm64 wpe, then I can try to repro on arm64 ToT. That would rule out missing backported patches

    Let me clarify it, We are NOT on the latest 2.46. The crash comes from the filed devices running 2.38.
    What we aim to fix/understand are the two interrelated tasks :

    1. Understand if it still happens in 2.46 .
    2. If yes - fix it in both 2.46 and 2.38 or, if it is fixed in 2.46, back port the existing fix to 2.38.

  17. GlebNovodranPE commented on Mar 4, 2026

    @GlebNovodranPE
    GlebNovodranPE
    on Mar 4, 2026

    1. collectContinuously=1 (might make the crash happen much sooner)

    2. useZombieMode=1 (this will tell us if we have a race or a uaf)

    3. useConcurrentGC=0

    4. useGenerationalGC=0

    5. useConcurrentJIT=0

    6. forceCodeBlockLiveness=1 (this will tell us if we have a code lifecycle issue)

    All those options in our build have the opposite (default) values. Completely like in
    https://github.com/WebPlatformForEmbedded/WPEWebKit/blob/wpe-2.38/Source/JavaScriptCore/runtime/OptionsList.h

  18. justinmichaud commented on Mar 4, 2026

    @justinmichaud
    justinmichaud
    on Mar 4, 2026

    All those options in our build have the opposite (default) values. Completely like in https://github.com/WebPlatformForEmbedded/WPEWebKit/blob/wpe-2.38/Source/JavaScriptCore/runtime/OptionsList.h

    I was confused, I thought this could be reproduced in the lab. In that case, this list of options would help to narrow down the potential cause, but these options aren't suitable for production. As long as your environment variables in production match https://github.com/WebPlatformForEmbedded/WPEWebKit/blob/wpe-2.38/Source/JavaScriptCore/runtime/OptionsList.h, we should be fine (although I thought some were different).

    We can continue this discussion in teams

  19. jnorthrup commented on Mar 20, 2026

    @jnorthrup
    jnorthrup
    on Mar 20, 2026

    Additional Evidence: Memory Leaks (Not Just Crashes)

    We've identified the same SlotVisitor::drainFromShared issue causing memory leaks (not just crashes) in Bun runtime.

    Our Findings

    4 unique JSC GC leaks identified via ASAN/LSAN:

    1. WTF::HashTable::removeIterator - 7,214 frames
    2. WTF::Vector<JSC::InByVariant>::operator= - 7,220 frames
    3. JSC::SlotVisitor::drainFromShared - 7,214 frames
    4. JSC::ASTBuilder::createForOfLoop - 713 frames

    Stack Trace (Leak #3 - SlotVisitor)

    #0 WTF::removeIterator<WTF::HashTable<JSC::JSGlobalObject*, ...>>
#10 JSC::SlotVisitor::drainFromShared
#11 JSC::Heap::runBeginPhase
#12 WTF::ParallelHelperClient::runTask
#13 WTF::ParallelHelperPool::Thread::work
... 7,214 frames

    Related Issues

    Impact

    • Memory growth ~1GB per 30 seconds during heavy GC
    • Affects all WebKit-based runtimes (Bun, Safari, React Native)
    • macOS AArch64 confirmed, likely all platforms

    Full analysis: https://github.com/oven-sh/bun/tree/main/test/ISSUE_WEBKIT_GC_LEAKS.md

    This appears to be a race condition in parallel GC marking phase.

  20. justinmichaud commented on Mar 21, 2026

    @justinmichaud
    justinmichaud
    on Mar 21, 2026

    This appears to be a race condition in parallel GC marking phase.

    This issue is affecting 32-bit, which cannot do concurrent gc. https://github.com/oven-sh/bun/tree/main/test/ISSUE_WEBKIT_GC_LEAKS.md is a 404, so I can't tell if you actually really do mean that it is a race in the parallel (but not concurrent) marking phase.

    How are you measuring that there is a leak? My guess is that ASAN is just wrong, this doesn't really make sense to me.

    I'd be curious to read your report when it doesn't 404, but I think these issues are extraordinarily unlikely to be related.

  21. 190n commented on Mar 21, 2026

    @190n
    190n
    on Mar 21, 2026

    @jnorthrup appears to be spreading LLM output across anything that seems related to their investigation. See all the issues that oven-sh/bun#28343 has been mentioned in, as well as their comments at anomalyco/opencode#17837 You can find ISSUE_WEBKIT_GC_LEAKS.md on their Bun fork.

  22. justinmichaud commented on Mar 21, 2026

    @justinmichaud
    justinmichaud
    on Mar 21, 2026

    That’s what I suspected, I confirmed with bun. What a useless waste of time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

wpe-2.38

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions