[jparise]

Our previous application termination path (e.g. Cmd-Q) ultimately led to Surface.deinit, which uses a detached Task to free its Ghostty surface. That didn't give the child processes (e.g. bash) a chance to receive a SIGHUP before the app completely exits, potentially resulting in lost shell history and un-run SIGHUP handlers.

This change implements a more graceful termination sequence that explicitly signals the child processes to hang up before PTY teardown.

All of the existing close/undo/termination behaviors remain unchanged (e.g. confirmation, quit-on-last-window-closed, etc.).

Verified using:

#!/bin/bash
trap 'echo "SIGHUP at $(date)" >> /tmp/ghostty-sigup.log' SIGHUP
echo "Waiting for SIGHUP... "
read -p "Press Cmd+Q to quit Ghostty..."

Fixes: #9773

[mitchellh]

Thanks for this. This is a real problem, but I don't love that we dig directly into the IO backend from embedded.zig. The intention of deinit is that we should be waiting for the process to exit, it shouldn't require a separate call and wait.

[jparise]

Thanks for this. This is a real problem, but I don't love that we dig directly into the IO backend from embedded.zig. The intention of deinit is that we should be waiting for the process to exit, it shouldn't require a separate call and wait.

The ghostty_surface_free path (that calls Surface.deinit) is ... complicated. Here's where we call it from the macOS side:

ghostty/macos/Sources/Ghostty/Ghostty.Surface.swift

Lines 26 to 36 in c45a762

	deinit {
	// deinit is not guaranteed to happen on the main actor and our API
	// calls into libghostty must happen there so we capture the surface
	// value so we don't capture `self` and then we detach it in a task.
	// We can't wait for the task to succeed so this will happen sometime
	// but that's okay.
	let surface = self.surface
	Task.detached { @MainActor in
	ghostty_surface_free(surface)
	}
	}

But further, deinit itself isn't guaranteed to be run on application terminate. (This behavior appears to be deliberately left ambiguous by Apple, but the observed behavior is that the OS has the freedom to bulk-free an application's objects on termination without running finalizers.)

If you combine those two things (detached/async ghostty_surface_free and deinit not guaranteed on run on termination), we have a pretty constrained set of options:

1. Block on ghostty_surface_free (always or at least during termination).
2. Perform a two phase shutdown (stop+wait and free).

I initially implemented a version of (1) in 2c46952ef198fc41036a7b3b4d6721919862ca57 because I also wanted to (re)use the ghostty_surface_free path. I ran into problems where freeing the surface before macOS app's Surface.deinit challenged the expected surface lifetime. For example, because we'd delayed app termination to ensure the processes finish before a brutal kill, the surface is still the target of I/O events, so we'd run into crashes.

There are some other related concerns with the ghostty_surface_free path that we'd also need to think through:

a. We need (I think?) to sequentially free the surfaces due to the main thread requirement, rather than quickly signalling all of the child processes and then waiting on the batch.
b. ghostty_surface_free blocks on killPid, which waits forever: https://github.com/ghostty-org/ghostty/blob/75953d7ca761ce17f9a06e5197e0b81dad7c1855/src/termio/Exec.zig#L1171-L1175

I then implemented a version of (2) in 75953d7ca761ce17f9a06e5197e0b81dad7c1855, by way of the new ghostty_surface_process_stop. If the main concern is with the direct IO backend interaction, I can think of two other things to try:

a. Add a "stop" function to Surface as an indirection so that the embedded apprt can stop the child process through a more formal interface. (edit: 0bce07d92f41c5f9c33d93229626d91cc538971b)
b. Instead of stopping the process, add a way to move the surface into a "shutting down" state that would make it possible to call ghostty_surface_free earlier and avoid some of the problems above.

And of course any other ideas that folks might have.

[luisdavim]

I'm sorry for asking but, is there any update on this?

[jparise]

I'm sorry for asking but, is there any update on this?

I just freshened it so it will cleanly merge with main. Still needs @mitchellh to revisit his review.

[jparise]

@mitchellh what do you think of the ghostty_surface_process_stop-based approach that I last implemented? Does that provide the right level of abstraction you were looking for, or:

b. Instead of stopping the process, add a way to move the surface into a "shutting down" state that would make it possible to call ghostty_surface_free earlier and avoid some of the problems above.

[jparise]

@mitchellh I reworked this around a new ghostty_app_quit function that coordinates the teardown work in the apprt. This means we can continue using the deinit path while still giving child processes some time to cleanly hangup.

@bo2themax This also simplifies the macOS-specific changes.

GTK and others could adopt something similar to ghostty_app_quit to realize the same benefits if needed, but I'll leave that to those maintainers.

