termio: cleanup

This commit is contained in:
Mitchell Hashimoto
2023-11-05 17:52:46 -08:00
parent 3dc2bbc9b0
commit 85a5a231f2
2 changed files with 72 additions and 42 deletions

View File

@ -1,7 +1,6 @@
const std = @import("std"); const std = @import("std");
const builtin = @import("builtin"); const builtin = @import("builtin");
const windows = @import("os/main.zig").windows; const windows = @import("os/main.zig").windows;
const fd_t = std.os.fd_t;
const c = switch (builtin.os.tag) { const c = switch (builtin.os.tag) {
.macos => @cImport({ .macos => @cImport({
@ -37,6 +36,8 @@ else
/// of Linux syscalls. The caller is responsible for detail-oriented handling /// of Linux syscalls. The caller is responsible for detail-oriented handling
/// of the returned file handles. /// of the returned file handles.
pub const PosixPty = struct { pub const PosixPty = struct {
pub const Fd = std.os.fd_t;
// https://github.com/ziglang/zig/issues/13277 // https://github.com/ziglang/zig/issues/13277
// Once above is fixed, use `c.TIOCSCTTY` // Once above is fixed, use `c.TIOCSCTTY`
const TIOCSCTTY = if (builtin.os.tag == .macos) 536900705 else c.TIOCSCTTY; const TIOCSCTTY = if (builtin.os.tag == .macos) 536900705 else c.TIOCSCTTY;
@ -44,16 +45,16 @@ pub const PosixPty = struct {
const TIOCGWINSZ = if (builtin.os.tag == .macos) 1074295912 else c.TIOCGWINSZ; const TIOCGWINSZ = if (builtin.os.tag == .macos) 1074295912 else c.TIOCGWINSZ;
/// The file descriptors for the master and slave side of the pty. /// The file descriptors for the master and slave side of the pty.
master: fd_t, master: Fd,
slave: fd_t, slave: Fd,
/// Open a new PTY with the given initial size. /// Open a new PTY with the given initial size.
pub fn open(size: winsize) !Pty { pub fn open(size: winsize) !Pty {
// Need to copy so that it becomes non-const. // Need to copy so that it becomes non-const.
var sizeCopy = size; var sizeCopy = size;
var master_fd: fd_t = undefined; var master_fd: Fd = undefined;
var slave_fd: fd_t = undefined; var slave_fd: Fd = undefined;
if (c.openpty( if (c.openpty(
&master_fd, &master_fd,
&slave_fd, &slave_fd,
@ -128,6 +129,8 @@ pub const PosixPty = struct {
/// Windows PTY creation and management. /// Windows PTY creation and management.
pub const WindowsPty = struct { pub const WindowsPty = struct {
pub const Fd = windows.HANDLE;
// Process-wide counter for pipe names // Process-wide counter for pipe names
var pipe_name_counter = std.atomic.Atomic(u32).init(1); var pipe_name_counter = std.atomic.Atomic(u32).init(1);

View File

@ -185,7 +185,7 @@ pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData {
const alloc = self.alloc; const alloc = self.alloc;
// Start our subprocess // Start our subprocess
try self.subprocess.start(alloc); const pty_fds = try self.subprocess.start(alloc);
errdefer self.subprocess.stop(); errdefer self.subprocess.stop();
const pid = pid: { const pid = pid: {
const command = self.subprocess.command orelse return error.ProcessNotStarted; const command = self.subprocess.command orelse return error.ProcessNotStarted;
@ -203,8 +203,7 @@ pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData {
errdefer alloc.destroy(ev_data_ptr); errdefer alloc.destroy(ev_data_ptr);
// Setup our stream so that we can write. // Setup our stream so that we can write.
const write_fd = if (builtin.os.tag == .windows) self.subprocess.pty.?.in_pipe else self.subprocess.pty.?.master; var stream = xev.Stream.initFd(pty_fds.write);
var stream = xev.Stream.initFd(write_fd);
errdefer stream.deinit(); errdefer stream.deinit();
// Wakeup watcher for the writer thread. // Wakeup watcher for the writer thread.
@ -264,11 +263,10 @@ pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData {
); );
// Start our reader thread // Start our reader thread
const read_fd = if (builtin.os.tag == .windows) self.subprocess.pty.?.out_pipe else self.subprocess.pty.?.master;
const read_thread = try std.Thread.spawn( const read_thread = try std.Thread.spawn(
.{}, .{},
if (builtin.os.tag == .windows) ReadThread.threadMainWindows else ReadThread.threadMainPosix, if (builtin.os.tag == .windows) ReadThread.threadMainWindows else ReadThread.threadMainPosix,
.{ read_fd, ev_data_ptr, pipe[0] }, .{ pty_fds.read, ev_data_ptr, pipe[0] },
); );
read_thread.setName("io-reader") catch {}; read_thread.setName("io-reader") catch {};
@ -278,7 +276,7 @@ pub fn threadEnter(self: *Exec, thread: *termio.Thread) !ThreadData {
.ev = ev_data_ptr, .ev = ev_data_ptr,
.read_thread = read_thread, .read_thread = read_thread,
.read_thread_pipe = pipe[1], .read_thread_pipe = pipe[1],
.read_thread_fd = if (builtin.os.tag == .windows) read_fd else {}, .read_thread_fd = if (builtin.os.tag == .windows) pty_fds.read else {},
}; };
} }
@ -296,7 +294,7 @@ pub fn threadExit(self: *Exec, data: ThreadData) void {
_ = std.os.write(data.read_thread_pipe, "x") catch |err| _ = std.os.write(data.read_thread_pipe, "x") catch |err|
log.warn("error writing to read thread quit pipe err={}", .{err}); log.warn("error writing to read thread quit pipe err={}", .{err});
if (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
if (windows.kernel32.CancelIoEx(data.read_thread_fd, null) == 0) { if (windows.kernel32.CancelIoEx(data.read_thread_fd, null) == 0) {
switch (windows.kernel32.GetLastError()) { switch (windows.kernel32.GetLastError()) {
@ -678,9 +676,13 @@ const Subprocess = struct {
const alloc = arena.allocator(); const alloc = arena.allocator();
// Determine the path to the binary we're executing // Determine the path to the binary we're executing
const default_command = if (builtin.os.tag == .windows) "cmd.exe" else "sh"; const path = try Command.expandPath(
const path = (try Command.expandPath(alloc, opts.full_config.command orelse default_command)) orelse alloc,
return error.CommandNotFound; opts.full_config.command orelse switch (builtin.os.tag) {
.windows => "cmd.exe",
else => "sh",
},
) orelse return error.CommandNotFound;
// On macOS, we launch the program as a login shell. This is a Mac-specific // On macOS, we launch the program as a login shell. This is a Mac-specific
// behavior (see other terminals). Terminals in general should NOT be // behavior (see other terminals). Terminals in general should NOT be
@ -850,7 +852,10 @@ const Subprocess = struct {
/// Start the subprocess. If the subprocess is already started this /// Start the subprocess. If the subprocess is already started this
/// will crash. /// will crash.
pub fn start(self: *Subprocess, alloc: Allocator) !void { pub fn start(self: *Subprocess, alloc: Allocator) !struct {
read: Pty.Fd,
write: Pty.Fd,
} {
assert(self.pty == null and self.command == null); assert(self.pty == null and self.command == null);
// Create our pty // Create our pty
@ -903,6 +908,11 @@ const Subprocess = struct {
// wait right now but that is fine too. This lets us read the // wait right now but that is fine too. This lets us read the
// parent and detect EOF. // parent and detect EOF.
_ = std.os.close(pty.slave); _ = std.os.close(pty.slave);
return .{
.read = pty.master,
.write = pty.master,
};
} }
// If we can't access the cwd, then don't set any cwd and inherit. // If we can't access the cwd, then don't set any cwd and inherit.
@ -937,10 +947,23 @@ const Subprocess = struct {
.data = &self.pty.?, .data = &self.pty.?,
}; };
try cmd.start(alloc); try cmd.start(alloc);
errdefer killCommand(cmd); errdefer killCommand(&cmd) catch |err| {
log.warn("error killing command during cleanup err={}", .{err});
};
log.info("started subcommand path={s} pid={?}", .{ self.path, cmd.pid }); log.info("started subcommand path={s} pid={?}", .{ self.path, cmd.pid });
self.command = cmd; self.command = cmd;
return switch (builtin.os.tag) {
.windows => .{
.read = pty.out_pipe,
.write = pty.in_pipe,
},
else => .{
.read = pty.master,
.write = pty.master,
},
};
} }
/// Called to notify that we exited externally so we can unset our /// Called to notify that we exited externally so we can unset our
@ -998,35 +1021,39 @@ const Subprocess = struct {
/// process. This doesn't wait for the child process to be exited. /// process. This doesn't wait for the child process to be exited.
fn killCommand(command: *Command) !void { fn killCommand(command: *Command) !void {
if (command.pid) |pid| { if (command.pid) |pid| {
if (builtin.os.tag == .windows) { switch (builtin.os.tag) {
if (windows.kernel32.TerminateProcess(pid, 0) == 0) { .windows => {
return windows.unexpectedError(windows.kernel32.GetLastError()); if (windows.kernel32.TerminateProcess(pid, 0) == 0) {
} return windows.unexpectedError(windows.kernel32.GetLastError());
} else {
const pgid_: ?c.pid_t = pgid: {
const pgid = c.getpgid(pid);
// Don't know why it would be zero but its not a valid pid
if (pgid == 0) break :pgid null;
// If the pid doesn't exist then... okay.
if (pgid == c.ESRCH) break :pgid null;
// If we have an error...
if (pgid < 0) {
log.warn("error getting pgid for kill", .{});
break :pgid null;
} }
},
break :pgid pgid; else => {
}; const pgid_: ?c.pid_t = pgid: {
const pgid = c.getpgid(pid);
if (pgid_) |pgid| { // Don't know why it would be zero but its not a valid pid
if (c.killpg(pgid, c.SIGHUP) < 0) { if (pgid == 0) break :pgid null;
log.warn("error killing process group pgid={}", .{pgid});
return error.KillFailed; // If the pid doesn't exist then... okay.
if (pgid == c.ESRCH) break :pgid null;
// If we have an error...
if (pgid < 0) {
log.warn("error getting pgid for kill", .{});
break :pgid null;
}
break :pgid pgid;
};
if (pgid_) |pgid| {
if (c.killpg(pgid, c.SIGHUP) < 0) {
log.warn("error killing process group pgid={}", .{pgid});
return error.KillFailed;
}
} }
} },
} }
} }
} }