From 7625f7529a60c2d37bdac74bd5071cf9c2c19db7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 21 Oct 2026 09:34:27 +0200 Subject: [PATCH] Debugger: Say in "deferred" when a ticketed reply follows input.buttons.press, gpu.stats.get, gpu.record.dump and the game.state.* operations with wait answer later with a message carrying the request's ticket. Their deferred acknowledgement carried that ticket too, and wsdbg took it for the answer, so a waited save that failed exited 0 and --sync moved on before it was done. Handlers that answer this way now call ReplyLater(), and the deferred event says "replyFollows": true. wsdbg keeps waiting for the reply in one-shot mode, under --sync and for :nowait lines. An error reply or a missing one to a :nowait line now fails the run, a raw line can't reuse a ticket that is still pending, and auto-assigned tickets skip past ones a raw line chose. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/Debugger/WebSocket.cpp | 2 +- .../WebSocket/GPURecordSubscriber.cpp | 1 + .../Debugger/WebSocket/GPUStatsSubscriber.cpp | 1 + Core/Debugger/WebSocket/GameSubscriber.cpp | 1 + Core/Debugger/WebSocket/InputSubscriber.cpp | 1 + Core/Debugger/WebSocket/WebSocketUtils.h | 17 +++- Tools/wsdbg/README.md | 13 +++ Tools/wsdbg/src/main.rs | 90 +++++++++++++++---- docs/WebSocketDebugger.md | 12 +++ docs/async-savestates.md | 4 +- 10 files changed, 122 insertions(+), 20 deletions(-) diff --git a/Core/Debugger/WebSocket.cpp b/Core/Debugger/WebSocket.cpp index adfc515c23..0c0e844959 100644 --- a/Core/Debugger/WebSocket.cpp +++ b/Core/Debugger/WebSocket.cpp @@ -288,7 +288,7 @@ void HandleDebuggerRequest(const http::ServerRequest &request) { // exactly what they saw before; see client.config.set for why it can't be the // default. if (client_info.acknowledgeDeferred) - ws->Send(DebuggerDeferredEvent(event, root)); + ws->Send(DebuggerDeferredEvent(event, root, req.RepliesLater())); // Poll more frequently for a second in case this triggers something. highActivity = 1000; } diff --git a/Core/Debugger/WebSocket/GPURecordSubscriber.cpp b/Core/Debugger/WebSocket/GPURecordSubscriber.cpp index bad43f8175..ff49fd70e1 100644 --- a/Core/Debugger/WebSocket/GPURecordSubscriber.cpp +++ b/Core/Debugger/WebSocket/GPURecordSubscriber.cpp @@ -89,6 +89,7 @@ void WebSocketGPURecordState::Dump(DebuggerRequest &req) { const JsonNode *value = req.data.get("ticket"); lastTicket_ = value ? json_stringify(value) : ""; + req.ReplyLater(); } // This handles the asynchronous gpu.record.dump response. diff --git a/Core/Debugger/WebSocket/GPUStatsSubscriber.cpp b/Core/Debugger/WebSocket/GPUStatsSubscriber.cpp index 03e2d3c588..0db8fc5bfb 100644 --- a/Core/Debugger/WebSocket/GPUStatsSubscriber.cpp +++ b/Core/Debugger/WebSocket/GPUStatsSubscriber.cpp @@ -168,6 +168,7 @@ void WebSocketGPUStatsState::Get(DebuggerRequest &req) { const JsonNode *value = req.data.get("ticket"); lastTicket_ = value ? json_stringify(value) : ""; + req.ReplyLater(); } // Setup GPU stats feed (gpu.stats.feed) diff --git a/Core/Debugger/WebSocket/GameSubscriber.cpp b/Core/Debugger/WebSocket/GameSubscriber.cpp index 5a84b1524b..989ec9fe14 100644 --- a/Core/Debugger/WebSocket/GameSubscriber.cpp +++ b/Core/Debugger/WebSocket/GameSubscriber.cpp @@ -191,6 +191,7 @@ void WebSocketGameState::StateOperation(DebuggerRequest &req, StateOp op) { } const JsonNode *ticket = req.data.get("ticket"); waiting_.push_back({ event, ticket ? json_stringify(ticket) : "", result }); + req.ReplyLater(); } void WebSocketGameState::Broadcast(net::WebSocketServer *ws) { diff --git a/Core/Debugger/WebSocket/InputSubscriber.cpp b/Core/Debugger/WebSocket/InputSubscriber.cpp index 4eb324656a..d984585ccb 100644 --- a/Core/Debugger/WebSocket/InputSubscriber.cpp +++ b/Core/Debugger/WebSocket/InputSubscriber.cpp @@ -198,6 +198,7 @@ void WebSocketInputState::ButtonsPress(DebuggerRequest &req) { // same however fast the emulator runs. This thread only reports when it's over. press.pressId = __CtrlPressFor(press.button, (int)press.duration); pressTickets_.push_back(press); + req.ReplyLater(); } void WebSocketInputState::Broadcast(net::WebSocketServer *ws) { diff --git a/Core/Debugger/WebSocket/WebSocketUtils.h b/Core/Debugger/WebSocket/WebSocketUtils.h index 54f33026c4..d3e50f4e8b 100644 --- a/Core/Debugger/WebSocket/WebSocketUtils.h +++ b/Core/Debugger/WebSocket/WebSocketUtils.h @@ -50,14 +50,16 @@ struct WebSocketClientInfo { // later - a resume, a step, a button held for some frames. Deliberately not named after the // request: the event that reports the actual outcome uses that name, and for input.buttons.press // it carries the same ticket too, so anything less distinct would be impossible to tell apart. +// replyFollows says that outcome is a reply carrying the request's ticket (see ReplyLater()). struct DebuggerDeferredEvent { - DebuggerDeferredEvent(const char *n, const JsonGet data) : name(n) { + DebuggerDeferredEvent(const char *n, const JsonGet data, bool replyFollows) : name(n), replyFollows(replyFollows) { const JsonNode *value = data ? data.get("ticket") : nullptr; if (value) ticketRaw = json_stringify(value); } const char *name; + bool replyFollows; std::string ticketRaw; operator std::string() const { @@ -65,6 +67,9 @@ struct DebuggerDeferredEvent { j.begin(); j.writeString("event", "deferred"); j.writeString("for", name); + if (replyFollows) { + j.writeBool("replyFollows", true); + } if (!ticketRaw.empty()) { j.writeRaw("ticket", ticketRaw); } @@ -136,11 +141,21 @@ struct DebuggerRequest { void Flush(); bool Finish(); + // For a handler that answers later with a message carrying the request's ticket, rather than + // a ticketless broadcast. The deferred event then tells the client to keep waiting for it. + void ReplyLater() { + replyLater_ = true; + } + bool RepliesLater() const { + return replyLater_; + } + private: JsonWriter writer_; bool responseBegun_ = false; bool responseSent_ = false; bool responsePartial_ = false; + bool replyLater_ = false; }; class DebuggerSubscriber { diff --git a/Tools/wsdbg/README.md b/Tools/wsdbg/README.md index bdd2e90b4b..37a4c36bdd 100644 --- a/Tools/wsdbg/README.md +++ b/Tools/wsdbg/README.md @@ -93,6 +93,19 @@ trips would otherwise hang the script forever, so it gives up after `--sync-time (default 30), reports it, and makes the run exit non-zero. An `error` reply to a line doesn't stop the script, but makes the run exit non-zero at the end. +That includes `:nowait` lines, whose replies are still collected (for up to ten seconds at EOF or +`:quit`); one that never arrives fails the run too. + +Some requests answer twice: an acknowledgement saying the reply will follow, then the reply itself, +both with the request's ticket. `game.state.save`/`load`/`rewind`/`verify` with `wait=true` do, and +so do `input.buttons.press`, `gpu.stats.get` and `gpu.record.dump`. Both one-shot mode and `--sync` +wait for the reply, so a failed save shows up as an error. To start one and then let the CPU run +the frame that finishes it, send it with `:nowait`: + +``` +:nowait game.state.save path=checkpoint.ppst wait=true +cpu.runUntilTime relativeUs=100000 +``` Matching is by ticket, always - `--sync` never waits for "whatever message arrives next", which is what used to quietly desynchronise a script. So prefer the `key=value` form for nested parameters diff --git a/Tools/wsdbg/src/main.rs b/Tools/wsdbg/src/main.rs index c8d441878d..7d4b8a1b33 100644 --- a/Tools/wsdbg/src/main.rs +++ b/Tools/wsdbg/src/main.rs @@ -127,14 +127,41 @@ fn compact() -> bool { // Tickets of requests sent without waiting, whose reply hasn't arrived yet, so :quit can wait for // them. Global for the same reason as COMPACT: a reply can turn up inside any of the loops that // read the socket (a --sync wait, :sleep, ...), and they all go through print_incoming. -static PENDING: Mutex> = Mutex::new(BTreeSet::new()); +// `failed` records an error reply to one of them, or one that never came. +struct Pending { + tickets: BTreeSet, + failed: bool, +} + +static PENDING: Mutex = Mutex::new(Pending { tickets: BTreeSet::new(), failed: false }); fn add_pending(ticket: u64) { - PENDING.lock().unwrap().insert(ticket); + PENDING.lock().unwrap().tickets.insert(ticket); +} + +fn is_pending(ticket: u64) -> bool { + PENDING.lock().unwrap().tickets.contains(&ticket) } fn pending_count() -> usize { - PENDING.lock().unwrap().len() + PENDING.lock().unwrap().tickets.len() +} + +fn pending_failed() -> bool { + PENDING.lock().unwrap().failed +} + +// Whether `v` is the answer to the request with this ticket. A deferred event is one, unless it +// says replyFollows: then the real reply, carrying the same ticket, is still to come (a waited +// savestate operation, input.buttons.press, ...). +fn is_final_reply(v: &serde_json::Value, ticket: u64) -> bool { + v.get("ticket").and_then(|t| t.as_u64()) == Some(ticket) + && !(v.get("event").and_then(|e| e.as_str()) == Some("deferred") + && v.get("replyFollows").and_then(|r| r.as_bool()) == Some(true)) +} + +fn is_error(v: &serde_json::Value) -> bool { + v.get("event").and_then(|e| e.as_str()) == Some("error") } // The "> " prompt is noise in a piped script, and interleaves with incoming messages. @@ -164,6 +191,12 @@ fn next_ticket() -> u64 { TICKET.fetch_add(1, Ordering::Relaxed) } +// Keeps auto-assigned tickets clear of one a raw line picked itself, so a late reply to it +// can't be mistaken for the answer to a later command. +fn reserve_ticket(ticket: u64) { + TICKET.fetch_max(ticket.saturating_add(1), Ordering::Relaxed); +} + // A minimal shell-like tokenizer: splits on unquoted whitespace, and lets 'single' or "double" // quotes protect spaces (and each other) from being treated as a separator - e.g. // `logFormat="job func v0={v0} a0={a0}"` becomes one token, not four bogus ones that @@ -396,8 +429,14 @@ fn request_deferred_acks(socket: &mut WebSocket) -> Result<()> { fn print_incoming(text: &str) { let parsed = serde_json::from_str::(text); - if let Some(t) = parsed.as_ref().ok().and_then(|v| v.get("ticket")).and_then(|t| t.as_u64()) { - PENDING.lock().unwrap().remove(&t); + if let Ok(v) = &parsed + && let Some(t) = v.get("ticket").and_then(|t| t.as_u64()) + && is_final_reply(v, t) + { + let mut pending = PENDING.lock().unwrap(); + if pending.tickets.remove(&t) && is_error(v) { + pending.failed = true; + } } if compact() { // One line per message, event name first so a script can grep for it without having to @@ -510,7 +549,7 @@ fn is_would_block(e: &tungstenite::Error) -> bool { // With deferred acks turned on (see request_deferred_acks) every request is answered, including // the ones whose result arrives later, so waiting for this request's ticket beats sleeping for a // fixed --wait: a quick question returns immediately instead of padding every invocation, and a -// slow one isn't cut off early. +// slow one isn't cut off early. A deferred event that says replyFollows isn't the answer yet. fn run_one_shot( mut socket: WebSocket, json_text: String, @@ -530,9 +569,11 @@ fn run_one_shot( let mut answered = false; let mut failed = false; pump_until(&mut socket, deadline, |v| { - if ticket.is_some() && v.get("ticket").and_then(|t| t.as_u64()) == ticket { + if let Some(t) = ticket + && is_final_reply(v, t) + { answered = true; - failed |= v.get("event").and_then(|e| e.as_str()) == Some("error"); + failed |= is_error(v); } answered && wait_for_ticket }); @@ -620,6 +661,12 @@ fn handle_repl_line(socket: &mut WebSocket, line: &str) -> Result None, }; + if let Some(t) = ticket { + if is_pending(t) { + return Err(anyhow!("ticket {t} is still waiting for its reply - pick another")); + } + reserve_ticket(t); + } warn_if_stepping_muted(&event, obj); match ticket { Some(t) => println!("-> (ticket {t}) {line}"), @@ -647,7 +694,8 @@ fn handle_repl_line(socket: &mut WebSocket, line: &str) -> Result bool { let wants_stepping = RESUME_FAMILY.contains(&event); let mut got_ticket = false; + let mut accepted = false; let mut got_stepping = !wants_stepping; let mut failed = false; let deadline = Instant::now() + Duration::from_secs_f64(timeout_secs.max(0.0)); let done = pump_until(socket, deadline, |v| { - if v.get("ticket").and_then(|t| t.as_u64()) == Some(ticket) { + if is_final_reply(v, ticket) { got_ticket = true; // An error reply is a final answer - there'll be no cpu.stepping to follow. - if v.get("event").and_then(|e| e.as_str()) == Some("error") { + if is_error(v) { failed = true; got_stepping = true; } + } else if v.get("ticket").and_then(|t| t.as_u64()) == Some(ticket) { + accepted = true; } // Only counts once the request has been acknowledged, so a cpu.stepping still in flight // from something earlier can't be mistaken for this command's. @@ -687,7 +738,13 @@ fn wait_for_sync_response( if !done { eprintln!( "! --sync: timed out after {timeout_secs}s waiting for '{event}' (ticket {ticket}){}", - if got_ticket { ", acknowledged but never stopped" } else { "" } + if got_ticket { + ", acknowledged but never stopped" + } else if accepted { + ", accepted but the reply never came" + } else { + "" + } ); } done && !failed @@ -921,6 +978,7 @@ fn drain_pending(socket: &mut WebSocket, secs: f64) { pending_count(), secs ); + PENDING.lock().unwrap().failed = true; } } @@ -969,7 +1027,7 @@ fn run_repl(mut socket: WebSocket, sync: bool, sync_timeout: f64) -> match words.next() { Some(":quit") | Some(":q") | Some(":exit") => { drain_pending(&mut socket, 10.0); - return Ok(!failed); + return Ok(!failed && !pending_failed()); } Some(":help") | Some(":h") => print_help(), Some(":snapshot") => { @@ -1036,7 +1094,7 @@ fn run_repl(mut socket: WebSocket, sync: bool, sync_timeout: f64) -> Err(mpsc::TryRecvError::Disconnected) => { // Piped script hit EOF - same race as :quit. drain_pending(&mut socket, 10.0); - return Ok(!failed); + return Ok(!failed && !pending_failed()); } Err(mpsc::TryRecvError::Empty) => {} } @@ -1048,13 +1106,13 @@ fn run_repl(mut socket: WebSocket, sync: bool, sync_timeout: f64) -> } Ok(Message::Close(frame)) => { println!("\n[connection closed by PPSSPP: {frame:?}]"); - return Ok(!failed); + return Ok(!failed && !pending_failed()); } Ok(_) => {} Err(ref e) if is_would_block(e) => {} Err(tungstenite::Error::ConnectionClosed | tungstenite::Error::AlreadyClosed) => { println!("\n[connection closed]"); - return Ok(!failed); + return Ok(!failed && !pending_failed()); } Err(e) => { eprintln!("\n[connection error: {e}]"); diff --git a/docs/WebSocketDebugger.md b/docs/WebSocketDebugger.md index 9d38871071..ac6a0e8889 100644 --- a/docs/WebSocketDebugger.md +++ b/docs/WebSocketDebugger.md @@ -88,6 +88,18 @@ stay that way: an extra message would break a client that correlates purely by ticket, and it can't reuse the request's event name because for `input.buttons.press` that is exactly what the real, later answer looks like. +A `deferred` event with `"replyFollows": true` means the outcome will arrive as +a message carrying the request's ticket - the response, or an `error` - so keep +waiting for that ticket. That holds for `input.buttons.press`, +`gpu.stats.get`, `gpu.record.dump` and the `game.state.*` operations sent with +`wait`. Without the field, the outcome (if any) is a ticketless broadcast. + +```json +-> { "event": "game.state.save", "path": "a.ppst", "wait": true, "ticket": 8 } +<- { "event": "deferred", "for": "game.state.save", "replyFollows": true, "ticket": 8 } +<- { "event": "game.state.save", "message": "Saved State", "metadata": "", "ticket": 8 } +``` + Errors look like this: ```json { "event": "error", "message": "...", "level": 2, "ticket": 1 } diff --git a/docs/async-savestates.md b/docs/async-savestates.md index 70b742a14a..367a3e6f2b 100644 --- a/docs/async-savestates.md +++ b/docs/async-savestates.md @@ -184,5 +184,5 @@ Rewind deltas went from 4-13 MB to 1.3-3.8 MB each, at 10-24 ms of worker time. Headless, wsdbg and the `game.state.*` debugger commands cover all of it. `game.state.save` and `game.state.load` take a `slot` instead of a `path`, which goes through the slot code like the save and load keys: undo rotation, load undo, repeats. Use a scratch `--memstick` for that, not the app's, -or the tests overwrite real slots. wsdbg's `--sync` moves on when a request is acknowledged, not when -a `wait` result arrives, so put a `:sleep` between steps whose order matters, and one at the end. +or the tests overwrite real slots. With `wait=true`, wsdbg's `--sync` waits for the operation's result +(it finishes while the CPU is stepping too), and a failed one makes the run exit non-zero.