Handle exec failures more gracefully (#7793)

Fixes #7792

Our error handling for `exec` failing within the forked process never
actually worked! It triggered all sorts of issues. We didn't catch this
before because it used to be exceptionally hard to fail an exec because
we used to wrap ALL commands in a `/bin/sh -c`.

However, we now support direction execution, most notably when you do
`ghostty -e <command>` but also via the `direct:` prefix on configured
commands.

This fixes up our exec failure handling by printing a useful error
message and avoiding any errdefers in the child which was causing the
double-close.
This commit is contained in:
Mitchell Hashimoto
2025-07-03 21:39:08 -07:00
committed by GitHub
2 changed files with 54 additions and 33 deletions

View File

@ -188,10 +188,31 @@ fn startPosix(self: *Command, arena: Allocator) !void {
// Finally, replace our process. // Finally, replace our process.
// Note: we must use the "p"-variant of exec here because we // Note: we must use the "p"-variant of exec here because we
// do not guarantee our command is looked up already in the path. // do not guarantee our command is looked up already in the path.
_ = posix.execvpeZ(self.path, argsZ, envp) catch null; const err = posix.execvpeZ(self.path, argsZ, envp);
// If we are executing this code, the exec failed. In that scenario, // If we are executing this code, the exec failed. We're in the
// we return a very specific error that can be detected to determine // child process so there isn't much we can do. We try to output
// something reasonable. Its important to note we MUST NOT return
// any other error condition from here on out.
const stderr = std.io.getStdErr().writer();
switch (err) {
error.FileNotFound => stderr.print(
\\Requested executable not found. Please verify the command is on
\\the PATH and try again.
\\
,
.{},
) catch {},
else => stderr.print(
\\exec syscall failed with unexpected error: {}
\\
,
.{err},
) catch {},
}
// We return a very specific error that can be detected to determine
// we're in the child. // we're in the child.
return error.ExecFailedInChild; return error.ExecFailedInChild;
} }

View File

@ -90,15 +90,13 @@ pub fn threadEnter(
// Start our subprocess // Start our subprocess
const pty_fds = self.subprocess.start(alloc) catch |err| { const pty_fds = self.subprocess.start(alloc) catch |err| {
// If we specifically got this error then we are in the forked // If we specifically got this error then we are in the forked
// process and our child failed to execute. In that case // process and our child failed to execute. If we DIDN'T
if (err != error.Termio) return err; // get this specific error then we're in the parent and
// we need to bubble it up.
if (err != error.ExecFailedInChild) return err;
// Output an error message about the exec faililng and exit. // We're in the child. Nothing more we can do but abnormal exit.
// This generally should NOT happen because we always wrap // The Command will output some additional information.
// our command execution either in login (macOS) or /bin/sh
// (Linux) which are usually guaranteed to exist. Still, we
// want to handle this scenario.
execFailedInChild() catch {};
posix.exit(1); posix.exit(1);
}; };
errdefer self.subprocess.stop(); errdefer self.subprocess.stop();
@ -272,25 +270,6 @@ pub fn resize(
return try self.subprocess.resize(grid_size, screen_size); return try self.subprocess.resize(grid_size, screen_size);
} }
/// This outputs an error message when exec failed and we are the
/// child process. This returns so the caller should probably exit
/// after calling this.
///
/// Note that this usually is only called under very very rare
/// circumstances because we wrap our command execution in login
/// (macOS) or /bin/sh (Linux). So this output can be pretty crude
/// because it should never happen. Notably, this is not the error
/// users see when `command` is invalid.
fn execFailedInChild() !void {
const stderr = std.io.getStdErr().writer();
try stderr.writeAll("exec failed\n");
try stderr.writeAll("press any key to exit\n");
var buf: [1]u8 = undefined;
var reader = std.io.getStdIn().reader();
_ = try reader.read(&buf);
}
fn processExitCommon(td: *termio.Termio.ThreadData, exit_code: u32) void { fn processExitCommon(td: *termio.Termio.ThreadData, exit_code: u32) void {
assert(td.backend == .exec); assert(td.backend == .exec);
const execdata = &td.backend.exec; const execdata = &td.backend.exec;
@ -895,6 +874,12 @@ const Subprocess = struct {
} { } {
assert(self.pty == null and self.command == null); assert(self.pty == null and self.command == null);
// This function is funny because on POSIX systems it can
// fail in the forked process. This is flipped to true if
// we're in an error state in the forked process (child
// process).
var in_child: bool = false;
// Create our pty // Create our pty
var pty = try Pty.open(.{ var pty = try Pty.open(.{
.ws_row = @intCast(self.grid_size.rows), .ws_row = @intCast(self.grid_size.rows),
@ -903,14 +888,14 @@ const Subprocess = struct {
.ws_ypixel = @intCast(self.screen_size.height), .ws_ypixel = @intCast(self.screen_size.height),
}); });
self.pty = pty; self.pty = pty;
errdefer { errdefer if (!in_child) {
if (comptime builtin.os.tag != .windows) { if (comptime builtin.os.tag != .windows) {
_ = posix.close(pty.slave); _ = posix.close(pty.slave);
} }
pty.deinit(); pty.deinit();
self.pty = null; self.pty = null;
} };
log.debug("starting command command={s}", .{self.args}); log.debug("starting command command={s}", .{self.args});
@ -1013,7 +998,22 @@ const Subprocess = struct {
.data = self, .data = self,
.linux_cgroup = self.linux_cgroup, .linux_cgroup = self.linux_cgroup,
}; };
try cmd.start(alloc);
cmd.start(alloc) catch |err| {
// We have to do this because start on Windows can't
// ever return ExecFailedInChild
const StartError = error{ExecFailedInChild} || @TypeOf(err);
switch (@as(StartError, err)) {
// If we fail in our child we need to flag it so our
// errdefers don't run.
error.ExecFailedInChild => {
in_child = true;
return err;
},
else => return err,
}
};
errdefer killCommand(&cmd) catch |err| { errdefer killCommand(&cmd) catch |err| {
log.warn("error killing command during cleanup err={}", .{err}); log.warn("error killing command during cleanup err={}", .{err});
}; };