Skip to content

Commit

Permalink
Improve js_os_exec (#295)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
chqrlie authored May 9, 2024
1 parent 97be5a3 commit d378a9f
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 4 deletions.
5 changes: 5 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions compat/test-closefrom.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include <unistd.h>

int main(void) {
closefrom(3);
return 0;
}
26 changes: 22 additions & 4 deletions quickjs-libc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand All @@ -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);
Expand Down

0 comments on commit d378a9f

Please sign in to comment.