Commit 11a12cc

Ian Tay  ·  2026-03-08 12:55:02 -0400 EDT
parent 690487b 
fix: only clean up socket on ConnectionRefused, not Timeout

A 1-second probe timeout doesn't mean the daemon is dead — it may just be
busy (heavy output, terminal resize reflow, serializing state for another
client). Deleting a live daemon's socket orphans it permanently with no
way to reach it via zmx commands.

Applied to all callsites: get_session_entries, ensureSession (worst case:
spawns a replacement daemon leaving the old one orphaned), kill, detachAll,
history. For kill, the user now gets a helpful message if the daemon is
busy vs. actually dead. The `zmx list` status label for error entries now
says `status=unreachable` (not `cleaning up`) on Timeout, so the display
doesn't contradict what we actually did.

`zmx wait` now treats is_error entries as done+failed: on Timeout the
socket persists, so without this the session would sit at task_ended_at==0
forever, defeating both the completion check and the zero-match timeout.
2 files changed,  +46, -9
M src/main.zig
M src/util.zig
M src/main.zig
+31, -7
......@@ -360,9 +360,18 @@ const Daemon = struct {
360360 if (self.command != null) {
361361 std.log.warn("session already exists, ignoring command session={s}", .{self.session_name});
362362 }
363- } else |_| {
364- socket.cleanupStaleSocket(dir, self.session_name);
365- should_create = true;
363+ } else |err| switch (err) {
364+ // Daemon is definitively gone: safe to replace.
365+ error.ConnectionRefused => {
366+ socket.cleanupStaleSocket(dir, self.session_name);
367+ should_create = true;
368+ },
369+ // Probe didn't respond in time -- daemon may just be busy.
370+ // The probe is only to decide create-vs-attach; the session
371+ // exists, so proceed to attach rather than fail or orphan.
372+ else => {
373+ std.log.warn("probe slow ({s}), proceeding to attach session={s}", .{ @errorName(err), self.session_name });
374+ },
366375 }
367376 }
368377
......@@ -710,6 +719,17 @@ fn wait(cfg: *Cfg, session_names: std.ArrayList([]const u8)) !void {
710719 }
711720
712721 total += 1;
722+ if (session.is_error) {
723+ // Daemon unreachable (probe timed out). On Timeout the socket
724+ // is no longer deleted, so this session would otherwise
725+ // persist as task_ended_at==0 forever → infinite "still
726+ // waiting". Count it as done+failed so wait terminates.
727+ try stdout.print("task unreachable: {s} ({s})\n", .{ session.name, session.error_name orelse "unknown" });
728+ try stdout.flush();
729+ agg_exit_code = 1;
730+ done += 1;
731+ continue;
732+ }
713733 if (session.task_ended_at == 0) {
714734 try stdout.print("still waiting task={s}\n", .{session.name});
715735 try stdout.flush();
......@@ -818,7 +838,7 @@ fn detachAll(cfg: *Cfg) !void {
818838 defer alloc.free(socket_path);
819839 const result = ipc.probeSession(alloc, socket_path) catch |err| {
820840 std.log.err("session unresponsive: {s}", .{@errorName(err)});
821- socket.cleanupStaleSocket(dir, session_name);
841+ if (err == error.ConnectionRefused) socket.cleanupStaleSocket(dir, session_name);
822842 return;
823843 };
824844 defer posix.close(result.fd);
......@@ -846,10 +866,14 @@ fn kill(cfg: *Cfg, session_name: []const u8) !void {
846866 defer alloc.free(socket_path);
847867 const result = ipc.probeSession(alloc, socket_path) catch |err| {
848868 std.log.err("session unresponsive: {s}", .{@errorName(err)});
849- socket.cleanupStaleSocket(dir, session_name);
850869 var buf: [4096]u8 = undefined;
851870 var w = std.fs.File.stdout().writer(&buf);
852- w.interface.print("cleaned up stale session {s}\n", .{session_name}) catch {};
871+ if (err == error.ConnectionRefused) {
872+ socket.cleanupStaleSocket(dir, session_name);
873+ w.interface.print("cleaned up stale session {s}\n", .{session_name}) catch {};
874+ } else {
875+ w.interface.print("session {s} is unresponsive ({s}) -- daemon may be busy, try again or kill the process directly\n", .{ session_name, @errorName(err) }) catch {};
876+ }
853877 w.interface.flush() catch {};
854878 return;
855879 };
......@@ -883,7 +907,7 @@ fn history(cfg: *Cfg, session_name: []const u8, format: util.HistoryFormat) !voi
883907 defer alloc.free(socket_path);
884908 const result = ipc.probeSession(alloc, socket_path) catch |err| {
885909 std.log.err("session unresponsive: {s}", .{@errorName(err)});
886- socket.cleanupStaleSocket(dir, session_name);
910+ if (err == error.ConnectionRefused) socket.cleanupStaleSocket(dir, session_name);
887911 return;
888912 };
889913 defer posix.close(result.fd);
M src/util.zig
+15, -2
......@@ -54,7 +54,12 @@ pub fn get_session_entries(alloc: std.mem.Allocator, socket_dir: []const u8) !st
5454 .task_exit_code = 1,
5555 .task_ended_at = 0,
5656 });
57- socket.cleanupStaleSocket(dir, entry.name);
57+ // Only clean up when the daemon is definitively gone. A busy
58+ // daemon can miss the probe timeout; deleting its socket
59+ // orphans it permanently.
60+ if (err == error.ConnectionRefused) {
61+ socket.cleanupStaleSocket(dir, entry.name);
62+ }
5863 continue;
5964 };
6065 posix.close(result.fd);
......@@ -292,10 +297,18 @@ pub fn writeSessionLine(writer: *std.Io.Writer, session: SessionEntry, short: bo
292297 }
293298
294299 if (session.is_error) {
295- try writer.print("{s}name={s}\terr={s}\tstatus=cleaning up\n", .{
300+ // "cleaning up" is only truthful when the probe was definitively
301+ // refused (socket deleted this pass). On Timeout/Unexpected the
302+ // daemon may just be busy, so don't lie about what we did.
303+ const status = if (std.mem.eql(u8, session.error_name.?, "ConnectionRefused"))
304+ "cleaning up"
305+ else
306+ "unreachable";
307+ try writer.print("{s}name={s}\terr={s}\tstatus={s}\n", .{
296308 prefix,
297309 session.name,
298310 session.error_name.?,
311+ status,
299312 });
300313 return;
301314 }