Commit 050a330
Kai Fronsdal
·
2026-06-10 19:47:51 -0400 EDT
parent fbbc6be
fix(kill): release socket before grace sleep so name is reusable `zmx kill X; zmx run X` raced: the daemon's shutdown defer ran handleKill() first and only then closed the listen socket and unlinked the socket file. Two changes: - Reorder the shutdown defer so the listen socket is closed and the socket file unlinked before handleKill()'s 500ms grace period. - Make `zmx kill` synchronous: after sending .Kill, drain-read the connection until EOF. The daemon closes client fds after unlinking the socket file, so when `kill` returns the name is guaranteed free. Avoids a client-side deleteFile, which would race with a fresh session created in between. Adds test/kill_run_race.bats which fails on main and passes here.
2 files changed,
+54,
-4
+19,
-4
| ... | ... | @@ -873,15 +873,19 @@ const Daemon = struct { | |
| 873 | 873 | }; | |
| 874 | 874 | ||
| 875 | 875 | defer { | |
| 876 | - | self.handleKill(); | |
| 877 | - | self.deinit(); | |
| 878 | - | posix.close(pty_fd); | |
| 879 | - | _ = posix.waitpid(self.pid, 0); | |
| 876 | + | // Close and unlink the listen socket BEFORE handleKill()'s | |
| 877 | + | // 500ms SIGHUP->SIGKILL grace sleep. Otherwise a `zmx run` | |
| 878 | + | // for the same name issued in that window will hang waiting | |
| 879 | + | // for a connect. | |
| 880 | 880 | posix.close(server_sock_fd); | |
| 881 | 881 | std.log.info("deleting socket file session={s}", .{self.session_name}); | |
| 882 | 882 | dir.deleteFile(self.session_name) catch |err| { | |
| 883 | 883 | std.log.warn("failed to delete socket file err={s}", .{@errorName(err)}); | |
| 884 | 884 | }; | |
| 885 | + | self.handleKill(); | |
| 886 | + | self.deinit(); | |
| 887 | + | posix.close(pty_fd); | |
| 888 | + | _ = posix.waitpid(self.pid, 0); | |
| 885 | 889 | } | |
| 886 | 890 | ||
| 887 | 891 | try daemonLoop(self, server_sock_fd, pty_fd); |
| ... | ... | @@ -1817,6 +1821,17 @@ fn kill(cfg: *Cfg, session_name: []const u8, force: bool) !void { | |
| 1817 | 1821 | else => return err, | |
| 1818 | 1822 | }; | |
| 1819 | 1823 | ||
| 1824 | + | // Block until the daemon hangs up. The daemon's shutdown defer closes | |
| 1825 | + | // and unlinks the listen socket before it closes client connections, | |
| 1826 | + | // so by the time we read EOF here the session name is free for reuse | |
| 1827 | + | // and a subsequent `zmx run <name>` can't land in the dying daemon's | |
| 1828 | + | // accept backlog. | |
| 1829 | + | var drain: [256]u8 = undefined; | |
| 1830 | + | while (true) { | |
| 1831 | + | const n = posix.read(fd, &drain) catch break; | |
| 1832 | + | if (n == 0) break; | |
| 1833 | + | } | |
| 1834 | + | ||
| 1820 | 1835 | var buf: [100]u8 = undefined; | |
| 1821 | 1836 | var w = std.fs.File.stdout().writer(&buf); | |
| 1822 | 1837 | try w.interface.print("killed session {s}\n", .{session_name}); |
+35,
-0
| ... | ... | @@ -0,0 +1,35 @@ | |
| 1 | + | #!/usr/bin/env bats | |
| 2 | + | # Regression test for the `zmx kill X; zmx run X` race. | |
| 3 | + | # | |
| 4 | + | # Previously `zmx kill` returned immediately after sending the IPC .Kill, | |
| 5 | + | # while the daemon's shutdown defer ran handleKill() -- SIGHUP, 500ms sleep, | |
| 6 | + | # SIGKILL -- BEFORE closing/unlinking the listen socket. A `zmx run X` | |
| 7 | + | # issued in that window would connect() into the kernel backlog of a | |
| 8 | + | # socket the daemon would never accept() on again, then get RST'd | |
| 9 | + | # (ConnectionResetByPeer) when the daemon finally closed the listen fd, | |
| 10 | + | # exiting 1 with no output and no session created. | |
| 11 | + | ||
| 12 | + | load test_helper | |
| 13 | + | ||
| 14 | + | @test "kill then immediate run with same name succeeds" { | |
| 15 | + | for i in 1 2 3; do | |
| 16 | + | "$ZMX" run race-x -d echo first | |
| 17 | + | wait_for_session race-x | |
| 18 | + | ||
| 19 | + | "$ZMX" kill race-x | |
| 20 | + | ||
| 21 | + | # Immediately reuse the same session name. Must not land in the | |
| 22 | + | # dying daemon's listen backlog. | |
| 23 | + | run "$ZMX" run race-x -d echo second | |
| 24 | + | echo "iteration $i: status=$status output=$output" | |
| 25 | + | [ "$status" -eq 0 ] | |
| 26 | + | [[ "$output" == *"session \"race-x\" created"* ]] | |
| 27 | + | ||
| 28 | + | # New session must be live and serving requests. | |
| 29 | + | wait_for_session race-x | |
| 30 | + | run "$ZMX" history race-x | |
| 31 | + | [ "$status" -eq 0 ] | |
| 32 | + | ||
| 33 | + | "$ZMX" kill race-x | |
| 34 | + | done | |
| 35 | + | } |