Skip to content

Commit 5df7e29

Browse files
authored
Merge pull request #247 from forketyfork/fix/cmdw-close-crash
fix(runtime): prevent Cmd+W close crash during session despawn
2 parents 8c14b0b + 65ba36c commit 5df7e29

File tree

3 files changed

+61
-6
lines changed

3 files changed

+61
-6
lines changed

docs/ARCHITECTURE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ Renderer draws attention border / story overlay
335335
| `platform/sdl.zig` | SDL3 initialization, window management, HiDPI | `init()`, `createWindow()`, `createRenderer()` | `c` |
336336
| `input/mapper.zig` | SDL keycodes to VT escape sequences, shortcut detection | `encodeKey()`, modifier helpers | `c` |
337337
| `c.zig` | C FFI re-exports (SDL3, SDL3_ttf constants) | `SDLK_*`, `SDL_*`, `TTF_*` re-exports | SDL3 system libs (via `@cImport`) |
338-
| `session/state.zig` | Terminal session lifecycle: PTY, ghostty-vt, process watcher, foreground agent detection, graceful agent teardown at quit | `SessionState`, `AgentKind`, `init()`, `deinit()`, `ensureSpawnedWithDir()`, `render_epoch`, `pending_write`, `detectForegroundAgent()`, `sendTermToForegroundPgrp()`, `drainOutputForMs()` | `shell`, `pty`, `vt_stream`, `cwd`, `font`, xev |
338+
| `session/state.zig` | Terminal session lifecycle: PTY, ghostty-vt, process watcher, foreground agent detection, graceful agent teardown at quit | `SessionState`, `AgentKind`, `init()`, `despawn()`, `deinit()`, `ensureSpawnedWithDir()`, `render_epoch`, `pending_write`, `detectForegroundAgent()`, `sendTermToForegroundPgrp()`, `drainOutputForMs()` | `shell`, `pty`, `vt_stream`, `cwd`, `font`, xev |
339339
| `session/notify.zig` | Background notification socket thread and queue; handles status and story notifications | `NotificationQueue`, `Notification` (union: status/story), `startThread()`, `push()`, `drain()` | std (socket, thread) |
340340
| `session/*` (shell, pty, vt_stream, cwd) | Shell spawning, PTY abstraction, VT parsing, working directory detection | `spawn()`, `Pty`, `VtStream.processBytes()`, `getCwd()` | std (posix), ghostty-vt |
341341
| `render/renderer.zig` | Scene rendering: terminals, borders, animations, terminal scrollbar painting | `render()`, `RenderCache`, per-session texture management | `font`, `font_cache`, `gfx/*`, `anim/easing`, `app/app_state`, `ui/components/scrollbar`, `c` |

src/app/runtime.zig

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1382,7 +1382,7 @@ pub fn run() !void {
13821382
}
13831383

13841384
// Close the terminal
1385-
session.deinit(allocator);
1385+
session.despawn(allocator);
13861386
session_interaction_component.resetView(session_idx);
13871387
session.markDirty();
13881388

@@ -1967,7 +1967,7 @@ pub fn run() !void {
19671967
break :blk null;
19681968
};
19691969
}
1970-
sessions[idx].deinit(allocator);
1970+
sessions[idx].despawn(allocator);
19711971
session_interaction_component.resetView(idx);
19721972
sessions[idx].markDirty();
19731973
compactSessions(sessions, session_interaction_component.viewSlice(), &render_cache, &anim_state);

src/session/state.zig

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,11 @@ pub const SessionState = struct {
144144
InvalidArgument,
145145
};
146146

147+
const WaitContextCleanup = enum {
148+
destroy_immediately,
149+
defer_if_active,
150+
};
151+
147152
pub fn init(
148153
allocator: std.mem.Allocator,
149154
slot_index: usize,
@@ -264,6 +269,16 @@ pub const SessionState = struct {
264269
}
265270

266271
pub fn deinit(self: *SessionState, allocator: std.mem.Allocator) void {
272+
self.teardown(allocator, .destroy_immediately);
273+
}
274+
275+
/// Runtime close path used while the event loop is still active.
276+
/// Keeps active process wait callbacks valid by deferring context destruction.
277+
pub fn despawn(self: *SessionState, allocator: std.mem.Allocator) void {
278+
self.teardown(allocator, .defer_if_active);
279+
}
280+
281+
fn teardown(self: *SessionState, allocator: std.mem.Allocator, wait_ctx_cleanup: WaitContextCleanup) void {
267282
self.pending_write.deinit(allocator);
268283
self.pending_write = .empty;
269284
self.quit_capture.deinit(allocator);
@@ -287,9 +302,16 @@ pub const SessionState = struct {
287302
self.process_watcher = null;
288303
}
289304
if (self.process_wait_ctx) |ctx| {
290-
// Free unconditionally: deinit runs after the event loop stops, so
291-
// processExitCallback will never fire for pending completions.
292-
allocator.destroy(ctx);
305+
switch (wait_ctx_cleanup) {
306+
.destroy_immediately => {
307+
allocator.destroy(ctx);
308+
},
309+
.defer_if_active => {
310+
if (ctx.completion.state() == .dead) {
311+
allocator.destroy(ctx);
312+
}
313+
},
314+
}
293315
}
294316
self.process_wait_ctx = null;
295317
// Wrap intentionally: process_generation is a bounded counter and may overflow.
@@ -818,6 +840,39 @@ test "SessionState assigns incrementing ids" {
818840
try std.testing.expectEqualStrings("1", std.mem.sliceTo(second.session_id_z[0..], 0));
819841
}
820842

843+
test "despawn keeps active wait context alive until callback reclaims it" {
844+
const allocator = std.testing.allocator;
845+
const size = pty_mod.winsize{
846+
.ws_row = 24,
847+
.ws_col = 80,
848+
.ws_xpixel = 0,
849+
.ws_ypixel = 0,
850+
};
851+
const notify_sock: [:0]const u8 = "sock";
852+
853+
var session = try SessionState.init(allocator, 0, "/bin/zsh", size, notify_sock);
854+
defer session.deinit(allocator);
855+
856+
const wait_ctx = try allocator.create(SessionState.WaitContext);
857+
wait_ctx.* = .{
858+
.session = &session,
859+
.generation = 0,
860+
.pid = 1,
861+
.completion = .{},
862+
};
863+
wait_ctx.completion.flags.state = @enumFromInt(1);
864+
865+
session.process_wait_ctx = wait_ctx;
866+
session.despawn(allocator);
867+
try std.testing.expect(session.process_wait_ctx == null);
868+
869+
var loop = try xev.Loop.init(.{});
870+
defer loop.deinit();
871+
var completion: xev.Completion = .{};
872+
const action = SessionState.processExitCallback(wait_ctx, &loop, &completion, 0);
873+
try std.testing.expectEqual(xev.CallbackAction.disarm, action);
874+
}
875+
821876
fn makeNonBlocking(fd: posix.fd_t) MakeNonBlockingError!void {
822877
const flags = try posix.fcntl(fd, posix.F.GETFL, 0);
823878
var o_flags: posix.O = @bitCast(@as(u32, @intCast(flags)));

0 commit comments

Comments
 (0)