SIGCHLD is not always returned from proc_open #11498
Description
Description
The following code:
<?php
ini_set('display_errors', '1');
ini_set('display_startup_errors', '1');
error_reporting(E_ALL);
// store processes in an array as [$pid => $process, ...]
$processes = [];
// catch signals from child processes and unset processes from the array
pcntl_async_signals(true);
pcntl_signal(SIGCHLD, function($sig, $info) use (&$processes) {
unset($processes[$info['pid']]);
}, false);
// open 10 processes and store them in an array
foreach (range(0, 10) as $i) {
$process = proc_open('echo $$', [], $pipes); // prints the pid to STDOUT
$pid = proc_get_status($process)['pid'];
$processes[$pid] = $process;
}
// wait for all processes to exit
while(true) {
echo ' processes remaining:' . count($processes) . PHP_EOL;
if (empty($processes)) {
break;
}
sleep(1);
}
// Check that all processes have been removed.
var_dump($processes);Resulted in this output:
921
922
processes remaining:9
925
926
924
processes remaining:8
923
processes remaining:6
931
929
927
processes remaining:5
928
930
processes remaining:2
processes remaining:2
processes remaining:2
processes remaining:2
processes remaining:2
processes remaining:2
processes remaining:2
...
But I expected this output instead:
837
838
processes remaining:10
842
840
processes remaining:8
processes remaining:7
846
841
processes remaining:6
847
processes remaining:5
processes remaining:4
839
843
processes remaining:3
844
processes remaining:1
845
processes remaining:0
array(0) {
}
There were some rare cases where the script would exit with code 255 with no errors.
root@c09c0fa26c4d:/app# php test.php
933
934
936
processes remaining:8
937
935
processes remaining:7
938
939
941
processes remaining:5
940
942
943
root@c09c0fa26c4d:/app# echo $?
255
PHP Version
PHP 8.2.7 (cli) (built: Jun 13 2023 23:05:53) (NTS)
Operating System
Debian GNU/Linux 12
Activity
ndossche commentedon Jun 21, 2023
I have a feeling multiple signals get collapsed into one and a loop is missing somewhere to go over all of them.
ndossche commentedon Jun 21, 2023
Yep, that's definitely the issue. Here's a quick PoC patch that solves it for me (although testing is appreciated ^^).
Note that this patch is incomplete: there's still some optimisation opportunity + todos in the patch. However, I'm out of time for today...
taka-oyama commentedon Jun 22, 2023
Hi, thanks for looking into the issue. I tried out the patch and it seems to be working.
The script no longer hangs!
There is still ~30% chance that it ends prematurely and exits with code 255 though.
# php test.php; echo $? 3863 3864 processes remaining:9 3868 3866 processes remaining:7 3867 processes remaining:6 3865 processes remaining:5 3873 3869 3872 3870 3871 255ndossche commentedon Jun 22, 2023
I can't reproduce the exit status 255 problem.
But exit status 255 usually means there's an uncaught error somewhere in your code.
Can you try to trigger the issue with
php -d error_reporting=-1 -d display_errors=1 test.php? By giving these options all errors should be displayed. Hopefully this will reveal something.If that doesn't reveal anything, would you be able to recompile with ASAN support? i.e. pass in
--enable-address-sanitizeras a./configureoption. You'll need to set the environment variableUSE_ZEND_ALLOC=0for that to work then though.taka-oyama commentedon Jun 22, 2023
Hi, thanks for the detailed instructions.
I couldn't get the error to show up even with the options so I rebuilt PHP with ASAN support.
I was able to reproduce it after a few tries and got the following error.
ndossche commentedon Jun 22, 2023
I can reproduce the "error code 255" issue sometimes, it's very hard to trigger though, I need to repeatedly start the process in a tight loop. I'll try to debug that.
ndossche commentedon Jun 22, 2023
Okay, partially found the reason it exits
sapi_cli_single_write()fails which insapi_cli_ub_write()sets the exit status to 255 and abort the program. Now the question is why the write fails...ndossche commentedon Jun 22, 2023
Bingo, a signal arrives during the write, which causes the return value to be -1. We handle EAGAIN already but we don't handle EINTR...
That's a separate issue though, so I'm gonna split the patch into two different PRs.
15 remaining items
taka-oyama commentedon Aug 3, 2023
Hi, thanks I will give that a try.
taka-oyama commentedon Aug 19, 2023
Hi @nielsdos, I tried the code you provided and I confirmed that it works. Thank you for the workaround.
However, I noticed that
pcntl_waitdoes not provide any way of getting$siginfothat pcntl_signal's handler provides. Is there any way to get that?ndossche commentedon Aug 19, 2023
If you perform the pcntl_wait inside the signal handler you can reuse the relevant fields, most fields should be the same for all children.
taka-oyama commentedon Aug 19, 2023
I'm not sure that you can reuse the
statusfield, since that contains the exit code for each process...?If you run the code below, the exit codes don't match.
taka-oyama commentedon Aug 19, 2023
Sorry, I just noticed you can get the exit code by passing
$statusintopcntl_wexitstatus.ndossche commentedon Aug 19, 2023
The kernel doesn't give us the siginfo_t, that information is lost because of the signal merging it performs. You can indeed get the status via the pcntl function.
taka-oyama commentedon Aug 20, 2023
Ok. Understood. Thank you for replying 🙂.