Applied-Software/ghostty
5
0
Fork 0
mirror of https://github.com/ghostty-org/ghostty.git synced 2025-07-14 15:56:13 +03:00
Code Issues Packages Projects Releases Wiki Activity

termio/exec: fix SIGPIPE crash when reader exits early

Browse Source 
If the read thread has already exited, it will have closed the read end
of the quit pipe. Unless SIGPIPE is masked with signal(SIGPIPE, SIG_IGN),
or the macOS-specific fcntl(F_SETNOSIGPIPE), writing to the write end of
a broken pipe kills the writer with SIGPIPE instead of returning -EPIPE
as an error. This causes a crash if the read thread exits before
threadExit.

This was already a possible race condition if read() returns
error.NotOpenForReading or error.InputOutput, but it's now much easier to
trigger due to the recent "termio/exec: fix 100% CPU usage after
wait-after-command process exits" fix.

Fix this by closing the quit pipe instead of writing to it.
This commit is contained in:
Danny Lin
2024-12-31 02:01:46 -08:00
committed by Mitchell Hashimoto
parent 542c655348
commit 3e24e96af5
1 changed files with 16 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/termio/Exec.zig
View File

@ -179,8 +179,11 @@ pub fn threadExit(self: *Exec, td: *termio.Termio.ThreadData) void {
// Quit our read thread after exiting the subprocess so that // Quit our read thread after exiting the subprocess so that
// we don't get stuck waiting for data to stop flowing if it is // we don't get stuck waiting for data to stop flowing if it is
// a particularly noisy process. // a particularly noisy process.
_ = posix.write(exec.read_thread_pipe, "x") catch |err| if (exec.read_thread_pipe) |pipe| {
log.warn("error writing to read thread quit pipe err={}", .{err}); posix.close(pipe);
// Tell deinit that we've already closed the pipe
exec.read_thread_pipe = null;
}
if (comptime builtin.os.tag == .windows) { if (comptime builtin.os.tag == .windows) {
// Interrupt the blocking read so the thread can see the quit message // Interrupt the blocking read so the thread can see the quit message
@ -639,7 +642,7 @@ pub const ThreadData = struct {
/// Reader thread state /// Reader thread state
read_thread: std.Thread, read_thread: std.Thread,
read_thread_pipe: posix.fd_t, read_thread_pipe: ?posix.fd_t,
read_thread_fd: posix.fd_t, read_thread_fd: posix.fd_t,
/// The timer to detect termios state changes. /// The timer to detect termios state changes.
@ -652,7 +655,8 @@ pub const ThreadData = struct {
termios_mode: ptypkg.Mode = .{}, termios_mode: ptypkg.Mode = .{},
pub fn deinit(self: *ThreadData, alloc: Allocator) void { pub fn deinit(self: *ThreadData, alloc: Allocator) void {
posix.close(self.read_thread_pipe); // If the pipe isn't closed, close it.
if (self.read_thread_pipe) |pipe| posix.close(pipe);
// Clear our write pools. We know we aren't ever going to do // Clear our write pools. We know we aren't ever going to do
// any more IO since we stop our data stream below so we can just // any more IO since we stop our data stream below so we can just
@ -1433,9 +1437,12 @@ pub const ReadThread = struct {
}; };
// This happens on macOS instead of WouldBlock when the // This happens on macOS instead of WouldBlock when the
// child process dies. To be safe, we just break the loop // child process dies. It's equivalent to NotOpenForReading
// and let our poll happen. // so we can just exit.
if (n == 0) break; if (n == 0) {
log.info("io reader exiting", .{});
return;
}
// log.info("DATA: {d}", .{n}); // log.info("DATA: {d}", .{n});
@call(.always_inline, termio.Termio.processOutput, .{ io, buf[0..n] }); @call(.always_inline, termio.Termio.processOutput, .{ io, buf[0..n] });
@ -1447,8 +1454,8 @@ pub const ReadThread = struct {
return; return;
}; };
// If our quit fd is set, we're done. // If our quit fd is closed, we're done.
if (pollfds[1].revents & posix.POLL.IN != 0) { if (pollfds[1].revents & posix.POLL.HUP != 0) {
log.info("read thread got quit signal", .{}); log.info("read thread got quit signal", .{});
return; return;
} }