Skip to content

Commit 953faee

Browse files
hyjorc1hyjorc1
committed
[JSC][WASM][Debugger] Fix WasmDebugger RWI lifecycle: init ordering, cache handling, and process exit
https://bugs.webkit.org/show_bug.cgi?id=312094 rdar://174599638 Reviewed by Devin Rousso and BJ Burg. Init ordering fix: WasmDebuggerDebuggable was created in didFinishLaunching() before startRWI() was called, so early LLDB packets could arrive before the debug server was ready. Fix by introducing a WasmDebugServerReady IPC sent from WebContent after startRWI() succeeds. The UIProcess creates the debuggable only in wasmDebugServerReady(), guaranteeing the debug server is initialized before LLDB can attach. Process cache improvement: Previously nothing happened on cache entry or exit, leaving the debuggable alive when the process was cached (LLDB stayed attached to an idle process) and never recreating it on cache reuse. Now on cache entry the debuggable is destroyed and a ResetServer IPC is dispatched to the WasmDebuggerDispatcher WorkQueue — which runs independently of the mutator — to cleanly reset server state. On cache exit, WebContent resends WasmDebugServerReady so the UIProcess recreates the debuggable for the new navigation. Process exit fix: destroyWasmDebuggerTarget() is now called before shutDownProcess() in shutDown() so LLDB is notified via RemoteInspector and detaches cleanly before the WebContent process is killed. Make DebugServer::reset() public so WasmDebuggerDispatcher can call it directly. Change DebugServer::startRWI() to return void since it always succeeds. Remove m_createWasmDebuggerDebuggable and move the enableWebAssemblyDebugger parameter from WebProcessProxy::create() into initializeNewWebProcess(), since debuggable creation is now driven by WasmDebugServerReady rather than launch. Add a complete Component Lifecycle section to RWI_ARCHITECTURE.md. Canonical link: https://commits.webkit.org/311292@main
1 parent de5a7d7 commit 953faee

15 files changed

Lines changed: 173 additions & 55 deletions

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

Lines changed: 97 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ WebKit's WebAssembly debugging uses a **singleton `WasmDebugServer`** that manag
1515
### Key Design Principles
1616

1717
1. **Process-Wide Singleton**: One `WasmDebugServer::singleton()` per WebContent process
18-
2. **WorkQueue-Based IPC**: Debugging commands processed on background thread (supports debugging infinite loops)
18+
2. **WorkQueue-Based IPC**: Debugging commands processed on background thread (supports debugging while main thread is blocked)
1919
3. **RWI Integration**: One `WasmDebuggerDebuggable` target per WebContent process
2020
4. **GDB Remote Serial Protocol**: LLDB communicates using standard GDB packets
2121

@@ -57,7 +57,7 @@ WebKit's WebAssembly debugging uses a **singleton `WasmDebugServer`** that manag
5757
### UI Process (Safari)
5858

5959
**WebProcessProxy** - Manages one WebContent process
60-
- Creates `WasmDebuggerDebuggable` on construction (if `enableWasmDebugger` enabled)
60+
- Creates `WasmDebuggerDebuggable` only after receiving `WasmDebugServerReady` from WebContent
6161
- Forwards debugging commands via IPC
6262
- Routes responses back to LLDB
6363

@@ -73,12 +73,12 @@ WebKit's WebAssembly debugging uses a **singleton `WasmDebugServer`** that manag
7373

7474
**WasmDebuggerDispatcher** - WorkQueue-based IPC receiver
7575
- Receives debugging commands on **WorkQueue thread** (not main thread)
76-
- Enables debugging when main thread blocked in infinite loop
76+
- Enables debugging when main thread is blocked
7777
- Matches `WebInspectorInterruptDispatcher` pattern (JavaScript debugging)
7878

7979
**WebProcess** - Process singleton
8080
- Owns `WasmDebuggerDispatcher` instance
81-
- Initializes `WasmDebugServer` on startup
81+
- Calls `DebugServer::singleton().startRWI()` in `initializeWebProcess()` and sends `WasmDebugServerReady` IPC on success
8282
- Sets up response handler for IPC communication
8383

8484
### JavaScriptCore (JSC)
@@ -113,21 +113,109 @@ WasmDebugServer (JSC)
113113

