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
This commit is contained in:
1 parent
65dd9d579f
commit
4a55abf1c1
2 files changed
+7
-2
No files matched your search
@@ -147,14 +147,18 @@ fn collect_output(child: &mut Child) -> io::Result<(Vec<u8>, Vec<u8>)> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn open_thread(mut child: Child) {
|
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,
|
Ok((_stdout, stderr)) => stderr,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
log::warn!("error collecting open command output err={e}");
|
log::warn!("error collecting open command output err={e}");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
if let Err(e) = child.wait() {
|
if let Err(e) = waited {
|
||||||
log::warn!("error waiting for open command err={e}");
|
log::warn!("error waiting for open command err={e}");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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` |
|
| 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` |
|
| 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`) |
|
| 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)
|
## Upstream quirks deliberately kept (not fixed)
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user