Skip to content

Commit

Permalink
darwin,libuv: use posix_spawn
Browse files Browse the repository at this point in the history
Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a squashed version of libuv/libuv#3064, with the addition of API availability annotations to fix a build warning (since Electron compiles with the `-Wunguarded-availability-new` flag). This patch should be removed when libuv PR 3064 is merged.

Fixes: libuv/libuv#3050
Fixes: electron#26143
PR-URL: libuv/libuv#3064

Authored-by: Juan Pablo Canepa <jpcanepa@gmail.com>
Co-authored-by: Marcello Bastéa-Forte <marcello@descript.com>
Electron patch prepared by: Pat DeSantis <pdesantis3@gmail.com>
  • Loading branch information
pdesantis committed Dec 14, 2020
1 parent 4588a41 commit 2fa0c14
Show file tree
Hide file tree
Showing 2 changed files with 268 additions and 0 deletions.
2 changes: 2 additions & 0 deletions patches/node/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,5 @@ src_allow_embedders_to_provide_a_custom_pageallocator_to.patch
allow_preventing_preparestacktracecallback.patch
fix_add_safeforterminationscopes_for_sigint_interruptions.patch
remove_makeexternal_case_for_uncached_internal_strings.patch
chore_expose_v8_initialization_isolate_callbacks.patch
darwin_libuv_use_posix_spawn.patch
266 changes: 266 additions & 0 deletions patches/node/darwin_libuv_use_posix_spawn.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,266 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Pat DeSantis <pdesantis3@gmail.com>
Date: Mon, 14 Dec 2020 16:34:33 -0500
Subject: darwin,libuv: use posix_spawn

Spawning child processes in an Electron application with a hardened runtime has become slow in macOS Big Sur.

This patch is a squashed version of https://github.com/libuv/libuv/pull/3064, with the addition of API availability annotations to fix a build warning (since Electron compiles with the `-Wunguarded-availability-new` flag). This patch should be removed when libuv PR 3064 is merged.

Fixes: https://github.com/libuv/libuv/issues/3050
Fixes: https://github.com/electron/electron/issues/26143
PR-URL: https://github.com/libuv/libuv/pull/3064

diff --git a/deps/uv/src/unix/darwin-stub.h b/deps/uv/src/unix/darwin-stub.h
index b93cf67c59628577774cae5238c0349d59b5081a..b79c2e29fb6d58b1497851c25078b1efb6421a1e 100644
--- a/deps/uv/src/unix/darwin-stub.h
+++ b/deps/uv/src/unix/darwin-stub.h
@@ -23,6 +23,7 @@
#define UV_DARWIN_STUB_H_

#include <stdint.h>
+#include <spawn.h>

struct CFArrayCallBacks;
struct CFRunLoopSourceContext;
@@ -94,4 +95,14 @@ static const int kFSEventStreamEventFlagRootChanged = 32;
static const int kFSEventStreamEventFlagUnmount = 128;
static const int kFSEventStreamEventFlagUserDropped = 2;

+/* Copied from https://opensource.apple.com/source/xnu/xnu-6153.101.6/libsyscall/wrappers/spawn/spawn_private.h.auto.html */
+int posix_spawnattr_set_uid_np(const posix_spawnattr_t*, uid_t)
+__API_AVAILABLE(macos(10.15), ios(13.0), tvos(13.0), watchos(6.0));
+
+int posix_spawnattr_set_gid_np(const posix_spawnattr_t*, gid_t)
+__API_AVAILABLE(macos(10.15), ios(13.0), tvos(13.0), watchos(6.0));
+
+int posix_spawnattr_set_groups_np(const posix_spawnattr_t*, int, gid_t*, uid_t)
+__API_AVAILABLE(macos(10.15), ios(13.0), tvos(13.0), watchos(6.0));
+
#endif /* UV_DARWIN_STUB_H_ */
diff --git a/deps/uv/src/unix/process.c b/deps/uv/src/unix/process.c
index b021aaeba87d0b466341f40a016ef69e8beb7543..2022d00335d7f49eda556c9b00e22c15c70895cc 100644
--- a/deps/uv/src/unix/process.c
+++ b/deps/uv/src/unix/process.c
@@ -34,8 +34,11 @@
#include <poll.h>

