Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: libuv patches to address child_process.spawn slowness #33407

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions patches/node/.patches
Expand Up @@ -33,3 +33,14 @@ darwin_bump_minimum_supported_version_to_10_15_3406.patch
fix_serdes_test.patch
fix_failing_node_js_test_on_outdated.patch
be_compatible_with_cppgc.patch
process_monitor_for_exit_with_kqueue_on_bsds_3441.patch
unix_protect_fork_in_uv_spawn_from_signals.patch
process_bsd_handle_kevent_note_exit_failure_3451.patch
reland_macos_use_posix_spawn_instead_of_fork_3257.patch
process_reset_the_signal_mask_if_the_fork_fails_3537.patch
process_only_use_f_dupfd_cloexec_if_it_is_defined_3512.patch
unix_simplify_uv_cloexec_fcntl_3492.patch
unix_remove_uv_cloexec_ioctl_3515.patch
process_simplify_uv_write_int_calls_3519.patch
macos_don_t_use_thread-unsafe_strtok_3524.patch
process_fix_hang_after_note_exit_3521.patch
54 changes: 54 additions & 0 deletions patches/node/macos_don_t_use_thread-unsafe_strtok_3524.patch
@@ -0,0 +1,54 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Wed, 9 Mar 2022 11:06:39 +0100
Subject: macos: don't use thread-unsafe strtok() (#3524)

Refs https://github.com/libuv/libuv/pull/3524

