Skip to content

Commit 0c98279

Browse files
weltlingweltling
committed
Fixed bug #51800 proc_open on Windows hangs forever
This loop can block for some minutes, theoretically. Practially however, this is a 99% non issue for a normal use case. This is required because read() is synchronous. The PHP streams API wants to fill its internal buffers, therefore it might try to read some more data than user has demanded. Also, for a case where we want to read X bytes, but neither enough data nor EOF arrives, read() will block until it could fill the buffer. If a counterpart station runs slowly or delivers not all the data at once, read() would still be waiting. If we quit too early, we possibly could loose some data from the pipe. Thus it has to emulate the read() behaviour, but obviously not completely, just to some grade. Reading big data amount is for sure an issue on any platforms, it depends on the pipe buffer size, which is controlled by the system. On Windows, the buffer size seems to be way too small, which causes buffer congestion and a dead lock. It is essential to read the pipe descriptors simultaneously and possibly in the same order as the opposite writes them. Thus, this will work with smaller buffer data sizes passed through pipes. As MSDN states, anonymous pipes don't support asynchronous operations. Neither anonymous pipes do support select() as they are not SOCKETs but file descriptors. Consequently - bigger data sizes will need a better solution based on threads. However it is much more expencive. Maybe a better solution could be exporting a part of the internal doing as a userspace function which could perform some kind of lookahead operation on the pipe descriptor. This is just the first stone, depending on the user feedback we might go for further improvements in this area.
1 parent ef39f40 commit 0c98279

File tree

6 files changed

+328
-0
lines changed

6 files changed

+328
-0
lines changed

‎NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ PHP NEWS
99
as 6.2 (instead of 6.3)). (Christian Wenz)
1010
. Fixed bug #67633 (A foreach on an array returned from a function not doing
1111
copy-on-write). (Nikita)
12+
. Fixed bug #51800 (proc_open on Windows hangs forever). (Anatol)
1213

1314
- FPM:
1415
. Fixed bug #65641 (PHP-FPM incorrectly defines the SCRIPT_NAME variable

