From 4a55abf1c1a9005b15ba4d9ba24304130ad8c219 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Mon, 12 Oct 2026 15:37:38 +0200 Subject: [PATCH] fix-deviant(os): always reap the URL opener child process Intentional deviation from Ghostty 1.3.1 (DEVIANT_FIXES.md row 32). Zig's os/open.zig runs `try collectOutput(...)` before `wait()`: when collection fails (e.g. a stream exceeds the 50 KiB limit), the error returns early and the child is never waited for, leaving a zombie process for the lifetime of Ghostty. The Rust opener thread now always waits for the child (the pipes are already closed at that point) and still logs the collection error. Test Plan: - cargo test -p ghostty-os open --- crates/ghostty-os/src/open.rs | 8 ++++++-- docs/rewrite/DEVIANT_FIXES.md | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/ghostty-os/src/open.rs b/crates/ghostty-os/src/open.rs index 1974707..7fd337a 100644 --- a/crates/ghostty-os/src/open.rs +++ b/crates/ghostty-os/src/open.rs @@ -147,14 +147,18 @@ fn collect_output(child: &mut Child) -> io::Result<(Vec, Vec)> { } fn open_thread(mut child: Child) { - let stderr = match collect_output(&mut child) { + let collected = collect_output(&mut child); + // Zig's `try collectOutput` skips `wait` on error; reap anyway so the + // child does not stay a zombie (the pipes are closed by now). + let waited = child.wait(); + let stderr = match collected { Ok((_stdout, stderr)) => stderr, Err(e) => { log::warn!("error collecting open command output err={e}"); return; } }; - if let Err(e) = child.wait() { + if let Err(e) = waited { log::warn!("error waiting for open command err={e}"); return; } diff --git a/docs/rewrite/DEVIANT_FIXES.md b/docs/rewrite/DEVIANT_FIXES.md index a18174e..35b616b 100644 --- a/docs/rewrite/DEVIANT_FIXES.md +++ b/docs/rewrite/DEVIANT_FIXES.md @@ -57,6 +57,7 @@ when this file was written. | 29 | CLI: `+list-themes` diagnostics | `cli/list_themes.zig:124-131,171` | The messages ("Could not find the Ghostty resources directory", "No themes found...") go to a 4096-byte stderr buffer that is never flushed, so they are lost. | Flushed when the action ends. | `410d8fc` fix-deviant(cli) | `zig_writer::tests::unflushed_buffer_is_flushed_on_drop` | | 30 | CLI: stdout as a regular file | `std.fs.File.writer` (Zig 0.15.2) as used in `cli/show_face.zig:68-73`, `cli/help.zig:34`, `cli/validate_config.zig:43-46`, `cli/show_config.zig:84` | The writer `pwrite`s at its own offset starting at 0: with stdout redirected to a file, output overwrites itself and earlier file content, and `Writer.end` truncates. | Writes sequentially at the inherited descriptor's current offset (no `pwrite` from offset 0, so earlier content and the writers' own output are not overwritten; this appends only when the descriptor is opened for append, e.g. `>>`); no truncation. | `410d8fc` fix-deviant(cli) (append behaviour first in `f76179f`) | `zig_writer::tests::regular_file_output_is_complete_and_appended` | | 31 | GTK: `+new-window` D-Bus arguments | `apprt/gtk/ipc/new_window.zig:49-53` (`Bytes.new(ptr, len + 1)` + `Variant.newFromBytes(..., trusted = true)`) | Arguments are passed as trusted GVariant strings without validation; invalid UTF-8 or an interior NUL makes GIO abort the process while serializing (input-triggered crash). | `arguments_variant` validates first and returns `Error::IpcFailed` (message on stderr, no D-Bus call); valid arguments are byte-identical to Zig. | uncommitted (Z18) | `ipc::new_window::tests::invalid_dbus_strings_are_rejected_without_abort` (`ghostty-apprt-gtk`) | +| 32 | OS: URL opener child reaping | `os/open.zig` (`try collectOutput(...)` before `wait()`) | When collecting the opener's output fails (e.g. more than 50 KiB on a stream), the error returns before `wait()`, leaving the child as a zombie. | The child is always reaped; the collection error is still logged. | (this register row's commit, fix-deviant(os)) | `ghostty-os` open tests | ## Upstream quirks deliberately kept (not fixed)