114114
**Key Points:**
115115
- External client relays raw GDB packets between LLDB and WebKit
116-
- Target registration: WebProcessProxy creates `WasmDebuggerDebuggable` with auto-assigned numeric Target ID and name showing OS PID
116+
- Target registration: WebProcessProxy creates `WasmDebuggerDebuggable` in `wasmDebugServerReady()` (after `WasmDebugServerReady` IPC) with auto-assigned numeric Target ID and name showing OS PID
117117
- IPC routing: Messages dispatched to WorkQueue in WebContent Process, NOT main thread
118-
- WorkQueue enables debugging when main thread blocked in infinite loop
118+
- WorkQueue enables debugging when main thread is blocked
119119
- Pattern matches `WebInspectorInterruptDispatcher` (JavaScript debugging)
120120

121121
**Critical Architecture Decision: WorkQueue Threading**
122122

123-
Main thread can be blocked in infinite Wasm loop, but debugging must still work:
123+
Main thread can be blocked, but debugging must still work:
124124
- Kernel queues IPC messages independently
125125
- WorkQueue thread (in WebContent Process) processes debug commands concurrently
126126
- Mutator thread waits on condition variable when paused
127127
- WorkQueue thread signals condition variable to resume execution
128128

129129
---
130130

131+
## Component Lifecycle
132+
133+
### Startup (Fresh Process Launch)
134+
135+
```
136+
UIProcess WebContent Process (main thread)
137+
─────────────────────────────────────────────────────────────────
138+
platformInitializeWebProcess()
139+
- shouldEnableWebAssemblyDebugger param
140+
- JSC::Options::enableWasmDebugger = true
141+
142+
initializeWebProcess()
143+
- DebugServer::singleton().startRWI()
144+
wasmDebugServerReady() ◄────── - send WasmDebugServerReady IPC
145+
- createWasmDebuggerTarget()
146+
- register with RemoteInspector
147+
```
148+
149+
**Key invariant**: The `WasmDebuggerDebuggable` is created only after `startRWI()` succeeds in the WebContent process. This guarantees that when LLDB attaches and sends packets, the `WasmDebugServer` is fully initialized and ready to handle them.
150+
151+
### Process Cache Entry
152+
153+
When a tab navigates away and the WebContent process enters the process cache:
154+
155+
```
156+
UIProcess (main thread) WebContent WorkQueue thread
157+
──────────────────────────────────────────────────────────────
158+
setIsInProcessCache(true)
159+
if m_wasmDebuggerDebuggable:
160+
destroyWasmDebuggerTarget()
161+
- detach from RemoteInspector
162+
send ResetServer IPC ──────► resetServer()
163+
- DebugServer::reset()
164+
- if VM stopped: resumeImpl()
165+
- clearAllBreakpoints()
166+
- reset protocol state
167+
```
168+
169+
**Why WorkQueue, not main thread**: The WorkQueue thread runs independently of the JS/Wasm mutator. If the main VM is stopped at a breakpoint when the tab closes, the main thread is unresponsive and cannot process IPCs. The WorkQueue handles `ResetServer` regardless, and `reset()` resumes the stopped VM before clearing state.
170+
171+
**STW invariant**: Stop-the-world stops ALL mutators simultaneously. If any VM is stopped (blocked in `stopCode()`), the main thread is also stopped — making it impossible to process `SetIsInProcessCache`. Therefore, when `setIsInProcessCache` runs on the main thread, no VM can be blocked in `stopCode()`, and the WorkQueue is always idle and ready to receive `ResetServer`.
172+
173+
### Process Cache Exit
174+
175+
When a cached process is reused for a new navigation:
176+
177+
```
178+
WebContent Process (main thread) UIProcess
179+
──────────────────────────────────────────────────────────────
180+
setIsInProcessCache(false)
181+
- send WasmDebugServerReady ────► wasmDebugServerReady()
182+
- createWasmDebuggerTarget()
183+
- re-register with RemoteInspector
184+
```
185+
186+
The `WasmDebugServer` itself never stops — it is a process-lifetime singleton. Only the `WasmDebuggerDebuggable` (the RWI target visible to LLDB) is destroyed on cache entry and recreated on cache exit.
187+
188+
### Process Exit (No Cache)
189+
190+
When the WebContent process exits normally (tab closed, no process cache):
191+
192+
```
193+
UIProcess (main thread)
194+
────────────────────────
195+
shutDown()
196+
if m_wasmDebuggerDebuggable:
197+
destroyWasmDebuggerTarget()
198+
- detach from RemoteInspector
199+
shutDownProcess()
200+
```
201+
202+
The debuggable is destroyed before the process is killed so LLDB detaches cleanly
203+
via RemoteInspector before the connection drops. No ResetServer IPC is needed —
204+
all WebContent process state is discarded on exit.
205+
206+
### Full Lifecycle Summary
207+
208+
```
209+
1. Fresh launch: enable option → startRWI → WasmDebugServerReady → createDebuggable
210+
2. Cache entry: destroyDebuggable → send ResetServer → reset()
211+
3. Cache exit: WasmDebugServerReady → createDebuggable
212+
4. Cache entry: destroyDebuggable → send ResetServer → reset()
213+
(steps 3-4 repeat indefinitely)
214+
5. Process exit: destroyDebuggable → shutDownProcess()
215+
```
216+
217+
---
218+
131219
## RWI Integration
132220

