Skip to content

Commit d6d27c0

Browse files
hyjorc1hyjorc1
committed
[JSC][WASM][Debugger] Fix WASM debugger stop-the-world races during VM construction and destruction
https://bugs.webkit.org/show_bug.cgi?id=310328 rdar://172974905 Reviewed by Mark Lam. Fix crashes and assertion failures in the Wasm debugger's stop-the-world (STW) machinery when VMs are created or destroyed concurrently with a debugger interrupt. - Move m_numberOfStoppedVMs and m_numberOfActiveVMs from Atomic<unsigned> to plain unsigned WTF_GUARDED_BY_LOCK(m_worldLock), and consolidate all counter updates and debugState()->setStopped()/clearStop() into the same lock acquisitions as the mode checks, eliminating races where the counters could be observed in an inconsistent state across separate lock acquisitions. - Remove vm.requestStop() from notifyVMConstruction and notifyVMActivation. requestStop() installs a VMTrap to indirectly drive notifyVMStop(), but both functions already call notifyVMStop() directly, making the requestStop() redundant. - Separate VMManager registration from VMThreadContext lifecycle. Call notifyVMConstruction() explicitly at the end of VM::VM() after all VM state is initialized, so any STW triggered immediately on registration sees a fully constructed VM. Call notifyVMDestruction() explicitly at the start of VM::~VM() before any teardown. VMThreadContext is now a pure data holder with a default constructor and destructor. - Fix VMTraps::handleTraps() to tolerate a NoEvent result from takeTopPriorityTrap() when cancelStop() races with trap dispatch. Tests: JSC::Wasm::testExecutionHandlerVMLifecycle Canonical link: https://commits.webkit.org/309965@main
1 parent aa5eadc commit d6d27c0

13 files changed

Lines changed: 406 additions & 92 deletions

File tree

