From 5572975ba72cc55a8c8b2240c88a8bba02d8ad42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cristian=20Rodr=C3=ADguez?= Date: Tue, 11 Jan 2022 20:49:07 +0000 Subject: [PATCH] proc_open: Use posix_spawn(3) interface on systems where it is profitable As the size of the PHP process increases, forking gets slower and memory consumption increases, degrading the performance in varying degrees. This patch makes proc_open use posix_spawn only on systems which is known to be safe, faster than the HAVE_FORK path and have posix_spawn_file_actions_addchdir_np(3) action. Non scientific benchmark shows running php own's test suite on linux completes dozens of seconds faster, the impact is probably higher on systems where posix_spawn is a syscall. Closes GH-7933 --- NEWS | 2 ++ ext/standard/config.m4 | 2 ++ ext/standard/proc_open.c | 74 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) diff --git a/NEWS b/NEWS index 68f948d4f79..0fdaff6cb23 100644 --- a/NEWS +++ b/NEWS @@ -38,6 +38,8 @@ PHP NEWS - Standard: . Added support for rounding negative places in number_format(). (Marc Bennewitz) + . Added usage of posix_spawn for proc_open when supported by OS. + (Cristian Rodriguez) - Streams: . Implemented GH-11242 (_php_stream_copy_to_mem: Allow specifying a maximum diff --git a/ext/standard/config.m4 b/ext/standard/config.m4 index 984dfcd8a25..f647a22a3f5 100644 --- a/ext/standard/config.m4 +++ b/ext/standard/config.m4 @@ -376,6 +376,8 @@ dnl PHP_CHECK_FUNC(res_search, resolv, bind, socket) +PHP_CHECK_FUNC(posix_spawn_file_actions_addchdir_np) + dnl dnl Check for strptime() dnl diff --git a/ext/standard/proc_open.c b/ext/standard/proc_open.c index 182860720c6..264a7c6250c 100644 --- a/ext/standard/proc_open.c +++ b/ext/standard/proc_open.c @@ -36,6 +36,18 @@ #include #endif +#ifdef HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP +/* Only defined on glibc >= 2.29, FreeBSD CURRENT, musl >= 1.1.24, + * MacOS Catalina or later.. + * It should be posible to modify this so it is also + * used in older systems when $cwd == NULL but care must be taken + * as at least glibc < 2.24 has a legacy implementation known + * to be really buggy. + */ +#include +#define USE_POSIX_SPAWN +#endif + /* This symbol is defined in ext/standard/config.m4. * Essentially, it is set if you HAVE_FORK || PHP_WIN32 * Other platforms may modify that configure check and add suitable #ifdefs @@ -982,6 +994,36 @@ static zend_result set_proc_descriptor_from_resource(zval *resource, descriptors } #ifndef PHP_WIN32 +#if defined(USE_POSIX_SPAWN) +static zend_result close_parentends_of_pipes(posix_spawn_file_actions_t * actions, descriptorspec_item *descriptors, int ndesc) +{ + int r; + for (int i = 0; i < ndesc; i++) { + if (descriptors[i].type != DESCRIPTOR_TYPE_STD) { + r = posix_spawn_file_actions_addclose(actions, descriptors[i].parentend); + if (r != 0) { + php_error_docref(NULL, E_WARNING, "Cannot close file descriptor %d: %s", descriptors[i].parentend, strerror(r)); + return FAILURE; + } + } + if (descriptors[i].childend != descriptors[i].index) { + r = posix_spawn_file_actions_adddup2(actions, descriptors[i].childend, descriptors[i].index); + if (r != 0) { + php_error_docref(NULL, E_WARNING, "Unable to copy file descriptor %d (for pipe) into " + "file descriptor %d: %s", descriptors[i].childend, descriptors[i].index, strerror(r)); + return FAILURE; + } + r = posix_spawn_file_actions_addclose(actions, descriptors[i].childend); + if (r != 0) { + php_error_docref(NULL, E_WARNING, "Cannot close file descriptor %d: %s", descriptors[i].childend, strerror(r)); + return FAILURE; + } + } + } + + return SUCCESS; +} +#else static zend_result close_parentends_of_pipes(descriptorspec_item *descriptors, int ndesc) { /* We are running in child process @@ -1005,6 +1047,7 @@ static zend_result close_parentends_of_pipes(descriptorspec_item *descriptors, i return SUCCESS; } #endif +#endif static void close_all_descriptors(descriptorspec_item *descriptors, int ndesc) { @@ -1216,6 +1259,37 @@ PHP_FUNCTION(proc_open) childHandle = pi.hProcess; child = pi.dwProcessId; CloseHandle(pi.hThread); +#elif defined(USE_POSIX_SPAWN) + posix_spawn_file_actions_t factions; + int r; + posix_spawn_file_actions_init(&factions); + + if (close_parentends_of_pipes(&factions, descriptors, ndesc) == FAILURE) { + posix_spawn_file_actions_destroy(&factions); + close_all_descriptors(descriptors, ndesc); + goto exit_fail; + } + + if (cwd) { + r = posix_spawn_file_actions_addchdir_np(&factions, cwd); + if (r != 0) { + php_error_docref(NULL, E_WARNING, "posix_spawn_file_actions_addchdir_np() failed: %s", strerror(r)); + } + } + + if (argv) { + r = posix_spawnp(&child, ZSTR_VAL(command_str), &factions, NULL, argv, (env.envarray ? env.envarray : environ)); + } else { + r = posix_spawn(&child, "/bin/sh" , &factions, NULL, + (char * const[]) {"sh", "-c", ZSTR_VAL(command_str), NULL}, + env.envarray ? env.envarray : environ); + } + posix_spawn_file_actions_destroy(&factions); + if (r != 0) { + close_all_descriptors(descriptors, ndesc); + php_error_docref(NULL, E_WARNING, "posix_spawn() failed: %s", strerror(r)); + goto exit_fail; + } #elif HAVE_FORK /* the Unix way */ child = fork();