Skip to content

Commit 91c2a01

Browse files
committed
eliminate posix_spawn from the standard library
Today I found out that posix_spawn is trash. It's actually implemented on top of fork/exec inside of libc (or libSystem in the case of macOS). So, anything posix_spawn can do, we can do better. In particular, what we can do better is handle spawning of child processes that are potentially foreign binaries. If you try to spawn a wasm binary, for example, posix spawn does the following: * Goes ahead and creates a child process. * The child process writes "foo.wasm: foo.wasm: cannot execute binary file" to stderr (yes, it prints the filename twice). * The child process then exits with code 126. This behavior is indistinguishable from the binary being successfully spawned, and then printing to stderr, and exiting with a failure - something that is an extremely common occurrence. Meanwhile, using the lower level fork/exec will simply return ENOEXEC code from the execve syscall (which is mapped to zig error.InvalidExe). The posix_spawn behavior means the zig build runner can't tell the difference between a failure to run a foreign binary, and a binary that did run, but failed in some other fashion. This is unacceptable, because attempting to excecve is the proper way to support things like Rosetta.
1 parent 95f6a59 commit 91c2a01

File tree

4 files changed

+3
-420
lines changed

4 files changed

+3
-420
lines changed

CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,6 @@ set(ZIG_STAGE2_SOURCES
303303
"${CMAKE_SOURCE_DIR}/lib/std/os/linux.zig"
304304
"${CMAKE_SOURCE_DIR}/lib/std/os/linux/io_uring.zig"
305305
"${CMAKE_SOURCE_DIR}/lib/std/os/linux/x86_64.zig"
306-
"${CMAKE_SOURCE_DIR}/lib/std/os/posix_spawn.zig"
307306
"${CMAKE_SOURCE_DIR}/lib/std/os/windows.zig"
308307
"${CMAKE_SOURCE_DIR}/lib/std/os/windows/ntstatus.zig"
309308
"${CMAKE_SOURCE_DIR}/lib/std/os/windows/win32error.zig"

lib/std/child_process.zig

Lines changed: 2 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ pub const ChildProcess = struct {
9090
os.SetIdError ||
9191
os.ChangeCurDirError ||
9292
windows.CreateProcessError ||
93-
windows.WaitForSingleObjectError ||
94-
os.posix_spawn.Error;
93+
windows.WaitForSingleObjectError;
9594

9695
pub const Term = union(enum) {
9796
Exited: u8,
@@ -143,10 +142,6 @@ pub const ChildProcess = struct {
143142
@compileError("the target operating system cannot spawn processes");
144143
}
145144

146-
if (comptime builtin.target.isDarwin()) {
147-
return self.spawnMacos();
148-
}
149-
150145
if (builtin.os.tag == .windows) {
151146
return self.spawnWindows();
152147
} else {
@@ -337,10 +332,7 @@ pub const ChildProcess = struct {
337332
}
338333

339334
fn waitUnwrapped(self: *ChildProcess) !void {
340-
const res: os.WaitPidResult = if (comptime builtin.target.isDarwin())
341-
try os.posix_spawn.waitpid(self.id, 0)
342-
else
343-
os.waitpid(self.id, 0);
335+
const res: os.WaitPidResult = os.waitpid(self.id, 0);
344336
const status = res.status;
345337
self.cleanupStreams();
346338
self.handleWaitResult(status);
@@ -416,121 +408,6 @@ pub const ChildProcess = struct {
416408
Term{ .Unknown = status };
417409
}
418410

419-
fn spawnMacos(self: *ChildProcess) SpawnError!void {
420-
const pipe_flags = if (io.is_async) os.O.NONBLOCK else 0;
421-
const stdin_pipe = if (self.stdin_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined;
422-
errdefer if (self.stdin_behavior == StdIo.Pipe) destroyPipe(stdin_pipe);
423-
424-
const stdout_pipe = if (self.stdout_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined;
425-
errdefer if (self.stdout_behavior == StdIo.Pipe) destroyPipe(stdout_pipe);
426-
427-
const stderr_pipe = if (self.stderr_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined;
428-
errdefer if (self.stderr_behavior == StdIo.Pipe) destroyPipe(stderr_pipe);
429-
430-
const any_ignore = (self.stdin_behavior == StdIo.Ignore or self.stdout_behavior == StdIo.Ignore or self.stderr_behavior == StdIo.Ignore);
431-
const dev_null_fd = if (any_ignore)
432-
os.openZ("/dev/null", os.O.RDWR, 0) catch |err| switch (err) {
433-
error.PathAlreadyExists => unreachable,
434-
error.NoSpaceLeft => unreachable,
435-
error.FileTooBig => unreachable,
436-
error.DeviceBusy => unreachable,
437-
error.FileLocksNotSupported => unreachable,
438-
error.BadPathName => unreachable, // Windows-only
439-
error.InvalidHandle => unreachable, // WASI-only
440-
error.WouldBlock => unreachable,
441-
else => |e| return e,
442-
}
443-
else
444-
undefined;
445-
defer if (any_ignore) os.close(dev_null_fd);
446-
447-
var attr = try os.posix_spawn.Attr.init();
448-
defer attr.deinit();
449-
var flags: u16 = os.darwin.POSIX_SPAWN_SETSIGDEF | os.darwin.POSIX_SPAWN_SETSIGMASK;
450-
if (self.disable_aslr) {
451-
flags |= os.darwin._POSIX_SPAWN_DISABLE_ASLR;
452-
}
453-
if (self.start_suspended) {
454-
flags |= os.darwin.POSIX_SPAWN_START_SUSPENDED;
455-
}
456-
try attr.set(flags);
457-
458-
var actions = try os.posix_spawn.Actions.init();
459-
defer actions.deinit();
460-
461-
try setUpChildIoPosixSpawn(self.stdin_behavior, &actions, stdin_pipe, os.STDIN_FILENO, dev_null_fd);
462-
try setUpChildIoPosixSpawn(self.stdout_behavior, &actions, stdout_pipe, os.STDOUT_FILENO, dev_null_fd);
463-
try setUpChildIoPosixSpawn(self.stderr_behavior, &actions, stderr_pipe, os.STDERR_FILENO, dev_null_fd);
464-
465-
if (self.cwd_dir) |cwd| {
466-
try actions.fchdir(cwd.fd);
467-
} else if (self.cwd) |cwd| {
468-
try actions.chdir(cwd);
469-
}
470-
471-
var arena_allocator = std.heap.ArenaAllocator.init(self.allocator);
472-
defer arena_allocator.deinit();
473-
const arena = arena_allocator.allocator();
474-
475-
const argv_buf = try arena.allocSentinel(?[*:0]u8, self.argv.len, null);
476-
for (self.argv, 0..) |arg, i| argv_buf[i] = (try arena.dupeZ(u8, arg)).ptr;
477-
478-
const envp = if (self.env_map) |env_map| m: {
479-
const envp_buf = try createNullDelimitedEnvMap(arena, env_map);
480-
break :m envp_buf.ptr;
481-
} else std.c.environ;
482-
483-
const pid = try os.posix_spawn.spawnp(self.argv[0], actions, attr, argv_buf, envp);
484-
485-
if (self.stdin_behavior == StdIo.Pipe) {
486-
self.stdin = File{ .handle = stdin_pipe[1] };
487-
} else {
488-
self.stdin = null;
489-
}
490-
if (self.stdout_behavior == StdIo.Pipe) {
491-
self.stdout = File{ .handle = stdout_pipe[0] };
492-
} else {
493-
self.stdout = null;
494-
}
495-
if (self.stderr_behavior == StdIo.Pipe) {
496-
self.stderr = File{ .handle = stderr_pipe[0] };
497-
} else {
498-
self.stderr = null;
499-
}
500-
501-
self.id = pid;
502-
self.term = null;
503-
504-
if (self.stdin_behavior == StdIo.Pipe) {
505-
os.close(stdin_pipe[0]);
506-
}
507-
if (self.stdout_behavior == StdIo.Pipe) {
508-
os.close(stdout_pipe[1]);
509-
}
510-
if (self.stderr_behavior == StdIo.Pipe) {
511-
os.close(stderr_pipe[1]);
512-
}
513-
}
514-
515-
fn setUpChildIoPosixSpawn(
516-
stdio: StdIo,
517-
actions: *os.posix_spawn.Actions,
518-
pipe_fd: [2]i32,
519-
std_fileno: i32,
520-
dev_null_fd: i32,
521-
) !void {
522-
switch (stdio) {
523-
.Pipe => {
524-
const idx: usize = if (std_fileno == 0) 0 else 1;
525-
try actions.dup2(pipe_fd[idx], std_fileno);
526-
try actions.close(pipe_fd[1 - idx]);
527-
},
528-
.Close => try actions.close(std_fileno),
529-
.Inherit => {},
530-
.Ignore => try actions.dup2(dev_null_fd, std_fileno),
531-
}
532-
}
533-
534411
fn spawnPosix(self: *ChildProcess) SpawnError!void {
535412
const pipe_flags = if (io.is_async) os.O.NONBLOCK else 0;
536413
const stdin_pipe = if (self.stdin_behavior == StdIo.Pipe) try os.pipe2(pipe_flags) else undefined;

lib/std/os.zig

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ pub const plan9 = @import("os/plan9.zig");
4141
pub const uefi = @import("os/uefi.zig");
4242
pub const wasi = @import("os/wasi.zig");
4343
pub const windows = @import("os/windows.zig");
44-
pub const posix_spawn = @import("os/posix_spawn.zig");
4544
pub const ptrace = @import("os/ptrace.zig");
4645

4746
comptime {
@@ -56,7 +55,6 @@ test {
5655
}
5756
_ = wasi;
5857
_ = windows;
59-
_ = posix_spawn;
6058

6159
_ = @import("os/test.zig");
6260
}
@@ -3998,8 +3996,7 @@ pub const WaitPidResult = struct {
39983996
};
39993997

40003998
/// Use this version of the `waitpid` wrapper if you spawned your child process using explicit
4001-
/// `fork` and `execve` method. If you spawned your child process using `posix_spawn` method,
4002-
/// use `std.os.posix_spawn.waitpid` instead.
3999+
/// `fork` and `execve` method.
40034000
pub fn waitpid(pid: pid_t, flags: u32) WaitPidResult {
40044001
const Status = if (builtin.link_libc) c_int else u32;
40054002
var status: Status = undefined;

0 commit comments

Comments
 (0)