‎Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1952,6 +1952,8 @@
19521952
DE26E9031CB5DD0500D2BE82 /* BuiltinExecutableCreator.h in Headers */ = {isa = PBXBuildFile; fileRef = DE26E9021CB5DD0500D2BE82 /* BuiltinExecutableCreator.h */; };
19531953
DEA7E2451BBC677F00D78440 /* JSTypedArrayViewPrototype.h in Headers */ = {isa = PBXBuildFile; fileRef = 53917E7C1B791106000EBD33 /* JSTypedArrayViewPrototype.h */; settings = {ATTRIBUTES = (Private, ); }; };
19541954
E124A8F70E555775003091F1 /* OpaqueJSString.h in Headers */ = {isa = PBXBuildFile; fileRef = E124A8F50E555775003091F1 /* OpaqueJSString.h */; settings = {ATTRIBUTES = (Private, ); }; };
1955+
E30001002EF0001000000004 /* MicrotaskCall.h in Headers */ = {isa = PBXBuildFile; fileRef = E30001002EF0001000000001 /* MicrotaskCall.h */; settings = {ATTRIBUTES = (Private, ); }; };
1956+
E30001002EF0001000000005 /* MicrotaskCallInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = E30001002EF0001000000002 /* MicrotaskCallInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
19551957
E306C1BC2D79000D0011D5AC /* MicrotaskQueueInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = E306C1BB2D7900050011D5AC /* MicrotaskQueueInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
19561958
E307178324C7827100DF0644 /* IntlRelativeTimeFormat.h in Headers */ = {isa = PBXBuildFile; fileRef = A3BF885024480BE1001B9F35 /* IntlRelativeTimeFormat.h */; };
19571959
E307178424C7827700DF0644 /* IntlRelativeTimeFormatConstructor.h in Headers */ = {isa = PBXBuildFile; fileRef = A3BF884E24480BE0001B9F35 /* IntlRelativeTimeFormatConstructor.h */; };
@@ -2308,6 +2310,7 @@
23082310
FFA2E02F2EA16F37006661A3 /* UnaryTests.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FFA2E0262EA16F37006661A3 /* UnaryTests.cpp */; };
23092311
FFB951142C043CA800349750 /* OrderedHashTableHelper.h in Headers */ = {isa = PBXBuildFile; fileRef = FFB951132C043CA800349750 /* OrderedHashTableHelper.h */; settings = {ATTRIBUTES = (Private, ); }; };
23102312
FFB963922E38302B0069A70F /* WasmBreakpointManager.h in Headers */ = {isa = PBXBuildFile; fileRef = FFB9638F2E38302B0069A70F /* WasmBreakpointManager.h */; };
2313+
FFBD27052F6B133900DE6C3D /* ExecutionHandlerVMLifecycleTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FFBD27042F6B133900DE6C3D /* ExecutionHandlerVMLifecycleTest.cpp */; };
23112314
FFBF548A2F189712008F124F /* WasmGDBPacketParser.h in Headers */ = {isa = PBXBuildFile; fileRef = FFBF54872F189712008F124F /* WasmGDBPacketParser.h */; settings = {ATTRIBUTES = (Private, ); }; };
23122315
FFCC9A412BDD7C4700C75345 /* OrderedHashTable.h in Headers */ = {isa = PBXBuildFile; fileRef = FFCC9A402BDD7C4700C75345 /* OrderedHashTable.h */; settings = {ATTRIBUTES = (Private, ); }; };
23132316
FFD49E6E2E276CA10083E383 /* WasmDebugServer.h in Headers */ = {isa = PBXBuildFile; fileRef = FFD49E5A2E276CA10083E383 /* WasmDebugServer.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -5795,6 +5798,9 @@
57955798
E178636C0D9BEEC300D74E75 /* InitializeThreading.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = InitializeThreading.cpp; sourceTree = "<group>"; };
57965799
E18E3A560DF9278C00D90B34 /* VM.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; lineEnding = 0; path = VM.h; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.objcpp; };
57975800
E18E3A570DF9278C00D90B34 /* VM.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; lineEnding = 0; path = VM.cpp; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.cpp; };
5801+
E30001002EF0001000000001 /* MicrotaskCall.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MicrotaskCall.h; sourceTree = "<group>"; };
5802+
E30001002EF0001000000002 /* MicrotaskCallInlines.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MicrotaskCallInlines.h; sourceTree = "<group>"; };
5803+
E30001002EF0001000000003 /* MicrotaskCall.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = MicrotaskCall.cpp; sourceTree = "<group>"; };
57985804
E3060128228F978100FAABDF /* UnlinkedMetadataTable.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = UnlinkedMetadataTable.cpp; sourceTree = "<group>"; };
57995805
E30677971B8BC6F5003F87F0 /* ModuleLoader.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = ModuleLoader.js; sourceTree = "<group>"; };
58005806
E306C1BB2D7900050011D5AC /* MicrotaskQueueInlines.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MicrotaskQueueInlines.h; sourceTree = "<group>"; };
@@ -6426,6 +6432,8 @@
64266432
FFB951132C043CA800349750 /* OrderedHashTableHelper.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OrderedHashTableHelper.h; sourceTree = "<group>"; };
64276433
FFB9638F2E38302B0069A70F /* WasmBreakpointManager.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = WasmBreakpointManager.h; sourceTree = "<group>"; };
64286434
FFB963902E38302B0069A70F /* WasmBreakpointManager.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = WasmBreakpointManager.cpp; sourceTree = "<group>"; };
6435+
FFBD27032F6B133900DE6C3D /* ExecutionHandlerVMLifecycleTest.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ExecutionHandlerVMLifecycleTest.h; sourceTree = "<group>"; };
6436+
FFBD27042F6B133900DE6C3D /* ExecutionHandlerVMLifecycleTest.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = ExecutionHandlerVMLifecycleTest.cpp; sourceTree = "<group>"; };
64296437
FFBF54872F189712008F124F /* WasmGDBPacketParser.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = WasmGDBPacketParser.h; sourceTree = "<group>"; };
64306438
FFBF54882F189712008F124F /* WasmGDBPacketParser.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = WasmGDBPacketParser.cpp; sourceTree = "<group>"; };
64316439
FFCC9A402BDD7C4700C75345 /* OrderedHashTable.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OrderedHashTable.h; sourceTree = "<group>"; };
@@ -10702,6 +10710,8 @@
1070210710
FFE88B9C2EC96CA700C9F5E2 /* ExecutionHandlerTest.h */,
1070310711
FFFAF8CD2EC9A7C000042238 /* ExecutionHandlerTestSupport.cpp */,
1070410712
FFFAF8CC2EC9A7C000042238 /* ExecutionHandlerTestSupport.h */,
10713+
FFBD27042F6B133900DE6C3D /* ExecutionHandlerVMLifecycleTest.cpp */,
10714+
FFBD27032F6B133900DE6C3D /* ExecutionHandlerVMLifecycleTest.h */,
1070510715
FF1344652EB3F2A600940C00 /* ExtGCTests.cpp */,
1070610716
FF06E8BE2F19BC33007B3403 /* GDBPacketParserTest.cpp */,
1070710717
FF06E8BD2F19BC33007B3403 /* GDBPacketParserTest.h */,
@@ -13994,6 +14004,7 @@
1399414004
FFE0C0472F24802E00198308 /* ExecutionHandlerIdleStopTest.cpp in Sources */,
1399514005
FFE88B9E2EC96CA700C9F5E2 /* ExecutionHandlerTest.cpp in Sources */,
1399614006
FFFAF8CE2EC9A7C000042238 /* ExecutionHandlerTestSupport.cpp in Sources */,
14007+
FFBD27052F6B133900DE6C3D /* ExecutionHandlerVMLifecycleTest.cpp in Sources */,
1399714008
FF1344662EB3F2A600940C00 /* ExtGCTests.cpp in Sources */,
1399814009
FF06E8BF2F19BC33007B3403 /* GDBPacketParserTest.cpp in Sources */,
1399914010
FFA2E02E2EA16F37006661A3 /* MemoryTests.cpp in Sources */,

‎Source/JavaScriptCore/runtime/VM.cpp‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@
172172
#include <wtf/darwin/DispatchExtras.h>
173173
#endif
174174

175+
#if ENABLE(WEBASSEMBLY_DEBUGGER)
176+
#include "WasmDebugServerUtilities.h"
177+
#endif
178+
175179
#include <span>
176180

177181
namespace JSC {
@@ -228,25 +232,6 @@ void VM::computeCanUseJIT()
228232
#endif
229233
}
230234

231-
// This function is not meant to be called by anyone. It just provides a convenient scope
232-
// that can is permitted to access private members of VM in order to do some needed
233-
// static_asserts.
234-
inline void VM::checkStaticAsserts()
235-
{
236-
// VM registration is done in the instantiation of its VMThreadContext.
237-
//
238-
// VM registration with the VMManager can only be done after m_apiLock is initialized
239-
// because VMManager may trigger traps to stop the VM, and VMTraps the which uses
240-
// m_apiLock for the VMTraps::SignalSender uses m_apiLock.
241-
//
242-
// VM registration needs to be done before the heap is initialized because we may Global GC
243-
// may want to block the VM from doing any heap activity. In a Global GC world, we would be
244-
// binding this VM to the global heap instead of instantiating the heap field. We want to
245-
// be able to block before that point.
246-
static_assert(OBJECT_OFFSETOF(VM, m_apiLock) < OBJECT_OFFSETOF(VM, m_threadContext));
247-
static_assert(OBJECT_OFFSETOF(VM, m_threadContext) < OBJECT_OFFSETOF(VM, heap));
248-
}
249-
250235
static bool vmCreationShouldCrash = false;
251236

252237
VM::VM(VMType vmType, HeapType heapType, WTF::RunLoop* runLoop, bool* success)
@@ -515,6 +500,10 @@ WTF_ALLOW_UNSAFE_BUFFER_USAGE_END
515500
// We must set this at the end only after the VM is fully initialized.
516501
WTF::storeStoreFence();
517502
m_isInService = true;
503+
504+
// Register after all VM state is initialized so that a stop-the-world triggered
505+
// immediately on registration sees a fully constructed VM.
506+
VMManager::singleton().notifyVMConstruction(*this);
518507
}
519508

