mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-09-14 18:01:58 +00:00
os/open: consume the newline when draining opener stderr (#14125)
Fixes the runaway-thread bug reported in #14100 (vouched there). `openThread` drains the spawned opener's stderr with `takeDelimiterExclusive('\n')`. That function tosses only the exclusive length, so the `'\n'` is never consumed. Once the child writes one line to stderr, every subsequent call returns an empty slice without advancing the stream: the `while (true)` loop spins forever — one pinned core per affected `open()`, logging empty `os-open: open stderr=` warnings at tens of thousands of messages per second for the lifetime of the process — and `exe.wait()` is never reached, so the child is never reaped. This change reads inclusively (`takeDelimiterInclusive`, which does consume the delimiter) and trims the `'\n'` for logging. Observed in the wild embedding libghostty on macOS: several days of uptime accumulated six leaked opener threads at ~70% of a core each (~4.4 cores), from six link clicks whose `/usr/bin/open` wrote to stderr. After the fix, the same workload shows zero `os-open` log traffic and no leaked threads. Repro without the fix: open a link whose handler writes to stderr (e.g. an OSC 8 link with an unknown scheme), then watch a core pin and `log stream --predicate 'subsystem == "com.mitchellh.ghostty"'` flood. **AI disclosure** (per `AI_POLICY.md`): the bug was diagnosed and this patch drafted with Claude Code (thread sampling, log analysis, and reading the Zig 0.16 `std.Io.Reader` source to confirm `takeDelimiterExclusive`/`takeDelimiterInclusive` toss semantics). I reviewed the analysis and the change, understand both, and verified the fix in a production build of the embedding app.
This commit is contained in:
@@ -86,7 +86,11 @@ fn openThread(io: std.Io, exe_: std.process.Child) void {
|
||||
var stream = stderr.readerStreaming(io, &buffer);
|
||||
const reader = &stream.interface;
|
||||
while (true) {
|
||||
const line = reader.takeDelimiterExclusive('\n') catch |outer| switch (outer) {
|
||||
// Read inclusively so the delimiter is consumed:
|
||||
// takeDelimiterExclusive leaves the '\n' buffered, so once the
|
||||
// child writes a line this loop would receive an empty slice
|
||||
// forever, pinning a core and spamming empty warnings.
|
||||
const line = reader.takeDelimiterInclusive('\n') catch |outer| switch (outer) {
|
||||
error.EndOfStream => break,
|
||||
error.ReadFailed => break,
|
||||
error.StreamTooLong => reader.take(buffer.len) catch |inner| switch (inner) {
|
||||
@@ -94,7 +98,7 @@ fn openThread(io: std.Io, exe_: std.process.Child) void {
|
||||
error.EndOfStream => break,
|
||||
},
|
||||
};
|
||||
log.warn("open stderr={s}", .{line});
|
||||
log.warn("open stderr={s}", .{std.mem.trimEnd(u8, line, "\n")});
|
||||
}
|
||||
}
|
||||
_ = exe.wait(io) catch {};
|
||||
|
||||
Reference in New Issue
Block a user