133221
**Target Type:** `RWIDebuggableTypeWasmDebugger` (distinct from `RWIDebuggableTypeWebPage`)
@@ -238,8 +326,8 @@ When the Wasm debugger pauses at a breakpoint (stop-the-world), the main thread
238326
3. **From the user's perspective** - Web Inspector appears frozen or disconnected
239327

240328
The interaction is asymmetric:
241-
- **Pausing in Wasm debugger** → Blocks main thread → **Freezes both JS execution and Web Inspector**
242-
- **Pausing in JS debugger** → Runs nested event loop → **Wasm debugger continues working** (uses WorkQueue IPC)
329+
- **Pausing in Wasm debugger** - Blocks main thread - **Freezes both JS execution and Web Inspector**
330+
- **Pausing in JS debugger** - Runs nested event loop - **Wasm debugger continues working** (uses WorkQueue IPC)
243331

244332
**Why Stop-the-World Is Correct**:
245333

‎Source/JavaScriptCore/wasm/debugger/WasmDebugServer.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,11 @@ bool DebugServer::start()
104104
}
105105

106106
#if ENABLE(REMOTE_INSPECTOR)
107-
bool DebugServer::startRWI(Function<bool(const String&)>&& rwiResponseHandler)
107+
void DebugServer::startRWI(Function<bool(const String&)>&& rwiResponseHandler)
108108
{
109109
if (isState(State::Running) || isState(State::Starting)) {
110110
dataLogLnIf(Options::verboseWasmDebugger(), "[Debugger] Server already running or is starting");
111-
return true;
111+
return;
112112
}
113113

114114
setState(State::Starting);
@@ -123,7 +123,6 @@ bool DebugServer::startRWI(Function<bool(const String&)>&& rwiResponseHandler)
123123

124124
setState(State::Running);
125125
dataLogLnIf(Options::verboseWasmDebugger(), "[Debugger] Wasm Debug Server started in RWI mode (WorkQueue-based)");
126-
return true;
127126
}
128127
#endif
129128