diff --git a/deps/uv/src/unix/process.c b/deps/uv/src/unix/process.c
index 8cde389b826b6b167437845eccd5fed440dcadc4..147164e7ea25abbf655452930d78ee0a714cce36 100644
--- a/deps/uv/src/unix/process.c
+++ b/deps/uv/src/unix/process.c
@@ -387,30 +387,22 @@ static void uv__spawn_init_posix_spawn_fncs(void) {


static void uv__spawn_init_can_use_setsid(void) {
- static const int MACOS_CATALINA_VERSION_MAJOR = 19;
- char version_str[256];
- char* version_major_str;
- size_t version_str_size = 256;
- int r;
- int version_major;
-
- /* Get a version string */
- r = sysctlbyname("kern.osrelease", version_str, &version_str_size, NULL, 0);
- if (r != 0)
+ int which[] = {CTL_KERN, KERN_OSRELEASE};
+ unsigned major;
+ unsigned minor;
+ unsigned patch;
+ char buf[256];
+ size_t len;
+
+ len = sizeof(buf);
+ if (sysctl(which, ARRAY_SIZE(which), buf, &len, NULL, 0))
return;

- /* Try to get the major version number. If not found
- * fall back to the fork/exec flow */
- version_major_str = strtok(version_str, ".");
- if (version_major_str == NULL)
+ /* NULL specifies to use LC_C_LOCALE */
+ if (3 != sscanf_l(buf, NULL, "%u.%u.%u", &major, &minor, &patch))
return;

- /* Parse the version major as a number. If it is greater than
- * the major version for macOS Catalina (aka macOS 10.15), then
- * the POSIX_SPAWN_SETSID flag is available */
- version_major = atoi_l(version_major_str, NULL); /* Use LC_C_LOCALE */
- if (version_major >= MACOS_CATALINA_VERSION_MAJOR)
- posix_spawn_can_use_setsid = 1;
+ posix_spawn_can_use_setsid = (major >= 19); /* macOS Catalina */
}


@@ -0,0 +1,70 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jameson Nash <vtjnash@gmail.com>
Date: Tue, 1 Feb 2022 15:27:12 -0500
Subject: process,bsd: handle kevent NOTE_EXIT failure (#3451)

The kernel may return ESRCH if the child has already exited here.
This is rather annoying, and means we must indirectly handle
notification to our event loop of the process exit.

Refs: https://github.com/libuv/libuv/pull/3441
Refs: https://github.com/libuv/libuv/pull/3257

diff --git a/deps/uv/src/unix/internal.h b/deps/uv/src/unix/internal.h
index 16be13b99f5db77741aa276e90a437ef4eb5ba32..2dcc8b32f5165dd75061a1b55cc1abd2ab93ccc9 100644
--- a/deps/uv/src/unix/internal.h
+++ b/deps/uv/src/unix/internal.h
@@ -145,7 +145,8 @@ typedef struct uv__stream_queued_fds_s uv__stream_queued_fds_t;

/* loop flags */
enum {
- UV_LOOP_BLOCK_SIGPROF = 1
+ UV_LOOP_BLOCK_SIGPROF = 0x1,
+ UV_LOOP_REAP_CHILDREN = 0x2
};

/* flags of excluding ifaddr */
diff --git a/deps/uv/src/unix/kqueue.c b/deps/uv/src/unix/kqueue.c
index 35200f17495d80ed2d19ef9f6f76bbc92ee042f6..071fe0ce0938657d0fb840af62a432352e938a8a 100644
--- a/deps/uv/src/unix/kqueue.c
+++ b/deps/uv/src/unix/kqueue.c
@@ -285,7 +285,7 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
for (i = 0; i < nfds; i++) {
ev = events + i;
if (ev->filter == EVFILT_PROC) {
- uv__wait_children(loop);
+ loop->flags |= UV_LOOP_REAP_CHILDREN;
nevents++;
continue;
}
@@ -383,6 +383,11 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
nevents++;
}

+ if (loop->flags & UV_LOOP_REAP_CHILDREN) {
+ loop->flags &= ~UV_LOOP_REAP_CHILDREN;
+ uv__wait_children(loop);
+ }
+
if (reset_timeout != 0) {
timeout = user_timeout;
reset_timeout = 0;
diff --git a/deps/uv/src/unix/process.c b/deps/uv/src/unix/process.c
index c1f6bd4b0076f0835caf83c45a6a896e7ae5def9..2920b942962357827bae9bcad23af8333e8b007f 100644
--- a/deps/uv/src/unix/process.c
+++ b/deps/uv/src/unix/process.c
@@ -507,8 +507,12 @@ int uv_spawn(uv_loop_t* loop,
#if defined(__APPLE__) || defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__)
struct kevent event;
EV_SET(&event, pid, EVFILT_PROC, EV_ADD | EV_ONESHOT, NOTE_EXIT, 0, 0);
- if (kevent(loop->backend_fd, &event, 1, NULL, 0, NULL))
- abort();
+ if (kevent(loop->backend_fd, &event, 1, NULL, 0, NULL)) {
+ if (errno != ESRCH)
+ abort();
+ /* Process already exited. Call waitpid on the next loop iteration. */
+ loop->flags |= UV_LOOP_REAP_CHILDREN;
+ }
#endif

QUEUE_INSERT_TAIL(&loop->process_handles, &process->queue);
158 changes: 158 additions & 0 deletions patches/node/process_fix_hang_after_note_exit_3521.patch
@@ -0,0 +1,158 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jameson Nash <vtjnash@gmail.com>
Date: Wed, 23 Mar 2022 15:39:38 +0900
Subject: process: fix hang after NOTE_EXIT (#3521)

Bug #3504 seems to affect more platforms than just OpenBSD. As this
seems to be a race condition in these kernels, we do not want to fail
because of it. Instead, we remove the WNOHANG flag from waitpid, and
track exactly which processes have exited. Should also be a slight speed
improvement for excessively large numbers of live children.

diff --git a/deps/uv/src/unix/kqueue.c b/deps/uv/src/unix/kqueue.c
index 071fe0ce0938657d0fb840af62a432352e938a8a..4c4d990ff5fa6c8ab937be2e4f79ccdaf90670c2 100644
--- a/deps/uv/src/unix/kqueue.c
+++ b/deps/uv/src/unix/kqueue.c
@@ -117,6 +117,7 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
unsigned int revents;
QUEUE* q;
uv__io_t* w;
+ uv_process_t* process;
sigset_t* pset;
sigset_t set;
uint64_t base;
@@ -284,12 +285,22 @@ void uv__io_poll(uv_loop_t* loop, int timeout) {
loop->watchers[loop->nwatchers + 1] = (void*) (uintptr_t) nfds;
for (i = 0; i < nfds; i++) {
ev = events + i;
+ fd = ev->ident;
+
+ /* Handle kevent NOTE_EXIT results */
if (ev->filter == EVFILT_PROC) {
- loop->flags |= UV_LOOP_REAP_CHILDREN;
+ QUEUE_FOREACH(q, &loop->process_handles) {
+ process = QUEUE_DATA(q, uv_process_t, queue);
+ if (process->pid == fd) {
+ process->flags |= UV_HANDLE_REAP;
+ loop->flags |= UV_LOOP_REAP_CHILDREN;
+ break;
+ }
+ }
nevents++;
continue;
}
- fd = ev->ident;
+
/* Skip invalidated events, see uv__platform_invalidate_fd */
if (fd == -1)
continue;
diff --git a/deps/uv/src/unix/process.c b/deps/uv/src/unix/process.c
index 147164e7ea25abbf655452930d78ee0a714cce36..c8816b85b7e531648064e739fb89257565ad64bb 100644
--- a/deps/uv/src/unix/process.c
+++ b/deps/uv/src/unix/process.c
@@ -63,12 +63,18 @@ extern char **environ;
# include "zos-base.h"
#endif

-#if defined(__APPLE__) || defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__)
+#if defined(__APPLE__) || \
+ defined(__DragonFly__) || \
+ defined(__FreeBSD__) || \
+ defined(__NetBSD__) || \
+ defined(__OpenBSD__)
#include <sys/event.h>
+#else
+#define UV_USE_SIGCHLD
#endif


-#if !(defined(__APPLE__) || defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__))
+#ifdef UV_USE_SIGCHLD
static void uv__chld(uv_signal_t* handle, int signum) {
assert(signum == SIGCHLD);
uv__wait_children(handle->loop);
@@ -80,6 +86,7 @@ void uv__wait_children(uv_loop_t* loop) {
int exit_status;
int term_signal;
int status;
+ int options;
pid_t pid;
QUEUE pending;
QUEUE* q;
@@ -93,19 +100,33 @@ void uv__wait_children(uv_loop_t* loop) {
process = QUEUE_DATA(q, uv_process_t, queue);
q = QUEUE_NEXT(q);

+#ifndef UV_USE_SIGCHLD
+ if ((process->flags & UV_HANDLE_REAP) == 0)
+ continue;
+ options = 0;
+ process->flags &= ~UV_HANDLE_REAP;
+#else
+ options = WNOHANG;
+#endif
+
do
- pid = waitpid(process->pid, &status, WNOHANG);
+ pid = waitpid(process->pid, &status, options);
while (pid == -1 && errno == EINTR);

- if (pid == 0)
+#ifdef UV_USE_SIGCHLD
+ if (pid == 0) /* Not yet exited */
continue;
+#endif

if (pid == -1) {
if (errno != ECHILD)
abort();
+ /* The child died, and we missed it. This probably means someone else
+ * stole the waitpid from us. Handle this by not handling it at all. */
continue;
}

+ assert(pid == process->pid);
process->status = status;
QUEUE_REMOVE(&process->queue);
QUEUE_INSERT_TAIL(&pending, &process->queue);
@@ -964,7 +985,7 @@ int uv_spawn(uv_loop_t* loop,
goto error;
}

-#if !(defined(__APPLE__) || defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__))
+#ifdef UV_USE_SIGCHLD
uv_signal_start(&loop->child_watcher, uv__chld, SIGCHLD);
#endif

@@ -983,13 +1004,14 @@ int uv_spawn(uv_loop_t* loop,
* fail to open a stdio handle. This ensures we can eventually reap the child
* with waitpid. */
if (exec_errorno == 0) {
-#if defined(__APPLE__) || defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__)
+#ifndef UV_USE_SIGCHLD
struct kevent event;
EV_SET(&event, pid, EVFILT_PROC, EV_ADD | EV_ONESHOT, NOTE_EXIT, 0, 0);
if (kevent(loop->backend_fd, &event, 1, NULL, 0, NULL)) {
if (errno != ESRCH)
abort();
/* Process already exited. Call waitpid on the next loop iteration. */
+ process->flags |= UV_HANDLE_REAP;
loop->flags |= UV_LOOP_REAP_CHILDREN;
}
#endif
diff --git a/deps/uv/src/uv-common.h b/deps/uv/src/uv-common.h
index 8a190bf8fa8c5a282feaf251aec2a30c95776888..6001b0cf68d0b0268b578218b664a737f43c9521 100644
--- a/deps/uv/src/uv-common.h
+++ b/deps/uv/src/uv-common.h
@@ -130,7 +130,10 @@ enum {
UV_SIGNAL_ONE_SHOT = 0x02000000,

/* Only used by uv_poll_t handles. */
- UV_HANDLE_POLL_SLOW = 0x01000000
+ UV_HANDLE_POLL_SLOW = 0x01000000,
+
+ /* Only used by uv_process_t handles. */
+ UV_HANDLE_REAP = 0x10000000
};

int uv__loop_configure(uv_loop_t* loop, uv_loop_option option, va_list ap);