f6a54d28
feat: transport death is a non-event — reconnect with have_seq, resume by delta
a73x 2026-08-08 14:08
Commit message
docs/decisions.md
| Old | New | ||
|---|---|---|---|
| @@ -444,6 +444,17 @@ path, and it is transport work, which is the verdict restated. | |||
| 444 | and it runs before the pty is spawned so a refusal costs no fork. This | 444 | and it runs before the pty is spawned so a refusal costs no fork. This |
| 445 | is the complementary half of deinit's existing unlink-only-if-still-ours | 445 | is the complementary half of deinit's existing unlink-only-if-still-ours |
| 446 | check: one guards the path on the way in, the other on the way out. | 446 | check: one guards the path on the way in, the other on the way out. |
| 447 | - **`AddressInUse` gets the same one-liner as a live socket.** Two daemons | ||
| 448 | starting at once can both see an empty path and both try to bind; the | ||
| 449 | loser has simply lost a dead heat, and by the time it reads "a daemon is | ||
| 450 | already running" one is. Knowingly, that message is also what a | ||
| 451 | *dangling symlink* at the socket path produces — connect through it | ||
| 452 | gets ENOENT so the probe reads the path as free, then bind gets | ||
| 453 | EADDRINUSE off the directory entry the symlink occupies. That is the | ||
| 454 | deterministic construction the test uses, and the wrong-ish message is | ||
| 455 | an accepted trade: distinguishing the two would need a | ||
| 456 | SYMLINK_NOFOLLOW re-stat purely to reword a rare operator mistake, and | ||
| 457 | the advice ("that path is not yours to take") is right either way. | ||
| 447 | - **Banked cleanup (post-M6, from the socket-claim review):** the systemd | 458 | - **Banked cleanup (post-M6, from the socket-claim review):** the systemd |
| 448 | `LISTEN_FDS` path has no automated test proving the probe is skipped | 459 | `LISTEN_FDS` path has no automated test proving the probe is skipped |
| 449 | (verified by hand only). `claimSockPath` still stack-traces on | 460 | (verified by hand only). `claimSockPath` still stack-traces on |
src/client.zig
| Old | New | ||
|---|---|---|---|
| @@ -21,6 +21,11 @@ fn onWinch(_: c_int) callconv(.c) void { | |||
| 21 | /// blindness is the point of the M6 spike. | 21 | /// blindness is the point of the M6 spike. |
| 22 | const Conn = struct { r: std.posix.fd_t, w: std.posix.fd_t }; | 22 | const Conn = struct { r: std.posix.fd_t, w: std.posix.fd_t }; |
| 23 | 23 | ||
| 24 | /// How long a reconnect keeps retrying an attach the daemon refuses. Long | ||
| 25 | /// enough to outlast the daemon reaping the slot our own dead connection | ||
| 26 | /// left behind, short enough that a genuinely full session still reports it. | ||
| 27 | const reconnect_grace_ms: i64 = 5000; | ||
| 28 | |||
| 24 | /// One live connection to a muxd, however it was reached. The point of the | 29 | /// One live connection to a muxd, however it was reached. The point of the |
| 25 | /// struct is that it can be closed and opened again from the same recipe | 30 | /// struct is that it can be closed and opened again from the same recipe |
| 26 | /// (`sock_path` or `via`), which is what lets a session outlive its | 31 | /// (`sock_path` or `via`), which is what lets a session outlive its |
| @@ -52,8 +57,18 @@ const Transport = struct { | |||
| 52 | return .{ .conn = .{ .r = stream.handle, .w = stream.handle } }; | 57 | return .{ .conn = .{ .r = stream.handle, .w = stream.handle } }; |
| 53 | } | 58 | } |
| 54 | 59 | ||
| 60 | /// Idempotent, and it has to be: reconnect() releases the dead transport | ||
| 61 | /// on entry, and if the user then aborts, attach()'s `defer | ||
| 62 | /// transport.close()` closes the very same value again. A second | ||
| 63 | /// close(2) on a stale fd is EBADF, which std.posix maps to | ||
| 64 | /// `unreachable` — a panic, not an error. `--via` hides that (killing a | ||
| 65 | /// reaped child is harmless), `--sock` does not, and `--sock` is exactly | ||
| 66 | /// the locally-killed-daemon case. | ||
| 55 | fn close(self: *Transport) void { | 67 | fn close(self: *Transport) void { |
| 68 | if (self.conn.r == -1) return; // already released | ||
| 69 | defer self.conn = .{ .r = -1, .w = -1 }; | ||
| 56 | if (self.child) |*c| { | 70 | if (self.child) |*c| { |
| 71 | defer self.child = null; | ||
| 57 | // Close stdin first so the command sees EOF and can wind down its | 72 | // Close stdin first so the command sees EOF and can wind down its |
| 58 | // remote end cleanly; then TERM it. kill() waitpid()s internally, | 73 | // remote end cleanly; then TERM it. kill() waitpid()s internally, |
| 59 | // so this also reaps — no zombie is left behind. | 74 | // so this also reaps — no zombie is left behind. |
| @@ -85,19 +100,15 @@ pub fn attach(alloc: std.mem.Allocator, sock_path: ?[]const u8, via: ?[]const u8 | |||
| 85 | } | 100 | } |
| 86 | 101 | ||
| 87 | /// `sock_path` and `via` are the recipe this session's transport was built | 102 | /// `sock_path` and `via` are the recipe this session's transport was built |
| 88 | /// from, carried so a future reconnect can rebuild it without unwinding back | 103 | /// from, carried so a reconnect can rebuild it in place without unwinding |
| 89 | /// to attach() and losing the replica. Nothing reads them yet — the | 104 | /// back to attach() — which would take the replica with it, and the replica |
| 90 | /// reconnect loop is the next task — which is why they are discarded | 105 | /// is the entire reason a dropped link can be a non-event. |
| 91 | /// explicitly below rather than silently ignored. | ||
| 92 | fn session( | 106 | fn session( |
| 93 | alloc: std.mem.Allocator, | 107 | alloc: std.mem.Allocator, |
| 94 | transport: *Transport, | 108 | transport: *Transport, |
| 95 | sock_path: ?[]const u8, | 109 | sock_path: ?[]const u8, |
| 96 | via: ?[]const u8, | 110 | via: ?[]const u8, |
| 97 | ) !u8 { | 111 | ) !u8 { |
| 98 | _ = sock_path; | ||
| 99 | _ = via; | ||
| 100 | |||
| 101 | // A daemon that dies mid-write must surface as an error return from | 112 | // A daemon that dies mid-write must surface as an error return from |
| 102 | // write(), not a fatal SIGPIPE. Zig's start.zig already installs a noop | 113 | // write(), not a fatal SIGPIPE. Zig's start.zig already installs a noop |
| 103 | // SIGPIPE handler, so this is defence in depth rather than the thing that | 114 | // SIGPIPE handler, so this is defence in depth rather than the thing that |
| @@ -177,17 +188,79 @@ fn session( | |||
| 177 | // from our tty (`size`); the replica follows the grid, the paint is | 188 | // from our tty (`size`); the replica follows the grid, the paint is |
| 178 | // clipped to the tty. | 189 | // clipped to the tty. |
| 179 | var grid = size; | 190 | var grid = size; |
| 180 | // Whether any session state has arrived. A refusal (session full) | 191 | // Whether state has arrived *on the current attach*. A refusal (session |
| 181 | // arrives as exit_status before anything else; without this we could | 192 | // full) arrives as exit_status before anything else; without this we |
| 182 | // not tell it apart from the shell exiting 1. | 193 | // could not tell it apart from the shell exiting 1. Per attach rather |
| 183 | var got_state = false; | 194 | // than per session because a reconnect produces the identical shape: |
| 184 | // The daemon instance we are talking to, learned from its snapshots. | 195 | // what we still hold from before the disconnect says nothing about |
| 185 | // Nothing reads it yet: this client always attaches fresh. Reconnect | 196 | // whether this attach was let in. |
| 186 | // logic (M6+) will send it back with a have_seq so the daemon can tell | 197 | var state_since_attach = false; |
| 187 | // whether that seq is one of its own. | 198 | // The daemon instance we are talking to, learned from its snapshots, |
| 199 | // and quoted back on reconnect so the daemon can tell whether the seq | ||
| 200 | // we hold is one of its own (a restarted daemon counts from zero over | ||
| 201 | // different content). | ||
| 188 | var session_epoch: u64 = 0; | 202 | var session_epoch: u64 = 0; |
| 203 | // The newest seq we hold. Sent with a reconnect attach; the daemon | ||
| 204 | // answers with a delta when it can still reach us from there. | ||
| 205 | var last_seq: u64 = 0; | ||
| 206 | // Set by any site that finds the transport dead; serviced at the top of | ||
| 207 | // the loop so the bookkeeping around a reconnect lives in one place. | ||
| 208 | var needs_reconnect = false; | ||
| 209 | // Absolute deadline for tolerating an attach refusal while reconnecting | ||
| 210 | // (the daemon may not have reaped our dead predecessor's slot yet). | ||
| 211 | // Set on the first reconnect, kept across its retries, cleared by state. | ||
| 212 | var reconnect_grace_until: ?i64 = null; | ||
| 213 | // The first paint after a reconnect must be a full one, so the | ||
| 214 | // [reconnecting] banner goes away with everything else now stale. | ||
| 215 | var repaint_after_resync = false; | ||
| 189 | var buf: [16 * 1024]u8 = undefined; | 216 | var buf: [16 * 1024]u8 = undefined; |
| 190 | while (true) { | 217 | while (true) { |
| 218 | if (needs_reconnect) { | ||
| 219 | needs_reconnect = false; | ||
| 220 | // Nothing to resume if there was never a session: a transport | ||
| 221 | // that died before the first snapshot carried none, so this is | ||
| 222 | // a bad host, a typo'd command, or a `muxd proxy` that exited — | ||
| 223 | // and retrying any of those helps nobody. Keep the old | ||
| 224 | // behaviour there: say it once and go. | ||
| 225 | // | ||
| 226 | // `session_epoch` is the right signal precisely because it is | ||
| 227 | // set from the first snapshot and never reset. | ||
| 228 | // `state_since_attach` cannot serve here: it is cleared on | ||
| 229 | // every re-attach, so mid-session it would send us down this | ||
| 230 | // exit path exactly when resuming is what we want. | ||
| 231 | if (session_epoch == 0) { | ||
| 232 | exit_msg = "mux: connection to muxd lost"; | ||
| 233 | return 1; | ||
| 234 | } | ||
| 235 | // A resync repaints live state, so a history page would be | ||
| 236 | // silently replaced a moment later — and the banner would sit | ||
| 237 | // over stale rows until the user happened to leave scroll mode. | ||
| 238 | // Every route into here shares that, so it is handled once. | ||
| 239 | scroll_pages = 0; | ||
| 240 | if (!reconnect( | ||
| 241 | alloc, | ||
| 242 | transport, | ||
| 243 | sock_path, | ||
| 244 | via, | ||
| 245 | size, | ||
| 246 | last_seq, | ||
| 247 | session_epoch, | ||
| 248 | stdin_fd, | ||
| 249 | stdout_fd, | ||
| 250 | is_tty, | ||
| 251 | )) { | ||
| 252 | // Ctrl-\ during a reconnect: the user is done waiting, but | ||
| 253 | // the session itself is still up wherever it lives. | ||
| 254 | exit_msg = "mux: detached while reconnecting (session still running; run mux to reattach)"; | ||
| 255 | return 0; | ||
| 256 | } | ||
| 257 | state_since_attach = false; | ||
| 258 | if (reconnect_grace_until == null) { | ||
| 259 | reconnect_grace_until = std.time.milliTimestamp() + reconnect_grace_ms; | ||
| 260 | } | ||
| 261 | repaint_after_resync = true; | ||
| 262 | continue; | ||
| 263 | } | ||
| 191 | if (winch_flag.swap(false, .acq_rel)) { | 264 | if (winch_flag.swap(false, .acq_rel)) { |
| 192 | if (ttySize(stdout_fd)) |new_size| { | 265 | if (ttySize(stdout_fd)) |new_size| { |
| 193 | if (new_size.cols != size.cols or new_size.rows != size.rows) { | 266 | if (new_size.cols != size.cols or new_size.rows != size.rows) { |
| @@ -202,8 +275,8 @@ fn session( | |||
| 202 | .resize, | 275 | .resize, |
| 203 | &proto.encodeSize(size.cols, size.rows), | 276 | &proto.encodeSize(size.cols, size.rows), |
| 204 | ) catch { | 277 | ) catch { |
| 205 | exit_msg = "mux: connection to muxd lost"; | 278 | needs_reconnect = true; |
| 206 | return 1; | 279 | continue; |
| 207 | }; | 280 | }; |
| 208 | } | 281 | } |
| 209 | } | 282 | } |
| @@ -225,8 +298,11 @@ fn session( | |||
| 225 | error.OutOfMemory => return err, | 298 | error.OutOfMemory => return err, |
| 226 | else => null, | 299 | else => null, |
| 227 | }) orelse { | 300 | }) orelse { |
| 228 | exit_msg = "mux: connection to muxd lost"; | 301 | // Not fatal any more: rebuild the transport and re-attach |
| 229 | return 1; | 302 | // with what we hold. The replica stays exactly as it is — |
| 303 | // that is the whole point of resuming rather than restarting. | ||
| 304 | needs_reconnect = true; | ||
| 305 | continue; | ||
| 230 | }; | 306 | }; |
| 231 | defer frame.deinit(alloc); | 307 | defer frame.deinit(alloc); |
| 232 | // The alternate screen waits for proof that the transport works. | 308 | // The alternate screen waits for proof that the transport works. |
| @@ -246,8 +322,10 @@ fn session( | |||
| 246 | // Set after the prefix parse here but before the compose | 322 | // Set after the prefix parse here but before the compose |
| 247 | // catch in the delta arm: a short snapshot proves nothing, | 323 | // catch in the delta arm: a short snapshot proves nothing, |
| 248 | // while a delta's arrival alone proves we were admitted. | 324 | // while a delta's arrival alone proves we were admitted. |
| 249 | got_state = true; | 325 | state_since_attach = true; |
| 326 | reconnect_grace_until = null; | ||
| 250 | session_epoch = prefix.epoch; | 327 | session_epoch = prefix.epoch; |
| 328 | last_seq = prefix.seq; | ||
| 251 | history_rows = prefix.history_rows; | 329 | history_rows = prefix.history_rows; |
| 252 | if (prefix.cols != grid.cols or prefix.rows != grid.rows) { | 330 | if (prefix.cols != grid.cols or prefix.rows != grid.rows) { |
| 253 | try replica.resize(prefix.cols, prefix.rows); | 331 | try replica.resize(prefix.cols, prefix.rows); |
| @@ -257,16 +335,27 @@ fn session( | |||
| 257 | // scrolled, so leaving scroll mode paints current state. | 335 | // scrolled, so leaving scroll mode paints current state. |
| 258 | replica.reset(); | 336 | replica.reset(); |
| 259 | replica.feed(frame.payload[proto.snapshot_prefix_len..]); | 337 | replica.feed(frame.payload[proto.snapshot_prefix_len..]); |
| 260 | if (scroll_pages == 0) try renderClipped(alloc, replica, size, stdout_fd); | 338 | if (scroll_pages == 0) { |
| 339 | try renderClipped(alloc, replica, size, stdout_fd); | ||
| 340 | repaint_after_resync = false; // banner painted over | ||
| 341 | } | ||
| 261 | }, | 342 | }, |
| 262 | .delta => { | 343 | .delta => { |
| 263 | got_state = true; | 344 | state_since_attach = true; |
| 345 | reconnect_grace_until = null; | ||
| 264 | const composed = proto.composeDelta(alloc, frame.payload) catch { | 346 | const composed = proto.composeDelta(alloc, frame.payload) catch { |
| 265 | // A rejected delta means the replica can no longer be | 347 | // A rejected delta means the replica can no longer be |
| 266 | // trusted; ask for a fresh snapshot rather than | 348 | // trusted; ask for a fresh snapshot rather than |
| 267 | // silently skipping it and desyncing for good. By | 349 | // silently skipping it and desyncing for good. By |
| 268 | // latest-wins this re-attach also re-asserts our size | 350 | // latest-wins this re-attach also re-asserts our size |
| 269 | // onto the shared session — accepted. | 351 | // onto the shared session — accepted. |
| 352 | // | ||
| 353 | // Deliberately NOT the reconnect path: the transport | ||
| 354 | // is alive here, and have_seq stays 0 because the | ||
| 355 | // whole problem is that what we hold is untrusted — | ||
| 356 | // quoting a seq would invite the delta that cannot | ||
| 357 | // fix us. A reconnect quotes last_seq for exactly the | ||
| 358 | // opposite reason: there, the replica is known good. | ||
| 270 | proto.writeFrame( | 359 | proto.writeFrame( |
| 271 | transport.conn.w, | 360 | transport.conn.w, |
| 272 | .attach, | 361 | .attach, |
| @@ -276,10 +365,22 @@ fn session( | |||
| 276 | }; | 365 | }; |
| 277 | defer alloc.free(composed.bytes); | 366 | defer alloc.free(composed.bytes); |
| 278 | history_rows = composed.header.history_rows; | 367 | history_rows = composed.header.history_rows; |
| 368 | last_seq = composed.header.seq; | ||
| 279 | replica.feed(composed.bytes); | 369 | replica.feed(composed.bytes); |
| 280 | // While scrolled the replica still tracks live output; the | 370 | // While scrolled the replica still tracks live output; the |
| 281 | // repaint on scroll exit comes from it. | 371 | // repaint on scroll exit comes from it. |
| 282 | if (scroll_pages == 0) try paintDeltaClipped(alloc, frame.payload, size, stdout_fd); | 372 | if (scroll_pages == 0) { |
| 373 | if (repaint_after_resync) { | ||
| 374 | // First frame back after a reconnect. The daemon | ||
| 375 | // sent only what changed, which is correct — but | ||
| 376 | // the screen still carries the banner, so repaint | ||
| 377 | // the whole thing from the replica instead. | ||
| 378 | try renderClipped(alloc, replica, size, stdout_fd); | ||
| 379 | repaint_after_resync = false; | ||
| 380 | } else { | ||
| 381 | try paintDeltaClipped(alloc, frame.payload, size, stdout_fd); | ||
| 382 | } | ||
| 383 | } | ||
| 283 | }, | 384 | }, |
| 284 | .scrollback_chunk => { | 385 | .scrollback_chunk => { |
| 285 | if (scroll_pages == 0 or frame.payload.len < 6) continue; | 386 | if (scroll_pages == 0 or frame.payload.len < 6) continue; |
| @@ -289,10 +390,23 @@ fn session( | |||
| 289 | // Before any session state, exit_status is almost always | 390 | // Before any session state, exit_status is almost always |
| 290 | // the daemon refusing the attach — say so, or it looks | 391 | // the daemon refusing the attach — say so, or it looks |
| 291 | // exactly like the shell itself exiting non-zero. | 392 | // exactly like the shell itself exiting non-zero. |
| 292 | if (!got_state) { | 393 | if (!state_since_attach) { |
| 394 | // Right after a reconnect the usual cause is the | ||
| 395 | // daemon not having reaped our dead predecessor's | ||
| 396 | // slot yet, so the session is full of *us*. Worth a | ||
| 397 | // few seconds of retrying rather than an exit the | ||
| 398 | // user has to undo by hand. | ||
| 399 | if (reconnect_grace_until) |until| { | ||
| 400 | if (std.time.milliTimestamp() < until) { | ||
| 401 | needs_reconnect = true; | ||
| 402 | continue; | ||
| 403 | } | ||
| 404 | } | ||
| 293 | exit_msg = "mux: attach refused or no state received (session full?)"; | 405 | exit_msg = "mux: attach refused or no state received (session full?)"; |
| 294 | return 1; | 406 | return 1; |
| 295 | } | 407 | } |
| 408 | // After state, this is the shell itself exiting — even | ||
| 409 | // if we reconnected moments ago. Never retried. | ||
| 296 | return if (frame.payload.len >= 1) frame.payload[0] else 0; | 410 | return if (frame.payload.len >= 1) frame.payload[0] else 0; |
| 297 | }, | 411 | }, |
| 298 | .taken_over => { | 412 | .taken_over => { |
| @@ -321,8 +435,8 @@ fn session( | |||
| 321 | const max_pages: u32 = (history_rows + size.rows - 1) / size.rows; | 435 | const max_pages: u32 = (history_rows + size.rows - 1) / size.rows; |
| 322 | if (scroll_pages < max_pages) scroll_pages += 1; | 436 | if (scroll_pages < max_pages) scroll_pages += 1; |
| 323 | requestScrollPage(transport.conn.w, scroll_pages, history_rows, size) catch { | 437 | requestScrollPage(transport.conn.w, scroll_pages, history_rows, size) catch { |
| 324 | exit_msg = "mux: connection to muxd lost"; | 438 | needs_reconnect = true; |
| 325 | return 1; | 439 | continue; |
| 326 | }; | 440 | }; |
| 327 | } | 441 | } |
| 328 | } else if (std.mem.eql(u8, buf[0..n], scroll_dn)) { | 442 | } else if (std.mem.eql(u8, buf[0..n], scroll_dn)) { |
| @@ -331,8 +445,8 @@ fn session( | |||
| 331 | try renderClipped(alloc, replica, size, stdout_fd); | 445 | try renderClipped(alloc, replica, size, stdout_fd); |
| 332 | } else { | 446 | } else { |
| 333 | requestScrollPage(transport.conn.w, scroll_pages, history_rows, size) catch { | 447 | requestScrollPage(transport.conn.w, scroll_pages, history_rows, size) catch { |
| 334 | exit_msg = "mux: connection to muxd lost"; | 448 | needs_reconnect = true; |
| 335 | return 1; | 449 | continue; |
| 336 | }; | 450 | }; |
| 337 | } | 451 | } |
| 338 | } else if (scroll_pages > 0) { | 452 | } else if (scroll_pages > 0) { |
| @@ -341,8 +455,11 @@ fn session( | |||
| 341 | try renderClipped(alloc, replica, size, stdout_fd); | 455 | try renderClipped(alloc, replica, size, stdout_fd); |
| 342 | } else { | 456 | } else { |
| 343 | proto.writeFrame(transport.conn.w, .input, buf[0..n]) catch { | 457 | proto.writeFrame(transport.conn.w, .input, buf[0..n]) catch { |
| 344 | exit_msg = "mux: connection to muxd lost"; | 458 | // These keystrokes are lost with the transport, by |
| 345 | return 1; | 459 | // the same policy that drops what is typed while |
| 460 | // disconnected. | ||
| 461 | needs_reconnect = true; | ||
| 462 | continue; | ||
| 346 | }; | 463 | }; |
| 347 | } | 464 | } |
| 348 | } | 465 | } |
| @@ -430,6 +547,113 @@ fn requestScrollPage( | |||
| 430 | try proto.writeFrame(w, .fetch_scrollback, &proto.encodeScrollbackReq(start, size.rows)); | 547 | try proto.writeFrame(w, .fetch_scrollback, &proto.encodeScrollbackReq(start, size.rows)); |
| 431 | } | 548 | } |
| 432 | 549 | ||
| 550 | /// An inverse status marker parked in the top-right corner: `[scroll]` when | ||
| 551 | /// viewing history, `[reconnecting]` when the transport is being rebuilt. | ||
| 552 | /// Text only, so a caller mid-repaint can append it into its own paint | ||
| 553 | /// buffer and keep the whole screen one synchronized update. | ||
| 554 | fn bannerText(buf: []u8, size: proto.Size, label: []const u8) ![]const u8 { | ||
| 555 | // Columns are 1-based; a label wider than the whole tty would otherwise | ||
| 556 | // address column 0, which terminals only silently forgive. | ||
| 557 | const col = @max(1, size.cols -| @as(u16, @intCast(label.len))); | ||
| 558 | return std.fmt.bufPrint(buf, "\x1b[1;{d}H\x1b[7m{s}\x1b[0m", .{ col, label }); | ||
| 559 | } | ||
| 560 | |||
| 561 | /// Drop a banner onto a screen that is otherwise staying put — the cursor is | ||
| 562 | /// saved and restored around it, so the shell's cursor does not visibly jump | ||
| 563 | /// to the corner. Best-effort: a status marker is never worth failing over. | ||
| 564 | fn paintBanner(out_fd: std.posix.fd_t, size: proto.Size, label: []const u8) void { | ||
| 565 | var buf: [96]u8 = undefined; | ||
| 566 | const mark = bannerText(&buf, size, label) catch return; | ||
| 567 | var paint: [128]u8 = undefined; | ||
| 568 | const text = std.fmt.bufPrint(&paint, "\x1b[s{s}\x1b[u", .{mark}) catch return; | ||
| 569 | proto.writeAllFd(out_fd, text) catch {}; | ||
| 570 | } | ||
| 571 | |||
| 572 | /// Wait up to `timeout_ms` for the user to give up on a reconnect. Input | ||
| 573 | /// typed while disconnected is read and dropped by policy — replaying a | ||
| 574 | /// burst of stale keystrokes into the shell on resume is worse than losing | ||
| 575 | /// them — but Ctrl-\ still means detach, so it is the one byte worth | ||
| 576 | /// watching for. Returns true iff it arrived. | ||
| 577 | fn drainStdinForQuit(stdin_fd: std.posix.fd_t, timeout_ms: u64) bool { | ||
| 578 | var timer = std.time.Timer.start() catch return false; | ||
| 579 | while (true) { | ||
| 580 | const elapsed = timer.read() / std.time.ns_per_ms; | ||
| 581 | if (elapsed >= timeout_ms) return false; | ||
| 582 | const remaining: i32 = @intCast(@min(timeout_ms - elapsed, std.math.maxInt(i32))); | ||
| 583 | var fds = [_]std.posix.pollfd{ | ||
| 584 | .{ .fd = stdin_fd, .events = std.posix.POLL.IN, .revents = 0 }, | ||
| 585 | }; | ||
| 586 | const ready = std.posix.poll(&fds, remaining) catch return false; | ||
| 587 | if (ready == 0) return false; // waited the backoff out, nothing typed | ||
| 588 | var buf: [1024]u8 = undefined; | ||
| 589 | const n = std.posix.read(stdin_fd, &buf) catch return false; | ||
| 590 | if (n == 0) { | ||
| 591 | // Closed stdin stays readable forever, so polling it again would | ||
| 592 | // spin instead of pacing the retries. Sleep out the rest of the | ||
| 593 | // budget: a piped client has nobody left to press Ctrl-\ anyway. | ||
| 594 | std.Thread.sleep((timeout_ms - elapsed) * std.time.ns_per_ms); | ||
| 595 | return false; | ||
| 596 | } | ||
| 597 | if (std.mem.indexOfScalar(u8, buf[0..n], 0x1c) != null) return true; | ||
| 598 | // Anything else is dropped, and we keep waiting out the backoff — | ||
| 599 | // returning early here would collapse the pacing the moment the | ||
| 600 | // user touched a key. | ||
| 601 | } | ||
| 602 | } | ||
| 603 | |||
| 604 | /// The transport died but the session behind it very likely did not. Keep | ||
| 605 | /// the replica, rebuild the pipe, and re-attach quoting what we already | ||
| 606 | /// hold: the daemon answers with a delta when it can still interpret our | ||
| 607 | /// seq, a full snapshot when it cannot (sendResync). Returns false only | ||
| 608 | /// when the user gave up. | ||
| 609 | /// | ||
| 610 | /// There is deliberately no retry cap. The user has both an abort key and a | ||
| 611 | /// visible indicator, and a laptop asleep for an hour resuming on wake is | ||
| 612 | /// the use case, not an edge case. | ||
| 613 | fn reconnect( | ||
| 614 | alloc: std.mem.Allocator, | ||
| 615 | transport: *Transport, | ||
| 616 | sock_path: ?[]const u8, | ||
| 617 | via: ?[]const u8, | ||
| 618 | size: proto.Size, | ||
| 619 | last_seq: u64, | ||
| 620 | session_epoch: u64, | ||
| 621 | stdin_fd: std.posix.fd_t, | ||
| 622 | stdout_fd: std.posix.fd_t, | ||
| 623 | is_tty: bool, | ||
| 624 | ) bool { | ||
| 625 | if (is_tty) paintBanner(stdout_fd, size, "[reconnecting]"); | ||
| 626 | // The dead transport is released exactly once, here. Everything after | ||
| 627 | // this point owns only what it opened itself, which is what keeps the | ||
| 628 | // retry loop from closing the same fd twice. | ||
| 629 | transport.close(); | ||
| 630 | |||
| 631 | var backoff_ms: u64 = 200; | ||
| 632 | while (true) { | ||
| 633 | if (drainStdinForQuit(stdin_fd, backoff_ms)) return false; | ||
| 634 | backoff_ms = @min(backoff_ms * 2, 2000); | ||
| 635 | |||
| 636 | var fresh = Transport.open(alloc, sock_path, via) catch continue; | ||
| 637 | proto.writeFrame( | ||
| 638 | fresh.conn.w, | ||
| 639 | .attach, | ||
| 640 | &proto.encodeAttach(size.cols, size.rows, last_seq, session_epoch), | ||
| 641 | ) catch { | ||
| 642 | // Ours, and already broken: close it here rather than letting | ||
| 643 | // the next iteration do it, which would otherwise be closing | ||
| 644 | // whatever the previous round left behind. | ||
| 645 | fresh.close(); | ||
| 646 | continue; | ||
| 647 | }; | ||
| 648 | // Overwrite the WHOLE value, never just `.conn`: close() left conn | ||
| 649 | // holding stale fds and `child` a corpse with its streams nulled, | ||
| 650 | // so patching the fds in place would strand that child — one leaked | ||
| 651 | // process per reconnect. | ||
| 652 | transport.* = fresh; | ||
| 653 | return true; // the main loop resumes; the next frame is the resync | ||
| 654 | } | ||
| 655 | } | ||
| 656 | |||
| 433 | /// Paint a fetched history page: clear, rows, and an inverse [scroll] | 657 | /// Paint a fetched history page: clear, rows, and an inverse [scroll] |
| 434 | /// marker top-right so the user knows they're not live. | 658 | /// marker top-right so the user knows they're not live. |
| 435 | fn renderScrollback( | 659 | fn renderScrollback( |
| @@ -442,15 +666,127 @@ fn renderScrollback( | |||
| 442 | defer paint.deinit(alloc); | 666 | defer paint.deinit(alloc); |
| 443 | try paint.appendSlice(alloc, "\x1b[?2026h\x1b[?25l\x1b[H\x1b[2J"); | 667 | try paint.appendSlice(alloc, "\x1b[?2026h\x1b[?25l\x1b[H\x1b[2J"); |
| 444 | try paint.appendSlice(alloc, rows_vt); | 668 | try paint.appendSlice(alloc, rows_vt); |
| 445 | var mark_buf: [64]u8 = undefined; | 669 | var mark_buf: [96]u8 = undefined; |
| 446 | const mark = try std.fmt.bufPrint(&mark_buf, "\x1b[1;{d}H\x1b[7m[scroll]\x1b[0m", .{ | 670 | try paint.appendSlice(alloc, try bannerText(&mark_buf, size, "[scroll]")); |
| 447 | size.cols -| 8, | ||
| 448 | }); | ||
| 449 | try paint.appendSlice(alloc, mark); | ||
| 450 | try paint.appendSlice(alloc, "\x1b[?2026l"); | 671 | try paint.appendSlice(alloc, "\x1b[?2026l"); |
| 451 | try proto.writeAllFd(out_fd, paint.items); | 672 | try proto.writeAllFd(out_fd, paint.items); |
| 452 | } | 673 | } |
| 453 | 674 | ||
| 675 | test "Transport.close is idempotent: the abort path closes what reconnect already closed" { | ||
| 676 | const alloc = std.testing.allocator; | ||
| 677 | |||
| 678 | var tmp = std.testing.tmpDir(.{}); | ||
| 679 | defer tmp.cleanup(); | ||
| 680 | var path_buf: [256]u8 = undefined; | ||
| 681 | const dir_path = try tmp.dir.realpath(".", &path_buf); | ||
| 682 | const sock_path = try std.fmt.allocPrint(alloc, "{s}/t.sock", .{dir_path}); | ||
| 683 | defer alloc.free(sock_path); | ||
| 684 | |||
| 685 | const addr = try std.net.Address.initUnix(sock_path); | ||
| 686 | var listener = try addr.listen(.{}); | ||
| 687 | defer listener.deinit(); | ||
| 688 | |||
| 689 | var transport = try Transport.open(alloc, sock_path, null); | ||
| 690 | |||
| 691 | // reconnect() closes the dead transport at entry; if the user then aborts, | ||
| 692 | // attach()'s `defer transport.close()` closes it a second time. Without a | ||
| 693 | // sentinel that is close(2) on a stale fd — EBADF, which std.posix.close | ||
| 694 | // maps to `unreachable`, i.e. a panic, and on --sock (the locally-killed | ||
| 695 | // daemon case) nothing masks it. | ||
| 696 | transport.close(); | ||
| 697 | transport.close(); | ||
| 698 | transport.close(); | ||
| 699 | try std.testing.expectEqual(@as(std.posix.fd_t, -1), transport.conn.r); | ||
| 700 | } | ||
| 701 | |||
| 702 | test "paintBanner parks an inverse label top-right without moving the cursor" { | ||
| 703 | const pipe = try std.posix.pipe(); | ||
| 704 | defer std.posix.close(pipe[0]); | ||
| 705 | paintBanner(pipe[1], .{ .cols = 80, .rows = 24 }, "[reconnecting]"); | ||
| 706 | std.posix.close(pipe[1]); | ||
| 707 | |||
| 708 | var out: [256]u8 = undefined; | ||
| 709 | const n = try std.posix.read(pipe[0], &out); | ||
| 710 | const text = out[0..n]; | ||
| 711 | // Row 1, right-aligned: 80 columns less the label's own width. | ||
| 712 | try std.testing.expect(std.mem.indexOf(u8, text, "\x1b[1;66H") != null); | ||
| 713 | try std.testing.expect(std.mem.indexOf(u8, text, "\x1b[7m[reconnecting]\x1b[0m") != null); | ||
| 714 | // Saved and restored around the paint, so the shell's cursor does not | ||
| 715 | // visibly jump into the corner while we reconnect. | ||
| 716 | try std.testing.expect(std.mem.startsWith(u8, text, "\x1b[s")); | ||
| 717 | try std.testing.expect(std.mem.endsWith(u8, text, "\x1b[u")); | ||
| 718 | } | ||
| 719 | |||
| 720 | test "paintBanner on a narrow tty clamps to column 1 instead of underflowing" { | ||
| 721 | const pipe = try std.posix.pipe(); | ||
| 722 | defer std.posix.close(pipe[0]); | ||
| 723 | // Label longer than the whole terminal: the saturating subtraction must | ||
| 724 | // land on column 1, never wrap around to a huge column. | ||
| 725 | paintBanner(pipe[1], .{ .cols = 4, .rows = 24 }, "[reconnecting]"); | ||
| 726 | std.posix.close(pipe[1]); | ||
| 727 | |||
| 728 | var out: [256]u8 = undefined; | ||
| 729 | const n = try std.posix.read(pipe[0], &out); | ||
| 730 | try std.testing.expect(std.mem.indexOf(u8, out[0..n], "\x1b[1;1H") != null); | ||
| 731 | } | ||
| 732 | |||
| 733 | test "drainStdinForQuit: the quit byte is seen, other input is dropped" { | ||
| 734 | // Ctrl-\ anywhere in what was typed means the user gave up. | ||
| 735 | { | ||
| 736 | const pipe = try std.posix.pipe(); | ||
| 737 | defer std.posix.close(pipe[0]); | ||
| 738 | _ = try std.posix.write(pipe[1], "ab\x1ccd"); | ||
| 739 | defer std.posix.close(pipe[1]); | ||
| 740 | try std.testing.expect(drainStdinForQuit(pipe[0], 5000)); | ||
| 741 | } | ||
| 742 | |||
| 743 | // Ordinary keystrokes are consumed and discarded — not queued for | ||
| 744 | // replay into the shell — and do not cut the backoff short. | ||
| 745 | { | ||
| 746 | const pipe = try std.posix.pipe(); | ||
| 747 | defer std.posix.close(pipe[0]); | ||
| 748 | _ = try std.posix.write(pipe[1], "hello"); | ||
| 749 | defer std.posix.close(pipe[1]); | ||
| 750 | |||
| 751 | var timer = try std.time.Timer.start(); | ||
| 752 | try std.testing.expect(!drainStdinForQuit(pipe[0], 200)); | ||
| 753 | const elapsed_ms = timer.read() / std.time.ns_per_ms; | ||
| 754 | // It waited the budget out rather than returning the moment a key | ||
| 755 | // arrived; without that, typing would spin the retry loop. | ||
| 756 | try std.testing.expect(elapsed_ms >= 150); | ||
| 757 | |||
| 758 | // And the bytes are gone: nothing is left to be read back. | ||
| 759 | var fds = [_]std.posix.pollfd{ | ||
| 760 | .{ .fd = pipe[0], .events = std.posix.POLL.IN, .revents = 0 }, | ||
| 761 | }; | ||
| 762 | try std.testing.expectEqual(@as(usize, 0), try std.posix.poll(&fds, 0)); | ||
| 763 | } | ||
| 764 | } | ||
| 765 | |||
| 766 | test "drainStdinForQuit: silence waits out the timeout and gives up" { | ||
| 767 | const pipe = try std.posix.pipe(); | ||
| 768 | defer std.posix.close(pipe[0]); | ||
| 769 | defer std.posix.close(pipe[1]); // held open: no EOF, just nothing typed | ||
| 770 | |||
| 771 | var timer = try std.time.Timer.start(); | ||
| 772 | try std.testing.expect(!drainStdinForQuit(pipe[0], 200)); | ||
| 773 | const elapsed_ms = timer.read() / std.time.ns_per_ms; | ||
| 774 | try std.testing.expect(elapsed_ms >= 150); | ||
| 775 | } | ||
| 776 | |||
| 777 | test "drainStdinForQuit: closed stdin still paces the retry instead of spinning" { | ||
| 778 | const pipe = try std.posix.pipe(); | ||
| 779 | defer std.posix.close(pipe[0]); | ||
| 780 | std.posix.close(pipe[1]); // EOF: readable forever, with nothing to read | ||
| 781 | |||
| 782 | var timer = try std.time.Timer.start(); | ||
| 783 | try std.testing.expect(!drainStdinForQuit(pipe[0], 200)); | ||
| 784 | const elapsed_ms = timer.read() / std.time.ns_per_ms; | ||
| 785 | // The point of the EOF branch: a piped client that has run out of input | ||
| 786 | // must not turn every reconnect backoff into a busy loop. | ||
| 787 | try std.testing.expect(elapsed_ms >= 150); | ||
| 788 | } | ||
| 789 | |||
| 454 | test "renderScrollback paints rows with an inverse scroll marker" { | 790 | test "renderScrollback paints rows with an inverse scroll marker" { |
| 455 | const alloc = std.testing.allocator; | 791 | const alloc = std.testing.allocator; |
| 456 | const pipe = try std.posix.pipe(); | 792 | const pipe = try std.posix.pipe(); |
test/e2e.sh
| Old | New | ||
|---|---|---|---|
| @@ -6,8 +6,17 @@ MUXD="$1" | |||
| 6 | MUX="$2" | 6 | MUX="$2" |
| 7 | SOCK="${TMPDIR:-/tmp}/muxd-e2e-$$.sock" | 7 | SOCK="${TMPDIR:-/tmp}/muxd-e2e-$$.sock" |
| 8 | OUT="${TMPDIR:-/tmp}/mux-e2e-out-$$" | 8 | OUT="${TMPDIR:-/tmp}/mux-e2e-out-$$" |
| 9 | # Second daemon, used only by the M7 abort scenario; declared here so the | ||
| 10 | # trap below can reference them under `set -u` before they are ever started. | ||
| 11 | SOCK2="${TMPDIR:-/tmp}/muxd-e2e-abort-$$.sock" | ||
| 12 | D2PID="" | ||
| 9 | 13 | ||
| 10 | cleanup() { kill "$DPID" 2>/dev/null || true; rm -f "$SOCK" "$OUT" "$OUT.kill" "$OUT.re" "$OUT.a" "$OUT.b" "$OUT.via"; } | 14 | cleanup() { |
| 15 | kill "$DPID" 2>/dev/null || true | ||
| 16 | [ -n "$D2PID" ] && kill "$D2PID" 2>/dev/null | ||
| 17 | rm -f "$SOCK" "$SOCK2" "$OUT" "$OUT.kill" "$OUT.re" "$OUT.a" "$OUT.b" \ | ||
| 18 | "$OUT.via" "$OUT.dead" "$OUT.abort" | ||
| 19 | } | ||
| 11 | trap cleanup EXIT INT TERM | 20 | trap cleanup EXIT INT TERM |
| 12 | 21 | ||
| 13 | "$MUXD" run --sock "$SOCK" --shell /bin/sh & | 22 | "$MUXD" run --sock "$SOCK" --shell /bin/sh & |
| @@ -88,4 +97,52 @@ grep -q "m6-via-pipe" "$OUT.via" || { | |||
| 88 | kill -0 "$DPID" || { echo "e2e FAIL: daemon died in --via scenario"; exit 1; } | 97 | kill -0 "$DPID" || { echo "e2e FAIL: daemon died in --via scenario"; exit 1; } |
| 89 | rm -f "$OUT.via" | 98 | rm -f "$OUT.via" |
| 90 | 99 | ||
| 100 | # --- M7: a transport that dies before any session must exit, not retry. The | ||
| 101 | # reconnect loop resumes sessions; it must not turn a bad --via command into | ||
| 102 | # an unkillable client. With piped stdin there is no Ctrl-\ to rescue it, so | ||
| 103 | # a regression here hangs forever: exit 124 below is the timeout, and it is | ||
| 104 | # the failure this test exists to catch. | ||
| 105 | set +e | ||
| 106 | timeout 10 "$MUX" --via "exit 7" < /dev/null > "$OUT.dead" 2>&1 | ||
| 107 | RC=$? | ||
| 108 | set -e | ||
| 109 | [ "$RC" -eq 1 ] || { | ||
| 110 | echo "e2e FAIL: dead first transport exited $RC (want 1; 124 means it hung retrying)" | ||
| 111 | cat "$OUT.dead"; exit 1; | ||
| 112 | } | ||
| 113 | grep -q "connection to muxd lost" "$OUT.dead" || { | ||
| 114 | echo "e2e FAIL: dead first transport lost its diagnostic; got:"; cat "$OUT.dead"; exit 1; | ||
| 115 | } | ||
| 116 | rm -f "$OUT.dead" | ||
| 117 | |||
| 118 | # --- M7: aborting a reconnect exits cleanly. The client establishes a real | ||
| 119 | # session (so reconnect is allowed), its daemon is then killed under it, and | ||
| 120 | # Ctrl-\ arrives while it is retrying. Exit 0 and the message; a double-close | ||
| 121 | # of the transport would abort here instead (SIGABRT = 134). | ||
| 122 | "$MUXD" run --sock "$SOCK2" --shell /bin/sh & | ||
| 123 | D2PID=$! | ||
| 124 | i=0 | ||
| 125 | while [ ! -S "$SOCK2" ] && [ "$i" -lt 50 ]; do sleep 0.1; i=$((i+1)); done | ||
| 126 | [ -S "$SOCK2" ] || { echo "e2e FAIL: second socket never appeared"; exit 1; } | ||
| 127 | |||
| 128 | set +e | ||
| 129 | { printf 'echo m7-abort-live\n'; sleep 3; printf '\034'; sleep 2; } | \ | ||
| 130 | timeout 20 "$MUX" --sock "$SOCK2" > "$OUT.abort" 2>&1 & | ||
| 131 | CLIPID=$! | ||
| 132 | # Let the session establish, then take the daemon away mid-session. | ||
| 133 | sleep 2 | ||
| 134 | kill -9 "$D2PID" 2>/dev/null | ||
| 135 | D2PID="" | ||
| 136 | wait "$CLIPID" | ||
| 137 | RC=$? | ||
| 138 | set -e | ||
| 139 | [ "$RC" -eq 0 ] || { | ||
| 140 | echo "e2e FAIL: abort during reconnect exited $RC (want 0; 134 = panic, 124 = hung)" | ||
| 141 | cat "$OUT.abort"; exit 1; | ||
| 142 | } | ||
| 143 | grep -q "detached while reconnecting" "$OUT.abort" || { | ||
| 144 | echo "e2e FAIL: abort during reconnect lost its message; got:"; cat "$OUT.abort"; exit 1; | ||
| 145 | } | ||
| 146 | rm -f "$OUT.abort" "$SOCK2" | ||
| 147 | |||
| 91 | echo "e2e OK" | 148 | echo "e2e OK" |