Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: libuv patches to address child_process.spawn slowness (#33407)
* fix: libuv patches to address child_process.spawn slowness * chore: backport additional patches * Update .patches Co-authored-by: Jeremy Rose <nornagon@nornagon.net> Co-authored-by: deepak1556 <hop2deep@gmail.com>
- Loading branch information
1 parent
d36dc1c
commit 8b996a4
Showing
12 changed files
with
1,805 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
54 changes: 54 additions & 0 deletions
54
patches/node/macos_don_t_use_thread-unsafe_strtok_3524.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 */ | ||
} | ||
|
||
|
70 changes: 70 additions & 0 deletions
70
patches/node/process_bsd_handle_kevent_note_exit_failure_3451.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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
158
patches/node/process_fix_hang_after_note_exit_3521.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); |
Oops, something went wrong.