e33768e7
test: the pty exit tests drain the master the daemon reads
a73x 2026-09-03 19:13
Commit message
src/server/pty.zig
| Old | New | ||
|---|---|---|---|
| @@ -274,6 +274,33 @@ test "Pty: spawn /bin/sh, echo round trip" { | |||
| 274 | try std.testing.expect(std.mem.indexOf(u8, out.items, "m1-pty-ok") != null); | 274 | try std.testing.expect(std.mem.indexOf(u8, out.items, "m1-pty-ok") != null); |
| 275 | } | 275 | } |
| 276 | 276 | ||
| 277 | /// Wait for the shell behind `pty` to exit, DRAINING the master while it | ||
| 278 | /// waits, and return its code (null if the budget ran out). Draining is not | ||
| 279 | /// tidiness: the daemon reads every session's master continuously, and on | ||
| 280 | /// Darwin a shell cannot finish exiting while the output it wrote sits | ||
| 281 | /// undrained in the tty — the last close of the slave waits for that queue | ||
| 282 | /// to empty, so a test that only slept would deadlock against a shell that | ||
| 283 | /// had already run `exit`. Measured 2026-09-03 on macOS 26 with a plain | ||
| 284 | /// forkpty and no mux in the picture: the same child exits in 600 ms when | ||
| 285 | /// the master is read and never at all when it is not. Linux lets the exit | ||
| 286 | /// through either way, which is why this went unnoticed until the Mac. | ||
| 287 | fn waitExitDraining(pty: *Pty, budget_ms: u64) ?u32 { | ||
| 288 | var buf: [4096]u8 = undefined; | ||
| 289 | var waited_ms: u64 = 0; | ||
| 290 | while (waited_ms < budget_ms) : (waited_ms += 50) { | ||
| 291 | if (pty.checkExited()) |code| return code; | ||
| 292 | var fds = [_]std.posix.pollfd{ | ||
| 293 | .{ .fd = pty.master, .events = std.posix.POLL.IN, .revents = 0 }, | ||
| 294 | }; | ||
| 295 | // EOF and EIO both mean the pty is finished; keep waiting for the | ||
| 296 | // status either way, because the exit code is what is being asked | ||
| 297 | // for and `checkExited` above is what answers it. | ||
| 298 | const ready = std.posix.poll(&fds, 50) catch 0; | ||
| 299 | if (ready > 0) _ = std.posix.read(pty.master, &buf) catch {}; | ||
| 300 | } | ||
| 301 | return pty.checkExited(); | ||
| 302 | } | ||
| 303 | |||
| 277 | /// Returns everything read, so a caller asserting absence can show what | 304 | /// Returns everything read, so a caller asserting absence can show what |
| 278 | /// it got. | 305 | /// it got. |
| 279 | fn readUntil( | 306 | fn readUntil( |
| @@ -403,14 +430,7 @@ test "Pty: checkExited reports shell exit" { | |||
| 403 | try std.testing.expect(pty.checkExited() == null); | 430 | try std.testing.expect(pty.checkExited() == null); |
| 404 | _ = try std.posix.write(pty.master, "exit 7\n"); | 431 | _ = try std.posix.write(pty.master, "exit 7\n"); |
| 405 | 432 | ||
| 406 | var waited_ms: u64 = 0; | 433 | try std.testing.expectEqual(@as(?u32, 7), waitExitDraining(&pty, 5000)); |
| 407 | var code: ?u32 = null; | ||
| 408 | while (waited_ms < 5000) : (waited_ms += 50) { | ||
| 409 | code = pty.checkExited(); | ||
| 410 | if (code != null) break; | ||
| 411 | std.Thread.sleep(50 * std.time.ns_per_ms); | ||
| 412 | } | ||
| 413 | try std.testing.expectEqual(@as(?u32, 7), code); | ||
| 414 | } | 434 | } |
| 415 | 435 | ||
| 416 | test "Pty: a later spawn does not inherit an earlier session's master" { | 436 | test "Pty: a later spawn does not inherit an earlier session's master" { |
| @@ -593,14 +613,7 @@ test "Pty.adopt: an adopted pair still reports the child's real exit code" { | |||
| 593 | 613 | ||
| 594 | _ = try std.posix.write(adopted.master, "exit 7\n"); | 614 | _ = try std.posix.write(adopted.master, "exit 7\n"); |
| 595 | 615 | ||
| 596 | var waited_ms: u64 = 0; | 616 | try std.testing.expectEqual(@as(?u32, 7), waitExitDraining(&adopted, 5000)); |
| 597 | var code: ?u32 = null; | ||
| 598 | while (waited_ms < 5000) : (waited_ms += 50) { | ||
| 599 | code = adopted.checkExited(); | ||
| 600 | if (code != null) break; | ||
| 601 | std.Thread.sleep(50 * std.time.ns_per_ms); | ||
| 602 | } | ||
| 603 | try std.testing.expectEqual(@as(?u32, 7), code); | ||
| 604 | 617 | ||
| 605 | adopted.deinit(); | 618 | adopted.deinit(); |
| 606 | } | 619 | } |