diff --git a/crates/ghostty-core/src/app.rs b/crates/ghostty-core/src/app.rs index cd38ed6..091b2f6 100644 --- a/crates/ghostty-core/src/app.rs +++ b/crates/ghostty-core/src/app.rs @@ -440,9 +440,14 @@ impl App { rt.perform_action(target, RtAction::NewWindow); } - /// Drain the mailbox. + /// Process at most one mailbox capacity per tick, then return to the + /// runtime so it can dispatch input, timers, and drawing. Producers can + /// refill the queue continuously under heavy terminal output. fn drain_mailbox(&self, rt: &dyn RtApp) { - while let Ok(message) = self.state.mailbox_rx.try_recv() { + for _ in 0..MAILBOX_CAPACITY { + let Ok(message) = self.state.mailbox_rx.try_recv() else { + return; + }; match message { Message::OpenConfig => { let _ = self.perform_action(rt, &BindingAction::OpenConfig); @@ -469,6 +474,12 @@ impl App { } } } + + // Producer wakeups may have coalesced during this tick. Ensure a + // fresh iteration for deferred messages even if producers stop now. + if !self.state.mailbox_rx.is_empty() { + self.state.mailbox.waker.wake(); + } } /// Close a surface (Zig: `closeSurface`). @@ -657,3 +668,65 @@ impl CoreApp for App { }) } } + +#[cfg(test)] +mod tests { + use std::sync::atomic::{AtomicUsize, Ordering}; + + use super::*; + + #[derive(Default)] + struct CountingWaker(AtomicUsize); + + impl Waker for CountingWaker { + fn wake(&self) { + self.0.fetch_add(1, Ordering::Relaxed); + } + } + + /// Refill the mailbox during each callback, like a renderer producing + /// redraw requests while the app processes earlier requests. + struct RefillingRuntime { + mailbox: Mailbox, + calls: Cell, + } + + impl RtApp for RefillingRuntime { + fn perform_action(&self, _: Target, action: RtAction) -> bool { + assert!(matches!(action, RtAction::NewWindow)); + let calls = self.calls.get() + 1; + self.calls.set(calls); + // Finite even with an unbounded drain, so regressions fail + // without hanging the test runner. + if calls < MAILBOX_CAPACITY * 2 { + self.mailbox + .push(Message::NewWindow { parent: None }, Timeout::Instant); + } + true + } + } + + #[test] + fn mailbox_tick_yields_and_rearms_when_producer_keeps_refilling() { + let waker = Arc::new(CountingWaker::default()); + let app = App::new(waker.clone()).unwrap(); + let rt = RefillingRuntime { + mailbox: app.mailbox(), + calls: Cell::new(0), + }; + rt.mailbox + .push(Message::NewWindow { parent: None }, Timeout::Instant); + + app.tick(&rt).unwrap(); + assert_eq!(rt.calls.get(), MAILBOX_CAPACITY); + assert_eq!(app.state.mailbox_rx.len(), 1); + // Initial push, one push per callback, and a wake to resume the + // pending work after the runtime has handled input and timers. + assert_eq!(waker.0.load(Ordering::Relaxed), MAILBOX_CAPACITY + 2); + + app.tick(&rt).unwrap(); + assert_eq!(rt.calls.get(), MAILBOX_CAPACITY * 2); + assert!(app.state.mailbox_rx.is_empty()); + assert_eq!(waker.0.load(Ordering::Relaxed), MAILBOX_CAPACITY * 2 + 1); + } +} diff --git a/docs/rewrite/ARCHITECTURE.md b/docs/rewrite/ARCHITECTURE.md index d0b0147..54d0d80 100644 --- a/docs/rewrite/ARCHITECTURE.md +++ b/docs/rewrite/ARCHITECTURE.md @@ -128,6 +128,10 @@ GTK invokes `glib-compile-resources` directly so generated XML/UI files in thread, plus a pty read thread. Mailboxes are `crossbeam-channel` / `std::sync::mpsc`; the GTK main thread is woken via `glib` channels. The `Terminal` is shared behind `Arc>` (Ghostty's renderer mutex). + `ghostty-core::App` handles at most 64 mailbox messages per tick and wakes + the runtime again if work remains. Continuous renderer notifications must + yield to GTK input, timers, and drawing instead of extending one tick + indefinitely. 9. **Config:** same file format, same keys, same defaults, same CLI flags, same docs text (used by `+show-config --docs` and `--help`). 10. **Tests:** every Zig `test "..."` block in a ported file is ported to a diff --git a/docs/rewrite/DEVIANT_FIXES.md b/docs/rewrite/DEVIANT_FIXES.md index e5f57f5..68825c4 100644 --- a/docs/rewrite/DEVIANT_FIXES.md +++ b/docs/rewrite/DEVIANT_FIXES.md @@ -65,6 +65,7 @@ when this file was written. | 37 | Termio: subprocess cleanup | `termio/Exec.zig:1188-1224` (`getpgid`) | Loops forever while `getpgid(child) == our pgid`, i.e. also when the child died before `setsid` and is an unreaped zombie (its pgid stays ours), hanging the surface close. | Still waits without a cap (no 5 s limit) while the child is alive, but stops (no SIGHUP, nothing to kill) once `waitid(WNOWAIT)` shows the child exited. | `b9ad595` fix-deviant(termio) | `getpgid_gives_up_on_exited_child_without_setsid`, `getpgid_waits_for_late_setsid` (`ghostty-termio/src/exec.rs`) | | 38 | Config (Windows): absolute theme paths | `config/Config.zig:9788-9799` (`Theme.parseCLI`) | Any value containing `,`, `=` or `:` is parsed as a `light:..,dark:..` pair, and a Windows absolute path (`C:\\themes\\x`, `\\\\?\\C:\\...`) always contains a colon, so `theme = ` is rejected on Windows (found by running the config tests under Wine). | On Windows a value that starts like an absolute Windows path (drive letter + `:` + separator, or `\\\\`) is a single theme; the pair syntax is unchanged (quote backslashes). Other platforms are unchanged. | `97a262d` fix-deviant(config) | `windows_absolute_path_detection`, `theme_windows_absolute_path_and_pair_with_paths` (`ghostty-config/src/types_b.rs`), `theme_loading` under Wine | | 39 | Termio (Windows): bare program names | `Command.zig:261,347-358` (`startWindows`, `CreateProcessW(application_w, ...)`) with `termio/Exec.zig:1004` (`.path = self.args[0]`) | `lpApplicationName` is never searched on `PATH` (a partial name only completes against the current directory), so a `direct:` command / `-e` argv[0] such as `cmd.exe` fails with `FileNotFound`; only the default shell works (it is wrapped as `%WINDIR%\\System32\\cmd.exe /C ...`). | A name without drive/separator is looked up on `PATH` (`.exe` implied) before `CreateProcessW`; failures keep the original name (same error as Zig). | `4d7ad7c` fix-deviant(termio) | `resolve_application_searches_path_for_bare_names`, `bare_program_name_starts_via_path_search` (`ghostty-termio/src/command_windows.rs`), `echo_output_reaches_terminal_and_exit_is_reported` (`tests/exec_windows.rs`, Wine) | +| 40 | App: mailbox starvation | `App.zig:238-266` (`drainMailbox`) | Drains until the mailbox is empty. A producer that keeps refilling it can keep one app tick running indefinitely, preventing the runtime from dispatching input and timers. | Handles at most one mailbox capacity (64 messages) per tick, then wakes the runtime if messages remain. This bounds queue processing without dropping or reordering messages. The regression uses finite callback-driven refills; the reported full freeze with continuous base64 is not consistently reproduced in headless GTK. | this commit (`fix-deviant(core)`) | `app::tests::mailbox_tick_yields_and_rearms_when_producer_keeps_refilling` (`ghostty-core/src/app.rs`) | ## Upstream quirks deliberately kept (not fixed)