#if defined(__APPLE__) && !TARGET_OS_IPHONE
+#include <spawn.h>
+#include <sys/kauth.h>
# include <crt_externs.h>
# define environ (*_NSGetEnviron())
+#include "darwin-stub.h"
#else
extern char **environ;
#endif
@@ -404,6 +407,182 @@ static void uv__process_child_init(const uv_process_options_t* options,
}
#endif

+#if defined(__APPLE__)
+int uv__spawn_and_init_child_posix_spawn(const uv_process_options_t* options,
+ int stdio_count,
+ int (*pipes)[2],
+ pid_t* pid)
+ __API_AVAILABLE(macos(10.15)) {
+ int fd;
+ int err;
+ sigset_t signal_set;
+ posix_spawnattr_t attrs;
+ posix_spawn_file_actions_t actions;
+ unsigned int flags;
+
+ err = posix_spawnattr_init(&attrs);
+ if(err != 0)
+ return err;
+
+ if (options->flags & UV_PROCESS_SETUID)
+ if(posix_spawnattr_set_uid_np(&attrs, options->uid) != 0)
+ return err;
+
+ if (options->flags & UV_PROCESS_SETGID)
+ if (posix_spawnattr_set_gid_np(&attrs, options->gid) != 0)
+ return err;
+
+ if(options->flags & (UV_PROCESS_SETUID | UV_PROCESS_SETGID)) {
+ /* See the comment on the call to setgroups in uv__process_child_init above
+ * for why this is not a fatal error */
+ SAVE_ERRNO(posix_spawnattr_set_groups_np(&attrs, 0, NULL, KAUTH_UID_NONE));
+ }
+
+ /* Set flags for spawn behavior
+ * 1) POSIX_SPAWN_CLOEXEC_DEFAULT: (Apple Extension) All descriptors in
+ * the parent will be treated as if they had been created with O_CLOEXEC.
+ * The only fds that will be passed on to the child are those manipulated by the file actions
+ * 2) POSIX_SPAWN_SETSIGDEF: Signals mentioned in spawn-sigdefault in the spawn attributes
+ * will be reset to behave as their default
+ * 3) POSIX_SPAWN_SETSIGMASK: Signal mask will be set to the value of spawn-sigmask in attributes
+ * 4) POSIX_SPAWN_SETSID: Make the process a new session leader if a detached session was requested. */
+ flags = POSIX_SPAWN_CLOEXEC_DEFAULT |
+ POSIX_SPAWN_SETSIGDEF |
+ POSIX_SPAWN_SETSIGMASK;
+ if(options->flags & UV_PROCESS_DETACHED)
+ flags |= POSIX_SPAWN_SETSID;
+ err = posix_spawnattr_setflags(&attrs, flags);
+ if(err != 0)
+ return err;
+
+ // Reset all signal the child to their default behavior
+ sigfillset(&signal_set);
+ err = posix_spawnattr_setsigdefault(&attrs, &signal_set);
+ if(err != 0)
+ return err;
+
+ // Reset the signal mask for all signals
+ sigemptyset(&signal_set);
+ err = posix_spawnattr_setsigmask(&attrs, &signal_set);
+ if(err != 0)
+ return err;
+
+ err = posix_spawn_file_actions_init(&actions);
+ if(err != 0)
+ return err;
+
+ if (options->cwd != NULL && posix_spawn_file_actions_addchdir_np(&actions, options->cwd))
+ return err;
+
+ // First, dupe any required fd into orbit, out of the range of
+ // the descriptors that should be mapped in.
+ for(fd = 0 ; fd < stdio_count; ++fd) {
+ if(pipes[fd][1] < 0)
+ continue;
+
+ err = posix_spawn_file_actions_adddup2(&actions, pipes[fd][1], stdio_count + fd);
+ if(err != 0)
+ return err;
+ }
+
+ // Second, move the descriptors into their respective places
+ for(fd = 0 ; fd < stdio_count; ++fd) {
+ if(pipes[fd][1] < 0)
+ continue;
+
+ err = posix_spawn_file_actions_adddup2(&actions, stdio_count + fd, fd);
+ if(err != 0)
+ return err;
+ }
+
+ // Finally, close all the superfluous descriptors
+ for(fd = 0; fd < stdio_count; ++fd) {
+ if(pipes[fd][1] < 0)
+ continue;
+
+ err = posix_spawn_file_actions_addclose(&actions, stdio_count + fd);
+ if(err != 0)
+ return err;
+ }
+
+ // Finally process the standard streams as per de documentation
+ for(fd = 0 ; fd < 3 ; ++fd) {
+ // If ignored, open as /dev/null
+ const int oflags = fd == 0 ? O_RDONLY : O_RDWR;
+ const int mode = 0;
+
+ if(pipes[fd][1] != -1)
+ continue;
+
+ err = posix_spawn_file_actions_addopen(&actions, fd, "/dev/null", oflags, mode);
+ if(err != 0)
+ return err;
+ }
+
+ // Preserve parent environment if not explicitly set
+ char ** env = options->env ? options->env : environ;
+
+ // Spawn the child
+ err = posix_spawnp(pid, options->file, &actions, &attrs, options->args, env);
+
+ return UV__ERR(err);
+}
+#endif
+
+int uv__spawn_and_init_child_fork(const uv_process_options_t* options,
+ int stdio_count,
+ int (*pipes)[2],
+ int error_fd,
+ pid_t* pid) {
+ *pid = fork();
+
+ if (*pid == -1) {
+ /* Failed to fork */
+ return UV__ERR(errno);
+ }
+
+ if (*pid == 0) {
+ /* Fork succeeded, in the child process */
+ uv__process_child_init(options, stdio_count, pipes, error_fd);
+ abort();
+ }
+
+ /* Fork succeeded, in the parent process */
+ return 0;
+}
+
+int uv__spawn_and_init_child(const uv_process_options_t* options,
+ int stdio_count,
+ int (*pipes)[2],
+ int error_fd,
+ pid_t* pid) {
+
+#if defined(__APPLE__)
+ if(__builtin_available(macOS 10.15, *)) {
+ /* Especial child process spawn case for macOS Big Sur (11.0) onwards
+ *
+ * Big Sur introduced a significant performance degradation on a call to
+ * fork/exec when the process has many pages mmaped in with MAP_JIT, like, say
+ * a javascript interpreter. Electron-based applications, for example,
+ * are impacted; though the magnitude of the impact depends on how much the
+ * app relies on subprocesses.
+ *
+ * On macOS, though, posix_spawn is implemented in a way that does not
+ * exhibit the problem. This block implements the forking and preparation
+ * logic with poxis_spawn and its related primitves. It also takes advantage of
+ * the macOS extension POSIX_SPAWN_CLOEXEC_DEFAULT that makes impossible to
+ * leak descriptors to the child process.
+ *
+ * see https://github.com/libuv/libuv/issues/3050
+ */
+ return uv__spawn_and_init_child_posix_spawn(options, stdio_count, pipes, pid);
+ } else {
+#endif
+ return uv__spawn_and_init_child_fork(options, stdio_count, pipes, error_fd, pid);
+#if defined(__APPLE__)
+ }
+#endif
+}

int uv_spawn(uv_loop_t* loop,
uv_process_t* process,
@@ -486,21 +665,16 @@ int uv_spawn(uv_loop_t* loop,

/* Acquire write lock to prevent opening new fds in worker threads */
uv_rwlock_wrlock(&loop->cloexec_lock);
- pid = fork();
-
- if (pid == -1) {
- err = UV__ERR(errno);
+
+ /* Spawn the child */
+ err = uv__spawn_and_init_child(options, stdio_count, pipes, signal_pipe[1], &pid);
+ if(err != 0) {
uv_rwlock_wrunlock(&loop->cloexec_lock);
uv__close(signal_pipe[0]);
uv__close(signal_pipe[1]);
goto error;
}
-
- if (pid == 0) {
- uv__process_child_init(options, stdio_count, pipes, signal_pipe[1]);
- abort();
- }
-
+
/* Release lock in parent process */
uv_rwlock_wrunlock(&loop->cloexec_lock);
uv__close(signal_pipe[1]);

0 comments on commit 2fa0c14

Please sign in to comment.