‎ext/standard/tests/streams/proc_open_bug51800.phpt

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
--TEST--
2+
Bug #51800 proc_open on Windows hangs forever
3+
--SKIPIF--
4+
<?php
5+
if (getenv("SKIP_SLOW_TESTS")) {
6+
die("skip slow test");
7+
}
8+
?>
9+
--XFAIL--
10+
pipes have to be read/written simultaneously
11+
--FILE--
12+
<?php
13+
/* This is the wrong way to do it. The parent will block till it has read all the STDIN.
14+
The smaller the pipe buffer is, the longer it will take. It might even pass at the end,
15+
after taking inappropriately long. Pipes have to be read simultaneously in smaller chunks,
16+
so then the pipe buffer is emptied more often and the child has chance to continue its
17+
write. The behaviour might look some better if write/read in a separate thread, however
18+
this is much more resource greedy and complexer to integrate into the user script. */
19+
20+
$callee = dirname(__FILE__) . "/process" . md5(uniqid()) . ".php";
21+
$php = PHP_BINARY;
22+
$cmd = "$php $callee";
23+
24+
$status;
25+
$stdout = "";
26+
$stderr = "";
27+
$pipes = array();
28+
29+
$descriptors = array(
30+
0 => array("pipe", "rb"), // stdin
31+
1 => array("pipe", "wb"), // stdout
32+
2 => array("pipe", "wb") // stderr
33+
);
34+
35+
/* create the proc file */
36+
$r = file_put_contents($callee, '<?php
37+
38+
$how_much = 10000;
39+
40+
$data0 = str_repeat("a", $how_much);
41+
$data1 = str_repeat("b", $how_much);
42+
fwrite(STDOUT, $data0);
43+
fwrite(STDERR, $data1);
44+
45+
exit(0);
46+
');
47+
48+
if (!$r) {
49+
die("couldn't create helper script '$callee'");
50+
}
51+
52+
$process = proc_open($cmd, $descriptors, $pipes);
53+
54+
if (is_resource($process))
55+
{
56+
fclose($pipes[0]);
57+
58+
while (!feof($pipes[1]))
59+
$stdout .= fread($pipes[1], 1024);
60+
fclose($pipes[1]);
61+
62+
while (!feof($pipes[2]))
63+
$stderr .= fread($pipes[2], 1024);
64+
fclose($pipes[2]);
65+
66+
$status = proc_close($process);
67+
}
68+
69+
var_dump(array(
70+
"status" => $status,
71+
"stdout" => $stdout,
72+
"stderr" => $stderr,
73+
), strlen($stdout), strlen($stderr));
74+
75+
unlink($callee);
76+
77+
?>
78+
===DONE===
79+
--EXPECTF--
80+
array(3) {
81+
["status"]=>
82+
int(0)
83+
["stdout"]=>
84+
string(10000) "a%s"
85+
["stderr"]=>
86+
string(10000) "b%s"
87+
}
88+
int(10000)
89+
int(10000)
90+
===DONE===
91+

‎ext/standard/tests/streams/proc_open_bug51800_right.phpt

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
--TEST--
2+
Bug #51800 proc_open on Windows hangs forever, the right way to do it
3+
--FILE--
4+
<?php
5+
$callee = dirname(__FILE__) . "/process" . md5(uniqid()) . ".php";
6+
$php = PHP_BINARY;
7+
$cmd = "$php $callee";
8+
9+
$status;
10+
$stdout = "";
11+
$stderr = "";
12+
$pipes = array();
13+
14+
$descriptors = array(
15+
0 => array("pipe", "rb"), // stdin
16+
1 => array("pipe", "wb"), // stdout
17+
2 => array("pipe", "wb") // stderr
18+
);
19+
20+
/* create the proc file */
21+
$r = file_put_contents($callee, '<?php
22+
23+
$how_much = 10000;
24+
25+
$data0 = str_repeat("a", $how_much);
26+
$data1 = str_repeat("b", $how_much);
27+
fwrite(STDOUT, $data0);
28+
fwrite(STDERR, $data1);
29+
30+
exit(0);
31+
');
32+
33+
if (!$r) {
34+
die("couldn't create helper script '$callee'");
35+
}
36+
37+
$process = proc_open($cmd, $descriptors, $pipes);
38+
39+
if (is_resource($process))
40+
{
41+
fclose($pipes[0]);
42+
43+
while (!feof($pipes[1]) || !feof($pipes[2])) {
44+
$stdout .= fread($pipes[1], 1024);
45+
$stderr .= fread($pipes[2], 1024);
46+
}
47+
fclose($pipes[1]);
48+
fclose($pipes[2]);
49+
50+
$status = proc_close($process);
51+
}
52+
53+
var_dump(array(
54+
"status" => $status,
55+
"stdout" => $stdout,
56+
"stderr" => $stderr,
57+
), strlen($stdout), strlen($stderr));
58+
59+
unlink($callee);
60+
61+
?>
62+
===DONE===
63+
--EXPECTF--
64+
array(3) {
65+
["status"]=>
66+
int(0)
67+
["stdout"]=>
68+
string(10000) "a%s"
69+
["stderr"]=>
70+
string(10000) "b%s"
71+
}
72+
int(10000)
73+
int(10000)
74+
===DONE===
75+

‎ext/standard/tests/streams/proc_open_bug60120.phpt

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
--TEST--
2+
Bug #60120 proc_open hangs with stdin/out with 2048+ bytes
3+
--FILE--
4+
<?php
5+
error_reporting(E_ALL);
6+
7+
$cmd = PHP_BINARY . ' -n -r "fwrite(STDOUT, $in = file_get_contents(\'php://stdin\')); fwrite(STDERR, $in);"';
8+
$descriptors = array(array('pipe', 'r'), array('pipe', 'w'), array('pipe', 'w'));
9+
$stdin = str_repeat('*', 1024 * 16) . '!';
10+
$stdin = str_repeat('*', 2049 );
11+
12+
$options = array_merge(array('suppress_errors' => true, 'binary_pipes' => true, 'bypass_shell' => false));
13+
$process = proc_open($cmd, $descriptors, $pipes, getcwd(), array(), $options);
14+
15+
foreach ($pipes as $pipe) {
16+
stream_set_blocking($pipe, false);
17+
}
18+
$writePipes = array($pipes[0]);
19+
$stdinLen = strlen($stdin);
20+
$stdinOffset = 0;
21+
22+
unset($pipes[0]);
23+
24+
while ($pipes || $writePipes) {
25+
$r = $pipes;
26+
$w = $writePipes;
27+
$e = null;
28+
$n = stream_select($r, $w, $e, 60);
29+
30+
if (false === $n) {
31+
break;
32+
} elseif ($n === 0) {
33+
proc_terminate($process);
34+
35+
}
36+
if ($w) {
37+
$written = fwrite($writePipes[0], (binary)substr($stdin, $stdinOffset), 8192);
38+
if (false !== $written) {
39+
$stdinOffset += $written;
40+
}
41+
if ($stdinOffset >= $stdinLen) {
42+
fclose($writePipes[0]);
43+
$writePipes = null;
44+
}
45+
}
46+
47+
foreach ($r as $pipe) {
48+
$type = array_search($pipe, $pipes);
49+
$data = fread($pipe, 8192);
50+
var_dump($data);
51+
if (false === $data || feof($pipe)) {
52+
fclose($pipe);
53+
unset($pipes[$type]);
54+
}
55+
}
56+
}
57+
58+
59+
?>
60+
===DONE===
61+
--EXPECTF--
62+
string(2049) "%s"
63+
string(2049) "%s"
64+
string(0) ""
65+
string(0) ""
66+
===DONE===
67+

‎ext/standard/tests/streams/proc_open_bug64438.phpt

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
--TEST--
2+
Bug #64438 proc_open hangs with stdin/out with 4097+ bytes
3+
--FILE--
4+
<?php
5+
6+
error_reporting(E_ALL);
7+
8+
$cmd = PHP_BINARY . ' -n -r "fwrite(STDOUT, $in = file_get_contents(\'php://stdin\')); fwrite(STDERR, $in);"';
9+
$descriptors = array(array('pipe', 'r'), array('pipe', 'w'), array('pipe', 'w'));
10+
$stdin = str_repeat('*', 4097);
11+
12+
$options = array_merge(array('suppress_errors' => true, 'binary_pipes' => true, 'bypass_shell' => false));
13+
$process = proc_open($cmd, $descriptors, $pipes, getcwd(), array(), $options);
14+
15+
foreach ($pipes as $pipe) {
16+
stream_set_blocking($pipe, false);
17+
}
18+
$writePipes = array($pipes[0]);
19+
$stdinLen = strlen($stdin);
20+
$stdinOffset = 0;
21+
22+
unset($pipes[0]);
23+
24+
while ($pipes || $writePipes) {
25+
$r = $pipes;
26+
$w = $writePipes;
27+
$e = null;
28+
$n = stream_select($r, $w, $e, 60);
29+
30+
if (false === $n) {
31+
break;
32+
} elseif ($n === 0) {
33+
proc_terminate($process);
34+
35+
}
36+
if ($w) {
37+
$written = fwrite($writePipes[0], (binary)substr($stdin, $stdinOffset), 8192);
38+
if (false !== $written) {
39+
$stdinOffset += $written;
40+
}
41+
if ($stdinOffset >= $stdinLen) {
42+
fclose($writePipes[0]);
43+
$writePipes = null;
44+
}
45+
}
46+
47+
foreach ($r as $pipe) {
48+
$type = array_search($pipe, $pipes);
49+
$data = fread($pipe, 8192);
50+
var_dump($data);
51+
if (false === $data || feof($pipe)) {
52+
fclose($pipe);
53+
unset($pipes[$type]);
54+
}
55+
}
56+
}
57+
58+
?>
59+
===DONE===
60+
--EXPECTF--
61+
string(4097) "%s"
62+
string(4097) "%s"
63+
string(0) ""
64+
string(0) ""
65+
===DONE===
66+

‎main/streams/plain_wrapper.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,34 @@ static size_t php_stdiop_read(php_stream *stream, char *buf, size_t count TSRMLS
348348
assert(data != NULL);
349349

350350
if (data->fd >= 0) {
351+
#ifdef PHP_WIN32
352+
php_stdio_stream_data *self = (php_stdio_stream_data*)stream->abstract;
353+
354+
if (self->is_pipe || self->is_process_pipe) {
355+
HANDLE ph = (HANDLE)_get_osfhandle(data->fd);
356+
int retry = 0;
357+
DWORD avail_read = 0;
358+
359+
do {
360+
/* Look ahead to get the available data amount to read. Do the same
361+
as read() does, however not blocking forever. In case it failed,
362+
no data will be read (better than block). */
363+
if (!PeekNamedPipe(ph, NULL, 0, NULL, &avail_read, NULL)) {
364+
break;
365+
}
366+
/* If there's nothing to read, wait in 100ms periods. */
367+
if (0 == avail_read) {
368+
usleep(100000);
369+
}
370+
} while (0 == avail_read && retry++ < 180);
371+
372+
/* Reduce the required data amount to what is available, otherwise read()
373+
will block.*/
374+
if (avail_read < count) {
375+
count = avail_read;
376+
}
377+
}
378+
#endif
351379
ret = read(data->fd, buf, count);
352380

353381
if (ret == (size_t)-1 && errno == EINTR) {

0 commit comments

Comments
 (0)