‎Source/JavaScriptCore/wasm/debugger/WasmDebugServer.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class DebugServer {
107107
// 1. Direct TCP socket mode (JSC shell debugging)
108108
// 2. Remote Web Inspector integration mode (WebKit debugging)
109109
bool isRWIMode() const { return !!m_rwiResponseHandler; }
110-
JS_EXPORT_PRIVATE bool startRWI(Function<bool(const String&)>&& rwiResponseHandler);
110+
JS_EXPORT_PRIVATE void startRWI(Function<bool(const String&)>&& rwiResponseHandler);
111111
#endif
112112

113113
void trackInstance(JSWebAssemblyInstance*);
@@ -144,8 +144,9 @@ class DebugServer {
144144

145145
JS_EXPORT_PRIVATE ModuleManager& moduleManager() const;
146146

147+
JS_EXPORT_PRIVATE void reset();
148+
147149
private:
148-
void reset();
149150

150151
void setState(State);
151152
JS_EXPORT_PRIVATE bool NODELETE isState(State) const;

‎Source/JavaScriptCore/wasm/debugger/tests/ExecutionHandlerTestSupport.cpp‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -265,13 +265,11 @@ void setupTestEnvironment(DebugServer*& debugServer, ExecutionHandler*& executio
265265
Options::setOptions("--enableWasmDebugger=true");
266266

267267
debugServer = &DebugServer::singleton();
268-
bool started = debugServer->startRWI([](const String& packet) {
268+
debugServer->startRWI([](const String& packet) {
269269
replyCount++;
270270
dataLogLnIf(verboseLogging, RWI_REPLY_PREFIX, packet);
271271
return true;
272272
});
273-
274-
RELEASE_ASSERT(started, "Failed to start DebugServer in RWI mode");
275273
RELEASE_ASSERT(debugServer->hasDebugger(), "DebugServer has no debug client after RWI start");
276274

277275
executionHandler = &debugServer->execution();

‎Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -402,10 +402,6 @@ static void logProcessPoolState(const WebProcessPool& pool)
402402
parameters.shouldEnableMemoryPressureReliefLogging = [defaults boolForKey:@"LogMemoryJetsamDetails"];
403403
parameters.shouldSuppressMemoryPressureHandler = [defaults boolForKey:WebKitSuppressMemoryPressureHandlerDefaultsKey];
404404

405-
#if ENABLE(WEBASSEMBLY_DEBUGGER) && ENABLE(REMOTE_INSPECTOR)
406-
parameters.shouldEnableWebAssemblyDebugger = process.createWasmDebuggerDebuggable();
407-
#endif
408-
409405
// FIXME: This should really be configurable; we shouldn't just blindly allow read access to the UI process bundle.
410406
parameters.uiProcessBundleResourcePath = m_resolvedPaths.uiProcessBundleResourcePath;
411407
if (auto handle = SandboxExtension::createHandleWithoutResolvingPath(parameters.uiProcessBundleResourcePath, SandboxExtension::Type::ReadOnly))

‎Source/WebKit/UIProcess/Inspector/WasmDebuggerDebuggable.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,9 @@ std::optional<ProcessID> WasmDebuggerDebuggable::webContentProcessPID() const
6868
if (!process)
6969
return std::nullopt;
7070

71-
// When WasmDebuggerDebuggable is created, the WebContent process is guaranteed to have
72-
// finished launching (see didFinishLaunching -> createWasmDebuggerTarget).
73-
// Therefore, processID() must return a valid non-zero PID.
71+
// WasmDebuggerDebuggable is created in wasmDebugServerReady(), which is an IPC handler
72+
// called by the WebContent process after startRWI() succeeds. Since the message came from
73+
// the process itself, it is guaranteed to be running and processID() must be non-zero.
7474
auto pid = process->processID();
7575
RELEASE_ASSERT(pid);
7676
return pid;

‎Source/WebKit/UIProcess/WebProcessPool.cpp‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -820,8 +820,8 @@ void WebProcessPool::resolvePathsForSandboxExtensions()
820820

821821
Ref<WebProcessProxy> WebProcessPool::createNewWebProcess(WebsiteDataStore* websiteDataStore, WebProcessProxy::LockdownMode lockdownMode, EnhancedSecurity enhancedSecurity, WebProcessProxy::EnableWebAssemblyDebugger enableWebAssemblyDebugger, WebProcessProxy::IsPrewarmed isPrewarmed, CrossOriginMode crossOriginMode)
822822
{
823-
auto processProxy = WebProcessProxy::create(*this, websiteDataStore, lockdownMode, enhancedSecurity, isPrewarmed, crossOriginMode, WebProcessProxy::ShouldLaunchProcess::Yes, enableWebAssemblyDebugger);
824-
initializeNewWebProcess(processProxy, websiteDataStore, isPrewarmed);
823+
auto processProxy = WebProcessProxy::create(*this, websiteDataStore, lockdownMode, enhancedSecurity, isPrewarmed, crossOriginMode, WebProcessProxy::ShouldLaunchProcess::Yes);
824+
initializeNewWebProcess(processProxy, websiteDataStore, isPrewarmed, enableWebAssemblyDebugger);
825825
m_processes.append(processProxy.copyRef());
826826

827827
return processProxy;
@@ -957,7 +957,7 @@ WebProcessDataStoreParameters WebProcessPool::webProcessDataStoreParameters(WebP
957957
};
958958
}
959959

960-
void WebProcessPool::initializeNewWebProcess(WebProcessProxy& process, WebsiteDataStore* websiteDataStore, WebProcessProxy::IsPrewarmed isPrewarmed)
960+
void WebProcessPool::initializeNewWebProcess(WebProcessProxy& process, WebsiteDataStore* websiteDataStore, WebProcessProxy::IsPrewarmed isPrewarmed, WebProcessProxy::EnableWebAssemblyDebugger enableWebAssemblyDebugger)
961961
{
962962
WebProcessCreationParameters parameters;
963963
parameters.auxiliaryProcessParameters = AuxiliaryProcessProxy::auxiliaryProcessParameters();
@@ -1043,6 +1043,12 @@ void WebProcessPool::initializeNewWebProcess(WebProcessProxy& process, WebsiteDa
10431043

10441044
parameters.memoryFootprintNotificationThresholds = m_configuration->memoryFootprintNotificationThresholds();
10451045

1046+
#if ENABLE(WEBASSEMBLY_DEBUGGER) && ENABLE(REMOTE_INSPECTOR)
1047+
parameters.shouldEnableWebAssemblyDebugger = (enableWebAssemblyDebugger == WebProcessProxy::EnableWebAssemblyDebugger::Yes);
1048+
#else
1049+
UNUSED_PARAM(enableWebAssemblyDebugger);
1050+
#endif
1051+
10461052
// Add any platform specific parameters
10471053
platformInitializeWebProcess(process, parameters);
10481054

@@ -1342,8 +1348,7 @@ Ref<WebPageProxy> WebProcessPool::createWebPage(PageClient& pageClient, Ref<API:
13421348
// In the common case, we delay process launch until something is actually loaded in the page.
13431349
process = dummyProcessProxy(pageConfiguration->websiteDataStore().sessionID());
13441350
if (!process) {
1345-
auto enableWebAssemblyDebugger = protect(pageConfiguration->preferences())->webAssemblyDebuggerEnabled() ? WebProcessProxy::EnableWebAssemblyDebugger::Yes : WebProcessProxy::EnableWebAssemblyDebugger::No;
1346-
process = WebProcessProxy::create(*this, protect(pageConfiguration->websiteDataStore()).ptr(), lockdownMode, enhancedSecurity, WebProcessProxy::IsPrewarmed::No, CrossOriginMode::Shared, WebProcessProxy::ShouldLaunchProcess::No, enableWebAssemblyDebugger);
1351+
process = WebProcessProxy::create(*this, protect(pageConfiguration->websiteDataStore()).ptr(), lockdownMode, enhancedSecurity, WebProcessProxy::IsPrewarmed::No, CrossOriginMode::Shared, WebProcessProxy::ShouldLaunchProcess::No);
13471352
m_dummyProcessProxies.add(pageConfiguration->websiteDataStore().sessionID(), *process);
13481353
m_processes.append(*process);
13491354
}

‎Source/WebKit/UIProcess/WebProcessPool.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -655,7 +655,7 @@ class WebProcessPool final
655655

656656
RefPtr<WebProcessProxy> tryTakePrewarmedProcess(WebsiteDataStore&, WebProcessProxy::LockdownMode, EnhancedSecurity, const API::PageConfiguration&);
657657

658-
void initializeNewWebProcess(WebProcessProxy&, WebsiteDataStore*, WebProcessProxy::IsPrewarmed = WebProcessProxy::IsPrewarmed::No);
658+
void initializeNewWebProcess(WebProcessProxy&, WebsiteDataStore*, WebProcessProxy::IsPrewarmed = WebProcessProxy::IsPrewarmed::No, WebProcessProxy::EnableWebAssemblyDebugger = WebProcessProxy::EnableWebAssemblyDebugger::No);
659659

660660
void handleMessage(IPC::Connection&, const String& messageName, const UserData& messageBody);
661661
void handleSynchronousMessage(IPC::Connection&, const String& messageName, const UserData& messageBody, CompletionHandler<void(UserData&&)>&&);

0 commit comments

Comments
 (0)