Skip to content

Commit 5572975

Browse files
crrodriguezbukka
authored andcommitted
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
1 parent a077c2d commit 5572975

File tree

3 files changed

+78
-0
lines changed

3 files changed

+78
-0
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ PHP NEWS
3838
- Standard:
3939
. Added support for rounding negative places in number_format().
4040
(Marc Bennewitz)
41+
. Added usage of posix_spawn for proc_open when supported by OS.
42+
(Cristian Rodriguez)
4143

4244
- Streams:
4345
. Implemented GH-11242 (_php_stream_copy_to_mem: Allow specifying a maximum

ext/standard/config.m4

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,8 @@ dnl
376376

377377
PHP_CHECK_FUNC(res_search, resolv, bind, socket)
378378

379+
PHP_CHECK_FUNC(posix_spawn_file_actions_addchdir_np)
380+
379381
dnl
380382
dnl Check for strptime()
381383
dnl

ext/standard/proc_open.c

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,18 @@
3636
#include <fcntl.h>
3737
#endif
3838

39+
#ifdef HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP
40+
/* Only defined on glibc >= 2.29, FreeBSD CURRENT, musl >= 1.1.24,
41+
* MacOS Catalina or later..
42+
* It should be posible to modify this so it is also
43+
* used in older systems when $cwd == NULL but care must be taken
44+
* as at least glibc < 2.24 has a legacy implementation known
45+
* to be really buggy.
46+
*/
47+
#include <spawn.h>
48+
#define USE_POSIX_SPAWN
49+
#endif
50+
3951
/* This symbol is defined in ext/standard/config.m4.
4052
* Essentially, it is set if you HAVE_FORK || PHP_WIN32
4153
* 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
982994
}
983995

984996
#ifndef PHP_WIN32
997+
#if defined(USE_POSIX_SPAWN)
998+
static zend_result close_parentends_of_pipes(posix_spawn_file_actions_t * actions, descriptorspec_item *descriptors, int ndesc)
999+
{
1000+
int r;
1001+
for (int i = 0; i < ndesc; i++) {
1002+
if (descriptors[i].type != DESCRIPTOR_TYPE_STD) {
1003+
r = posix_spawn_file_actions_addclose(actions, descriptors[i].parentend);
1004+
if (r != 0) {
1005+
php_error_docref(NULL, E_WARNING, "Cannot close file descriptor %d: %s", descriptors[i].parentend, strerror(r));
1006+
return FAILURE;
1007+
}
1008+
}
1009+
if (descriptors[i].childend != descriptors[i].index) {
1010+
r = posix_spawn_file_actions_adddup2(actions, descriptors[i].childend, descriptors[i].index);
1011+
if (r != 0) {
1012+
php_error_docref(NULL, E_WARNING, "Unable to copy file descriptor %d (for pipe) into "
1013+
"file descriptor %d: %s", descriptors[i].childend, descriptors[i].index, strerror(r));
1014+
return FAILURE;
1015+
}
1016+
r = posix_spawn_file_actions_addclose(actions, descriptors[i].childend);
1017+
if (r != 0) {
1018+
php_error_docref(NULL, E_WARNING, "Cannot close file descriptor %d: %s", descriptors[i].childend, strerror(r));
1019+
return FAILURE;
1020+
}
1021+
}
1022+
}
1023+
1024+
return SUCCESS;
1025+
}
1026+
#else
9851027
static zend_result close_parentends_of_pipes(descriptorspec_item *descriptors, int ndesc)
9861028
{
9871029
/* We are running in child process
@@ -1005,6 +1047,7 @@ static zend_result close_parentends_of_pipes(descriptorspec_item *descriptors, i
10051047
return SUCCESS;
10061048
}
10071049
#endif
1050+
#endif
10081051

10091052
static void close_all_descriptors(descriptorspec_item *descriptors, int ndesc)
10101053
{
@@ -1216,6 +1259,37 @@ PHP_FUNCTION(proc_open)
12161259
childHandle = pi.hProcess;
12171260
child = pi.dwProcessId;
12181261
CloseHandle(pi.hThread);
1262+
#elif defined(USE_POSIX_SPAWN)
1263+
posix_spawn_file_actions_t factions;
1264+
int r;
1265+
posix_spawn_file_actions_init(&factions);
1266+
1267+
if (close_parentends_of_pipes(&factions, descriptors, ndesc) == FAILURE) {
1268+
posix_spawn_file_actions_destroy(&factions);
1269+
close_all_descriptors(descriptors, ndesc);
1270+
goto exit_fail;
1271+
}
1272+
1273+
if (cwd) {
1274+
r = posix_spawn_file_actions_addchdir_np(&factions, cwd);
1275+
if (r != 0) {
1276+
php_error_docref(NULL, E_WARNING, "posix_spawn_file_actions_addchdir_np() failed: %s", strerror(r));
1277+
}
1278+
}
1279+
1280+
if (argv) {
1281+
r = posix_spawnp(&child, ZSTR_VAL(command_str), &factions, NULL, argv, (env.envarray ? env.envarray : environ));
1282+
} else {
1283+
r = posix_spawn(&child, "/bin/sh" , &factions, NULL,
1284+
(char * const[]) {"sh", "-c", ZSTR_VAL(command_str), NULL},
1285+
env.envarray ? env.envarray : environ);
1286+
}
1287+
posix_spawn_file_actions_destroy(&factions);
1288+
if (r != 0) {
1289+
close_all_descriptors(descriptors, ndesc);
1290+
php_error_docref(NULL, E_WARNING, "posix_spawn() failed: %s", strerror(r));
1291+
goto exit_fail;
1292+
}
12191293
#elif HAVE_FORK
12201294
/* the Unix way */
12211295
child = fork();

0 commit comments

Comments
 (0)