From d378a9f3a583cb787c390456e27276d0ee377d23 Mon Sep 17 00:00:00 2001 From: Charlie Gordon Date: Fri, 10 May 2024 01:57:55 +0200 Subject: [PATCH] Improve `js_os_exec` (#295) - use $(shell) make command to test if closefrom() is available - use closefrom() if available in js_os_exec() - limit the fallback loop to 1024 handles to avoid costly loop on linux alpine. PR inspired by @nicolas-duteil-nova --- Makefile | 5 +++++ compat/test-closefrom.c | 6 ++++++ quickjs-libc.c | 26 ++++++++++++++++++++++---- 3 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 compat/test-closefrom.c diff --git a/Makefile b/Makefile index be0ff471a..c84bc44f2 100644 --- a/Makefile +++ b/Makefile @@ -143,6 +143,11 @@ endif ifdef CONFIG_WIN32 DEFINES+=-D__USE_MINGW_ANSI_STDIO # for standard snprintf behavior endif +ifndef CONFIG_WIN32 +ifeq ($(shell $(CC) -o /dev/null compat/test-closefrom.c 2>/dev/null && echo 1),1) +DEFINES+=-DHAVE_CLOSEFROM +endif +endif CFLAGS+=$(DEFINES) CFLAGS_DEBUG=$(CFLAGS) -O0 diff --git a/compat/test-closefrom.c b/compat/test-closefrom.c new file mode 100644 index 000000000..1324e97bf --- /dev/null +++ b/compat/test-closefrom.c @@ -0,0 +1,6 @@ +#include + +int main(void) { + closefrom(3); + return 0; +} diff --git a/quickjs-libc.c b/quickjs-libc.c index dd9f55f14..81371507f 100644 --- a/quickjs-libc.c +++ b/quickjs-libc.c @@ -3015,7 +3015,6 @@ static JSValue js_os_exec(JSContext *ctx, JSValueConst this_val, } if (pid == 0) { /* child */ - int fd_max = sysconf(_SC_OPEN_MAX); /* remap the stdin/stdout/stderr handles if necessary */ for(i = 0; i < 3; i++) { @@ -3024,9 +3023,28 @@ static JSValue js_os_exec(JSContext *ctx, JSValueConst this_val, _exit(127); } } - - for(i = 3; i < fd_max; i++) - close(i); +#if defined(HAVE_CLOSEFROM) + /* closefrom() is available on many recent unix systems: + Linux with glibc 2.34+, Solaris 9+, FreeBSD 7.3+, + NetBSD 3.0+, OpenBSD 3.5+. + Linux with the musl libc and macOS don't have it. + */ + + closefrom(3); +#else + { + /* Close the file handles manually, limit to 1024 to avoid + costly loop on linux Alpine where sysconf(_SC_OPEN_MAX) + returns a huge value 1048576. + Patch inspired by nicolas-duteil-nova. See also: + https://stackoverflow.com/questions/73229353/ + https://stackoverflow.com/questions/899038/#918469 + */ + int fd_max = min_int(sysconf(_SC_OPEN_MAX), 1024); + for(i = 3; i < fd_max; i++) + close(i); + } +#endif if (cwd) { if (chdir(cwd) < 0) _exit(127);