[jparise]

@bo2themax rebased on top of #12742

[jparise]

@mitchellh I iterated on this some more, and I believe we can address the original issue by explicitly sending SIGHUP to all child processes immediately before application termination. Previously, PTY teardown could cause bash to begin its normal exit handling before it got the PTY-generated SIGHUP, preventing it from writing history. Sending SIGHUP first produces the expected ordering without requiring us to wait for the child processes to exit.

I verified this with both a delayed SIGHUP trap handler and bash’s built-in history writing. The handler continued after Ghostty exited, confirming that we don’t need to delay/block termination or drain the mailbox.

[mitchellh]

Would it work to simplify this and just send the hangup before the kill? Or are we relying on some implicit delay timing here? Or am I misunderstanding the whole thing and our app quitting happens too fast before the kill (would still be relying on some implicit timing then for the sighup?)

[jparise]

Would it work to simplify this and just send the hangup before the kill? Or are we relying on some implicit delay timing here? Or am I misunderstanding the whole thing and our app quitting happens too fast before the kill (would still be relying on some implicit timing then for the sighup?)

The child process termination in this case isn't initiated through our kill* functions. Ghostty exits, the OS closes the PTY, and the resulting PTY teardown causes the child to exit. Our kill* functions already send SIGHUP (not SIGKILL) through Subprocess.stop() during surface deinit.

The root problem is that this deinit path is not reliably called when the macOS application terminates. Swift object finalizers are not guaranteed to run, and when Ghostty.Surface.deinit does run, it schedules ghostty_surface_free in a detached task that may not execute before the application exits.

My latest approach here (ghostty_app_quit) makes the hangups explicit before the application terminates, ensuring the signal is sent before PTY teardown (but we don't need to wait for it to be handled).

[jparise]

If ghostty_app_quit itself doesn't feel right, I think we could also expose a surface-level function that (just) sends the hangup signal, and we could call it for all of our active (macOS) surfaces in the termination path (i.e. implement ghostty_quit_app in the app itself, using a new ghostty_surface_hangup or similar).

That's close to what I had originally on this branch, except I was more focused on cleaning up the surface resources then too.

Or, maybe ghostty_app_quit is renamed ghostty_app_hangup. I think it's still nicer to do it in the apprt because it owns the authoritative set of surfaces.

[mitchellh]

Okay okay I think I get it. Sorry I know you've been working on this a while so forgive the catch-up again.

The root problem is that this deinit path is not reliably called when the macOS application terminates. Swift object finalizers are not guaranteed to run, and when Ghostty.Surface.deinit does run, it schedules ghostty_surface_free in a detached task that may not execute before the application exits.

Okay, alternate idea. Would it be cleaner to make surfaces have a free-path that isn't through deinit (but is idempotent so when deinit is called its safe). Then on quite we can iterate through all open surfaces and force the free?

[mitchellh]

To add-on here, my motivation is just to try to bring together all the "close" paths through one path rather than more APIs.

[jparise]

The root problem is that this deinit path is not reliably called when the macOS application terminates. Swift object finalizers are not guaranteed to run, and when Ghostty.Surface.deinit does run, it schedules ghostty_surface_free in a detached task that may not execute before the application exits.

Okay, alternate idea. Would it be cleaner to make surfaces have a free-path that isn't through deinit (but is idempotent so when deinit is called its safe). Then on quite we can iterate through all open surfaces and force the free?

That would be similar to where I started in c7bde6a0cdf0b77e831d10021b7520eda6e997fe. The main problem is that our "surface free" operation destroys the underlying (C) allocation, so we'd need to make all Swift-level surface access safe, probably by making the ghostty_surface_t an optional:

private var surface: ghostty_surface_t?

@MainActor
func free() {
    guard let surface else { return }
    self.surface = nil
    ghostty_surface_free(surface)
}

deinit {
    // call free()
}

There was some other complication I ran into back then, too, I think related to in-flight callbacks reentering and during the destruction, but maybe that's just a matter of guarding surface in more places.

[jparise]

The root problem is that this deinit path is not reliably called when the macOS application terminates. Swift object finalizers are not guaranteed to run, and when Ghostty.Surface.deinit does run, it schedules ghostty_surface_free in a detached task that may not execute before the application exits.

Okay, alternate idea. Would it be cleaner to make surfaces have a free-path that isn't through deinit (but is idempotent so when deinit is called its safe). Then on quite we can iterate through all open surfaces and force the free?

I implemented this approach, and it works well across all of my local tests. It became possible once I realized we don't need to explicitly wait on signal delivery (the previous commit), so a better outcome overall!

[jparise]

@mitchellh give this another look, potentially for 1.4?