diff --git a/docs/gpu-attach-robustness-framing.md b/docs/gpu-attach-robustness-framing.md index 3f90ab5..91e1cbd 100644 --- a/docs/gpu-attach-robustness-framing.md +++ b/docs/gpu-attach-robustness-framing.md @@ -136,9 +136,19 @@ Landed as framed; all three in `pmacs-gpu`, no protocol/daemon change. same-kind `Viewport`/`Pointer{Drag}` (coalesce), appends everything else lossless, and on a lossless append past `OUTBOX_MAX` sets `closed` + returns `false` (fail-fast). The writer waits on the condvar, `mem::take`s - the whole batch, and writes with the lock released. 5 unit tests - (coalesce-to-latest, clicks keep order, same-kind-tail only, overflow - closes, coalescing is uncapped). + the whole batch, and writes with the lock released. + **Fail-fast is a real teardown, not just a flag** (review follow-up): + closing the outbox alone left the reader blocked on its still-open + socket clone, so the optimistic edit whose `send_crdt_op` failed was + applied locally, logged, and forgotten — silent divergence, the exact + stalled-daemon case F-008 targets. Now a `shutdown_handle` clone is + `shutdown(Both)` whenever the outbox closes (overflow in `send_event`, + or a writer write error): the reader wakes with EOF and fires the + existing `Disconnected` path (`(daemon disconnected)` in the window), + and the daemon sees the half-close. 6 unit tests (coalesce-to-latest, + clicks keep order, same-kind-tail only, overflow closes, coalescing is + uncapped, and a socketpair test asserting a closed outbox drives the + peer to EOF). - **F-007** (`main.rs`): a pure `mb_dropdown_window(n, selected, band_top) -> Option<(first, count)>` clamps the row count to what fits above the band (hides entirely when not even one row fits, so `top_y` is never @@ -150,9 +160,11 @@ Landed as framed; all three in `pmacs-gpu`, no protocol/daemon change. byte-identical to before. 1 unit test. Validated: `cargo fmt` clean; `clippy -p pmacs-gpu --all-targets` clean; -51 pmacs-gpu unit tests pass, including the two headless render tests on -this box's Vulkan adapter (`PMACS_REQUIRE_GPU=1`). No divergence from the -framing. +52 pmacs-gpu unit tests pass, including the two headless render tests on +this box's Vulkan adapter (`PMACS_REQUIRE_GPU=1`). Divergence from the +framing: the F-008 fail-fast needed an actual socket teardown, added as a +review follow-up (above); the framing's "clean disconnect" was otherwise +aspirational. **Still needs a human eyeball before merge** (per the validation implication above): attach to a non-CRDT daemon → the actionable banner; @@ -162,11 +174,13 @@ with the selection visible; and a normal attach still renders/resizes ## Deferred (named) -- **F-008 degraded banner.** Overflow-close currently surfaces only as - logged send errors (the reader thread still drives the visible - `Disconnected` status). A dedicated "daemon not keeping up — reconnect" - banner + auto-reconnect is deferred to the reconnect thread - ([[attach_reconnect]] already exists daemon-side for the TUI). +- **F-008 auto-reconnect / resync.** Overflow-close now actively tears the + session down and shows `(daemon disconnected)` (the review follow-up + above). What's still deferred is *recovery*: `pmacs-gpu` has no + auto-reconnect, so the diverged optimistic replica is reconciled only by + a fresh `BufferSnapshot` on a manual re-attach. A "daemon not keeping up + — reconnecting…" banner + automatic re-attach belongs with the reconnect + thread ([[attach_reconnect]] already exists daemon-side for the TUI). - **F-003 capability renegotiation.** We reject on missing caps; a friendlier flow would offer to relaunch the daemon with `crdt`. Out of scope — the frontend can't manage the daemon's lifecycle. diff --git a/pmacs-gpu/src/attach.rs b/pmacs-gpu/src/attach.rs index ca5f6df..ad4226d 100644 --- a/pmacs-gpu/src/attach.rs +++ b/pmacs-gpu/src/attach.rs @@ -276,6 +276,15 @@ pub fn connect( // buffer state — safe to read on one clone while the other writes // (the FD is full-duplex). let mut read_stream = stream.try_clone().map_err(AttachClientError::Connect)?; + // A third clone kept solely to *shut the socket down* (F-008). When the + // outbox closes — a lossless overflow, or a writer write error — we + // `shutdown(Both)` so the reader (blocked in `read_message` on its own + // clone) wakes with EOF and fires `Disconnected`. That routes the + // stall into the existing visible teardown instead of leaving the GPU + // showing locally-applied edits the daemon never received. Clones share + // the socket's file description, so a shutdown on any of them affects + // all — and it half-closes toward the daemon so it sees the departure. + let shutdown_handle = stream.try_clone().map_err(AttachClientError::Connect)?; let write_stream = stream; // Bounded, coalescing outbound queue (F-008) shared with the writer // thread; the `Condvar` wakes the writer when the UI thread enqueues. @@ -336,6 +345,10 @@ pub fn connect( if let Err(e) = write_message(&mut write_stream, &event) { eprintln!("pmacs-gpu: attach writer stopped: {e}"); lock.lock().expect("outbox lock").closed = true; + // Wake the reader (and signal the daemon) so the + // session tears down visibly (F-008) rather than + // leaving the reader blocked on a dead socket. + let _ = write_stream.shutdown(std::net::Shutdown::Both); return; } } @@ -345,6 +358,7 @@ pub fn connect( Ok(AttachClient { outbox, + shutdown_handle, frontend_id: hello.assigned_frontend_id, server_protocol_version: hello.protocol_version, }) @@ -357,6 +371,10 @@ pub struct AttachClient { /// thread. `send_event` locks it, applies the enqueue policy, and /// wakes the writer via the paired `Condvar`. outbox: Arc<(Mutex, Condvar)>, + /// Socket clone used only to `shutdown(Both)` when the outbox closes + /// (F-008), so the reader wakes and the session tears down visibly + /// instead of diverging silently. See [`connect`]. + shutdown_handle: UnixStream, /// Assigned by the daemon in the `Hello` response. Every /// `FrontendEvent` carries this so the daemon can route input back /// to the per-session `SemanticRenderState`. @@ -480,6 +498,15 @@ impl AttachClient { cvar.notify_one(); Ok(()) } else { + // Refused because the outbox is closed — a lossless overflow + // against a stalled daemon (or an earlier writer failure). + // Tear the session down actively (F-008): shut the socket so + // the reader wakes with EOF and fires `Disconnected`, giving a + // visible "(daemon disconnected)" instead of a GPU that keeps + // showing optimistic edits the daemon never received. Idempotent + // — a second shutdown just returns `NotConnected`, ignored. + drop(ob); + let _ = self.shutdown_handle.shutdown(std::net::Shutdown::Both); Err(TransportError::Io(std::io::Error::new( std::io::ErrorKind::BrokenPipe, "attach writer stopped or outbound queue overflowed", @@ -667,4 +694,32 @@ mod tests { assert_eq!(ob.queue.len(), 1); assert!(!ob.closed); } + + #[test] + fn a_closed_outbox_shuts_the_socket_down_to_wake_the_reader() { + use std::io::Read; + // A socketpair stands in for the daemon connection; `a` is the peer + // a blocked reader would be reading from. + let (mut a, b) = UnixStream::pair().expect("socketpair"); + // The post-overflow state: the outbox is already closed. + let mut outbox = Outbox::new(); + outbox.closed = true; + let client = AttachClient { + outbox: Arc::new((Mutex::new(outbox), Condvar::new())), + shutdown_handle: b, + frontend_id: FrontendId::LOCAL, + server_protocol_version: PROTOCOL_VERSION, + }; + // A send against the closed outbox fails *and* shuts the socket + // down (F-008 fail-fast is now a real teardown, not just a flag). + assert!(client.send_event(fe_key('x')).is_err()); + // The peer reads EOF: a real blocked reader would wake here and + // fire Disconnected, instead of the session diverging silently. + let mut buf = [0u8; 8]; + assert_eq!( + a.read(&mut buf).expect("read peer"), + 0, + "peer should see EOF after the shutdown" + ); + } }