From ef426c8f82881e8378aa03fb47b627caa81f92e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 17 Aug 2026 19:33:26 +0200 Subject: [PATCH] Reply to every debugger request, even the ones that finish later Eight events answered nothing at all: cpu.stepping, cpu.resume, gpu.stats.feed and the five stepping requests. Their documented contract was "no immediate response, an event follows", which leaves a client unable to tell an accepted request from one that was dropped - and forces any request/response correlation to carry a hardcoded list of events that don't answer. wsdbg's --sync doesn't have that list, so it waits for the next message and treats whatever broadcast arrives first as the answer, silently misattributing every later response in the script. Fixed centrally in the dispatch loop rather than in the eight handlers: if a handler finishes without having sent anything, send an empty response carrying its ticket. That also covers handlers added later, which is the part a per-handler fix wouldn't. The asynchronous event that reports the real outcome is unchanged and still follows. The two are easy to tell apart - the acknowledgement carries the ticket from the request, a broadcast has none: -> {"event":"cpu.stepInto","ticket":3} <- {"event":"cpu.stepInto","ticket":3} <- {"event":"cpu.stepping","pc":142622896,"reason":"cpu.stepInto",...} Existing clients ignore events they didn't ask for, and this adds a message rather than changing or removing one, so nothing that worked before breaks. pspautotests 314/314. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/Debugger/WebSocket.cpp | 8 ++++++++ Core/Debugger/WebSocket/CPUCoreSubscriber.cpp | 6 ++++-- .../Debugger/WebSocket/GPUStatsSubscriber.cpp | 3 ++- .../Debugger/WebSocket/SteppingSubscriber.cpp | 20 +++++++++---------- docs/WebSocketDebugger.md | 8 ++++++++ 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/Core/Debugger/WebSocket.cpp b/Core/Debugger/WebSocket.cpp index 90673cddce..4b0023bab9 100644 --- a/Core/Debugger/WebSocket.cpp +++ b/Core/Debugger/WebSocket.cpp @@ -224,6 +224,14 @@ void HandleDebuggerRequest(const http::ServerRequest &request) { if (eventFunc != eventHandlers.end()) { eventFunc->second(req); if (!req.Finish()) { + // The handler arranged something that finishes later - a step, a resume, a stats + // feed - rather than answering now. Acknowledge it anyway, so that *every* request + // gets exactly one reply. Without this a client can't tell "accepted, wait for the + // event" from "dropped on the floor", and any request/response correlation has to + // special-case a list of events that don't answer. The event that actually reports + // the result (cpu.stepping, and so on) still follows. + req.Respond(); + req.Finish(); // Poll more frequently for a second in case this triggers something. highActivity = 1000; } diff --git a/Core/Debugger/WebSocket/CPUCoreSubscriber.cpp b/Core/Debugger/WebSocket/CPUCoreSubscriber.cpp index 2fd0c62ae8..5603b94e9e 100644 --- a/Core/Debugger/WebSocket/CPUCoreSubscriber.cpp +++ b/Core/Debugger/WebSocket/CPUCoreSubscriber.cpp @@ -66,7 +66,8 @@ static DebugInterface *CPUFromRequest(DebuggerRequest &req) { // // No parameters. // -// No immediate response. Once CPU is stepping, a "cpu.stepping" event will be sent. +// Response (same event name) with no extra data, acknowledging the request. The CPU may not be +// stepping yet at that point - a "cpu.stepping" event follows once it is. void WebSocketCPUStepping(DebuggerRequest &req) { if (!currentDebugMIPS->isAlive()) { return req.Fail("CPU not started"); @@ -83,7 +84,8 @@ void WebSocketCPUStepping(DebuggerRequest &req) { // // No parameters. // -// No immediate response. Once CPU is stepping, a "cpu.resume" event will be sent. +// Response (same event name) with no extra data, acknowledging the request. A "cpu.resume" +// event follows once the CPU is actually running again. void WebSocketCPUResume(DebuggerRequest &req) { if (!currentDebugMIPS->isAlive()) { return req.Fail("CPU not started"); diff --git a/Core/Debugger/WebSocket/GPUStatsSubscriber.cpp b/Core/Debugger/WebSocket/GPUStatsSubscriber.cpp index c0d686b9b0..bbe4b43a07 100644 --- a/Core/Debugger/WebSocket/GPUStatsSubscriber.cpp +++ b/Core/Debugger/WebSocket/GPUStatsSubscriber.cpp @@ -175,7 +175,8 @@ void WebSocketGPUStatsState::Get(DebuggerRequest &req) { // Parameters: // - enable: optional boolean, pass false to stop the feed. // -// No immediate response. Events sent each frame (as gpu.stats.get.) +// Response (same event name) with no extra data, acknowledging the request. Stats events then +// arrive each frame (as gpu.stats.get.) // // Note: info and timing will be accurate after the first frame. void WebSocketGPUStatsState::Feed(DebuggerRequest &req) { diff --git a/Core/Debugger/WebSocket/SteppingSubscriber.cpp b/Core/Debugger/WebSocket/SteppingSubscriber.cpp index bf834008f7..82a732ce72 100644 --- a/Core/Debugger/WebSocket/SteppingSubscriber.cpp +++ b/Core/Debugger/WebSocket/SteppingSubscriber.cpp @@ -85,7 +85,8 @@ static DebugInterface *CPUFromRequest(DebuggerRequest &req, uint32_t *threadID = // Parameters: // - thread: optional number indicating the thread id to plan stepping on. // -// No immediate response on success. A cpu.stepping event will be sent once complete. +// Response (same event name) with no extra data on success. A cpu.stepping event follows once +// the step completes. // May fail (same-thread case only) if another step/run request is already pending this host // frame - safe to retry shortly after. // @@ -114,11 +115,10 @@ void WebSocketSteppingState::Into(DebuggerRequest &req) { g_breakpoints.SetSkipFirst(currentMIPS->pc); // Core_RequestCPUStep() can fail (a step or run request is already queued this host - // frame - see its own "Can't submit two steps in one host frame" log). Previously - // unchecked here: on failure, no step ever happens and no cpu.stepping event ever - // fires, but the client got no response either (this event's contract is "no - // immediate response, a cpu.stepping event follows") - so a rejected step looked - // identical to one that's just still in flight, indefinitely. Surface it instead. + // frame - see its own "Can't submit two steps in one host frame" log). On failure no + // step ever happens and no cpu.stepping event ever fires, so a rejected step would + // otherwise be indistinguishable from one still in flight - the acknowledgement every + // request now gets doesn't tell those apart. Surface it as an error instead. if (!Core_RequestCPUStep(CPUStepType::Into)) { req.Fail("Could not step: a step or run request is already pending"); return; @@ -145,7 +145,7 @@ void WebSocketSteppingState::Into(DebuggerRequest &req) { // Parameters: // - thread: optional number indicating the thread id to plan stepping on. // -// No immediate response. A cpu.stepping event will be sent once complete. +// Response (same event name) with no extra data. A cpu.stepping event follows once complete. // // Note: any thread can wake the cpu when it hits the next instruction currently. void WebSocketSteppingState::Over(DebuggerRequest &req) { @@ -200,7 +200,7 @@ void WebSocketSteppingState::Over(DebuggerRequest &req) { // Parameters: // - thread: optional number indicating the thread id to plan stepping on. // -// No immediate response. A cpu.stepping event will be sent once complete. +// Response (same event name) with no extra data. A cpu.stepping event follows once complete. // // Note: any thread can wake the cpu when it hits the next instruction currently. void WebSocketSteppingState::Out(DebuggerRequest &req) { @@ -253,7 +253,7 @@ void WebSocketSteppingState::Out(DebuggerRequest &req) { // Parameters: // - address: number parameter for destination. // -// No immediate response. A cpu.stepping event will be sent once complete. +// Response (same event name) with no extra data. A cpu.stepping event follows once complete. void WebSocketSteppingState::RunUntil(DebuggerRequest &req) { if (!currentDebugMIPS->isAlive()) { return req.Fail("CPU not started"); @@ -338,7 +338,7 @@ void WebSocketSteppingState::RunUntilTime(DebuggerRequest &req) { // // No parameters. // -// No immediate response. A cpu.stepping event will be sent once complete. +// Response (same event name) with no extra data. A cpu.stepping event follows once complete. void WebSocketSteppingState::HLE(DebuggerRequest &req) { if (!currentDebugMIPS->isAlive()) { return req.Fail("CPU not started"); diff --git a/docs/WebSocketDebugger.md b/docs/WebSocketDebugger.md index deb9806e52..9b44ef17e1 100644 --- a/docs/WebSocketDebugger.md +++ b/docs/WebSocketDebugger.md @@ -57,6 +57,14 @@ Responses use the *same* event name as the request: ``` Responses are not always immediate - some handlers respond asynchronously. +**Every request gets exactly one reply**: either a response, or an `error`. A +handler whose real result arrives later (`cpu.stepInto`, `cpu.resume`, +`gpu.stats.feed`, ...) is acknowledged with an empty response carrying your +ticket, and the event reporting the actual outcome (`cpu.stepping`, ...) +follows separately. So a client can always correlate request to reply without +keeping a list of events that don't answer, and "no reply" unambiguously means +the request is still being processed. + Errors look like this: ```json { "event": "error", "message": "...", "level": 2, "ticket": 1 }