520509
static ReadWriteLock s_destructionLock;
@@ -531,6 +520,10 @@ void VM::setCrossTaskToken(RefPtr<CrossTaskToken>&& token)
531520

532521
VM::~VM()
533522
{
523+
// Remove from VMManager before marking as no longer in service or cancelling traps,
524+
// so requestStopAllInternal() never iterates a VM with m_isShuttingDown set.
525+
VMManager::singleton().notifyVMDestruction(*this);
526+
534527
Locker destructionLocker { s_destructionLock.read() };
535528

536529
if (vmType == VMType::Default)

‎Source/JavaScriptCore/runtime/VM.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1345,7 +1345,6 @@ class VM : public ThreadSafeRefCountedWithSuppressingSaferCPPChecking<VM> {
13451345

13461346
DoublyLinkedList<Debugger> m_debuggers;
13471347

1348-
void checkStaticAsserts(); // Not for calling.
13491348

13501349
friend class Heap;
13511350
friend class ExceptionScope; // Friend for exception checking purpose only.

‎Source/JavaScriptCore/runtime/VMManager.cpp‎

Lines changed: 41 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -48,21 +48,8 @@ VMManager& VMManager::singleton()
4848
return manager.get();
4949
}
5050

51-
VMThreadContext::VMThreadContext()
52-
{
53-
VM* vm = VM::fromThreadContext(this);
54-
// Ensure that VM is not in-service yet. Since notifyVMConstruction has memory barrier (lock),
55-
// if we are ensuring this condition here, concurrent threads will see this consistent state.
56-
// Make sure m_isInService is initialized to false before VMThreadContext is initialized.
57-
RELEASE_ASSERT(!vm->isInService());
58-
VMManager::singleton().notifyVMConstruction(*vm);
59-
}
60-
61-
VMThreadContext::~VMThreadContext()
62-
{
63-
VM* vm = VM::fromThreadContext(this);
64-
VMManager::singleton().notifyVMDestruction(*vm);
65-
}
51+
VMThreadContext::VMThreadContext() = default;
52+
VMThreadContext::~VMThreadContext() = default;
6653

6754
bool VMManager::isValidVMSlow(VM* vm)
6855
{
@@ -142,7 +129,7 @@ auto VMManager::info() -> Info
142129
Locker lock { manager.m_worldLock };
143130
info.numberOfVMs = manager.m_numberOfVMs;
144131
info.numberOfActiveVMs = manager.m_numberOfActiveVMs;
145-
info.numberOfStoppedVMs = manager.m_numberOfStoppedVMs.loadRelaxed();
132+
info.numberOfStoppedVMs = manager.m_numberOfStoppedVMs;
146133
info.worldMode = manager.m_worldMode;
147134
info.targetVM = manager.m_targetVM;
148135
return info;
@@ -165,10 +152,7 @@ void VMManager::setMemoryDebuggerCallback(StopTheWorldCallback callback)
165152

166153
void VMManager::incrementActiveVMs(VM& vm) WTF_REQUIRES_LOCK(m_worldLock)
167154
{
168-
if (m_worldMode == Mode::RunAll) {
169-
RELEASE_ASSERT(m_numberOfActiveVMs == invalidNumberOfActiveVMs);
170-
return;
171-
}
155+
RELEASE_ASSERT(m_worldMode != Mode::RunAll);
172156

173157
if (!vm.traps().m_hasBeenCountedAsActive) {
174158
m_numberOfActiveVMs++;
@@ -190,7 +174,7 @@ void VMManager::decrementActiveVMs(VM& vm) WTF_REQUIRES_LOCK(m_worldLock)
190174
vm.traps().m_hasBeenCountedAsActive = false;
191175
}
192176

193-
auto shouldResumeAll = [&] {
177+
auto shouldResumeAll = [&] WTF_REQUIRES_LOCK(m_worldLock) {
194178
if (m_worldMode != Mode::RunAll && !m_numberOfActiveVMs)
195179
return true;
196180
if (m_worldMode == Mode::RunOne) {
@@ -351,7 +335,23 @@ void VMManager::notifyVMStop(VM& vm, StopTheWorldEvent event)
351335
// counted (entry services succeeded), this does nothing.
352336
{
353337
Locker lock { m_worldLock };
338+
// Guard against a late-arriving notifyVMStop() after resumeTheWorld() has already
339+
// completed. Once the world is back in RunAll, touching the counters would corrupt
340+
// the m_numberOfStoppedVMs == m_numberOfActiveVMs invariant used in the following stop section.
341+
if (m_worldMode == Mode::RunAll)
342+
return;
343+
354344
incrementActiveVMs(vm);
345+
++m_numberOfStoppedVMs;
346+
347+
// Must be inside this lock. Once m_numberOfStoppedVMs == m_numberOfActiveVMs,
348+
// the STW callback fires and the debugger assumes isStopped() on every VM.
349+
// A VM preempted between releasing this lock and calling setStopped() would
350+
// cause isStopped() to return false after the STW callback fires.
351+
#if ENABLE(WEBASSEMBLY_DEBUGGER)
352+
if (Options::enableWasmDebugger()) [[unlikely]]
353+
vm.debugState()->setStopped();
354+
#endif
355355
}
356356

357357
// Due to races, we may end up calling notifyVMStop() even when there is no stop to be serviced.
@@ -365,19 +365,12 @@ void VMManager::notifyVMStop(VM& vm, StopTheWorldEvent event)
365365
// called when in Mode::RunOne because new VM thread can be started, and we want those new
366366
// threads to also stop since they aren't the targetVM thread.
367367

368-
#if ENABLE(WEBASSEMBLY_DEBUGGER)
369-
bool enableWasmDebugger = Options::enableWasmDebugger();
370-
if (enableWasmDebugger) [[unlikely]]
371-
vm.debugState()->setStopped();
372-
#endif
373-
374-
m_numberOfStoppedVMs.exchangeAdd(1);
375368

376369
for (;;) {
377370
{
378371
Locker lock { m_worldLock };
379372

380-
RELEASE_ASSERT(m_numberOfStoppedVMs.loadRelaxed() <= m_numberOfActiveVMs);
373+
RELEASE_ASSERT(m_numberOfStoppedVMs <= m_numberOfActiveVMs);
381374

382375
auto fetchTopPriorityStopReason = [&] {
383376
auto pendingRequests = m_pendingStopRequestBits.loadRelaxed();
@@ -401,7 +394,7 @@ void VMManager::notifyVMStop(VM& vm, StopTheWorldEvent event)
401394
// below.
402395
}
403396

404-
auto shouldStop = [&] {
397+
auto shouldStop = [&] WTF_REQUIRES_LOCK(m_worldLock) {
405398
// 1. If the targetVM is already selected, and we're not the targetVM, then stop.
406399
// We need to check this first because in RunOne mode, even if there is no more
407400
// STW request to service, any VM that is not the targetVM still needs to stop.
@@ -415,7 +408,9 @@ void VMManager::notifyVMStop(VM& vm, StopTheWorldEvent event)
415408

416409
// 3. We have a STW request. If not all active VMs are at the stopping point yet,
417410
// then stop and wait for the last VM to stop.
418-
return m_numberOfStoppedVMs.loadRelaxed() != m_numberOfActiveVMs;
411+
// FIXME: rdar://173360944 Any VM may serve the STW callback, not just the last to stop,
412+
// since once the counter lock above is released, any VM can observe m_numberOfStoppedVMs == m_numberOfActiveVMs.
413+
return m_numberOfStoppedVMs != m_numberOfActiveVMs;
419414
};
420415

421416
while (shouldStop())
@@ -487,20 +482,24 @@ void VMManager::notifyVMStop(VM& vm, StopTheWorldEvent event)
487482
}
488483
}
489484

490-
#if ENABLE(WEBASSEMBLY_DEBUGGER)
491-
if (enableWasmDebugger) [[unlikely]] {
492-
vm.debugState()->clearStop();
493-
WTF::storeLoadFence();
494-
}
495-
#endif
485+
unsigned numberOfStoppedVMs = UINT_MAX;
486+
487+
{
488+
Locker lock { m_worldLock };
496489

497-
auto previousCount = m_numberOfStoppedVMs.exchangeSub(1);
490+
// If we get here, we're either transitioning to RunOne or Running mode.
491+
RELEASE_ASSERT(!m_targetVM || m_targetVM == &vm);
498492

499-
// If we get here, we're either transitioning to RunOne or Running mode.
500-
RELEASE_ASSERT(!m_targetVM || m_targetVM == &vm);
493+
numberOfStoppedVMs = --m_numberOfStoppedVMs;
494+
495+
#if ENABLE(WEBASSEMBLY_DEBUGGER)
496+
if (Options::enableWasmDebugger()) [[unlikely]]
497+
vm.debugState()->clearStop();
498+
#endif
499+
}
501500

502501
// Call post-resume callback once when last VM exits and all VMs are running.
503-
if (previousCount == 1 && m_needsWasmDebuggerOnResume.exchange(false))
502+
if (!numberOfStoppedVMs && m_needsWasmDebuggerOnResume.exchange(false))
504503
g_jscConfig.wasmDebuggerOnResume();
505504
}
506505

@@ -518,7 +517,6 @@ void VMManager::notifyVMConstruction(VM& vm)
518517
// If a stop is in progress, we cannot proceed onto initializing (i.e. mutating)
519518
// the heap in the VM constructor. GlobalGC may be expecting a quiescent world
520519
// state at this point. So, go park this thread if needed.
521-
vm.requestStop();
522520
notifyVMStop(vm, StopTheWorldEvent::VMCreated); // Cannot be called while holding m_worldLock.
523521

524522
Locker locker { m_worldLock };
@@ -555,10 +553,8 @@ void VMManager::notifyVMActivation(VM& vm)
555553
s_recentVM = &vm;
556554
needsStopping = m_worldMode != Mode::RunAll;
557555
}
558-
if (needsStopping) {
559-
vm.requestStop();
556+
if (needsStopping)
560557
notifyVMStop(vm, StopTheWorldEvent::VMActivated);
561-
}
562558
}
563559

564560
void VMManager::notifyVMDeactivation(VM& vm)

‎Source/JavaScriptCore/runtime/VMManager.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,9 +384,9 @@ class VMManager {
384384
//
385385
// The choice to not track a valid m_numberOfActiveVMs at all times is just an optimization so
386386
// that we can skip this work when not doing Stop the World.
387-
unsigned m_numberOfActiveVMs { invalidNumberOfActiveVMs };
387+
unsigned m_numberOfActiveVMs WTF_GUARDED_BY_LOCK(m_worldLock) { invalidNumberOfActiveVMs };
388388

389-
Atomic<unsigned> m_numberOfStoppedVMs { 0 };
389+
unsigned m_numberOfStoppedVMs WTF_GUARDED_BY_LOCK(m_worldLock) { 0 };
390390

391391
// === End of variables only relevant for StopTheWorld =================================
392392

‎Source/JavaScriptCore/runtime/VMTraps.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ bool VMTraps::handleTraps(VMTraps::BitField mask)
435435
VM& vm = this->vm();
436436
auto scope = DECLARE_THROW_SCOPE(vm);
437437
ASSERT(onlyContainsAsyncEvents(mask));
438-
ASSERT(needHandling(mask));
438+
// No ASSERT(needHandling(mask)): cancelStop() from resumeTheWorld() can race with this call.
439439

440440
if (m_trapsDeferred)
441441
return false; // We'll service them on the next opportunity after deferring has stopped.
@@ -505,6 +505,10 @@ bool VMTraps::handleTraps(VMTraps::BitField mask)
505505
didHandleTrap = true;
506506
break;
507507

508+
// cancelStop() cleared the bit between needHandling() and takeTopPriorityTrap().
509+
case NoEvent:
510+
break;
511+
508512
case NeedExceptionHandling:
509513
default:
510514
RELEASE_ASSERT_NOT_REACHED();

‎Source/JavaScriptCore/shell/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ if (DEVELOPER_MODE)
8989
../wasm/debugger/tests/ExecutionHandlerIdleStopTest.cpp
9090
../wasm/debugger/tests/ExecutionHandlerTest.cpp
9191
../wasm/debugger/tests/ExecutionHandlerTestSupport.cpp
92+
../wasm/debugger/tests/ExecutionHandlerVMLifecycleTest.cpp
9293
../wasm/debugger/tests/ExtGCTests.cpp
9394
../wasm/debugger/tests/GDBPacketParserTest.cpp
9495
../wasm/debugger/tests/MemoryTests.cpp

‎Source/JavaScriptCore/wasm/debugger/README.md‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -230,15 +230,6 @@ See [RWI_ARCHITECTURE.md](./RWI_ARCHITECTURE.md) for complete setup instructions
230230
- **Issue**: Thread select and stop reply protocol handlers need improvement to correctly display multi-VM data in LLDB
231231
- **Current Status**: Multi-VM stop-the-world is implemented, but thread information may not display correctly in LLDB UI
232232

233-
### VM Lifecycle and Synchronization Testing
234-
235-
- **Issue**: ExecutionHandler stress tests need additional coverage for VM lifecycle edge cases and race conditions
236-
- **Current Test Limitations**: Tests wait for VM construction and instance registration, but this doesn't guarantee VMs are actively running code that checks traps
237-
- **Missing Test Coverage**:
238-
- VM lifecycle edge cases (construction, initialization, instance registration)
239-
- `interrupt()` race conditions when VMs are not yet executing code
240-
- Synchronization between VM construction and actual code execution
241-
242233
### X86_64 Support
243234

244235
- **Issue**: The WebAssembly debugger is currently restricted to ARM64 platforms only

0 commit comments

Comments
 (0)