`--parallel` option fails if pcntl_waitpid returns non-managed process ID

[NanoSector]

Describe the bug

The returned PID of pcntl_waitpid is not checked against the list of known PIDs in src/Runner.php. In our setup, this call always returns a PID with a value 1 lower than the lowest expected PID in the $childProcs list which causes phpcs and phpcbf to fail.

E.g. the $childProcs list contains [5125, 5126], but the function first returns 5124 and this causes the runner to crash with the following exception:

PHP Fatal error:  Uncaught PHP_CodeSniffer\Exceptions\RuntimeException: Undefined array key 5124 in .../vendor/squizlabs/php_codesniffer/src/Runner.php on line 789 in .../vendor/squizlabs/php_codesniffer/src/Runner.php:624
Stack trace:
#0 .../vendor/squizlabs/php_codesniffer/src/Runner.php(789): PHP_CodeSniffer\Runner->handleErrors()
#1 .../vendor/squizlabs/php_codesniffer/src/Runner.php(560): PHP_CodeSniffer\Runner->processChildProcs()
#2 .../vendor/squizlabs/php_codesniffer/src/Runner.php(216): PHP_CodeSniffer\Runner->run()
#3 .../vendor/squizlabs/php_codesniffer/bin/phpcbf(14): PHP_CodeSniffer\Runner->runPHPCBF()
#4 .../vendor/bin/phpcbf(119): include('...')
#5 {main}
  thrown in .../vendor/squizlabs/php_codesniffer/src/Runner.php on line 624

Code sample

N/A, this bug happens in every project regardless of code snippets.

To reproduce

Steps to reproduce the behavior:

1. Create two test PHP files and point phpcbf to them with the following command: bash -c 'bash --init-file <(echo "cd /home/vagrant/projects/my-project") -i -t -c "vendor/bin/phpcbf --parallel=8 -p test.php test2.php"'
2. Notice the exception

Expected behavior

The parallel option works & phpcs only takes care of managed processes, even when invoked with constructs like these.

Versions (please complete the following information)

Operating System	Ubuntu 22.04
PHP version	8.2
PHP_CodeSniffer version	3.13.0
Standard	PSR12
Install type	Composer (local)

Additional context

pcntl_waitpid already returns the lower value before the first call in pcntl_fork in Runner.php so this is very likely an external factor.

Patching Runner.php with the following code is sufficient to fix the issue, but I am not sure if this is the wanted solution:

        while (count($childProcs) > 0) {
            $pid = pcntl_waitpid(0, $status);
            if ($pid <= 0) {
                continue;
            }

change to

        while (count($childProcs) > 0) {
            $pid = pcntl_waitpid(0, $status);
            if ($pid <= 0 || !\array_key_exists($pid, $childProcs)) {
                continue;
            }

Please confirm

• I have searched the issue list and am not opening a duplicate issue.
• I have read the Contribution Guidelines and this is not a support question.
• I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
• I have verified the issue still exists in the master branch of PHP_CodeSniffer.

[rodrigoprimo]

Thanks for reporting this issue, @NanoSector!

I was unable to reproduce it on an Ubuntu 24.04 installation running PHP 8.3. I tested using the following commands:

phpcs test.php test2.php --parallel=2
phpcbf test.php test2.php --parallel=2

Both PHPCS and PHPCBF worked as expected. I was also able to confirm using Xdebug that pcntl_waitpid() returns the correct PIDs. Is there anything unique to your environment that might help explain why pcntl_waitpid() is not returning the expected values? Are you able to test this on a different machine/environment to see if you can reproduce it?

As a side note, the example you provided (phpcs test.php test.php --parallel=2 - note that the command is passing the same file twice) uncovered what I believe to be another issue. I reported it here: #1113.

[NanoSector]

Thanks @rodrigoprimo, I'm not really sure what is causing this either, I hadn't looked into it that far. My assumption is that a PHP extension is starting a thread (maybe XDebug?) for some reason as there's no code preceding the phpcbf/phpcs execution.

The Ubuntu 22.04 machine in question is a slightly modified Laravel Homestead VM, but I understand that's a bit much to ask to setup to try to reproduce this issue.

I'll try to reproduce it on my personal MacBook later tonight, if I can't reproduce it there I'll make a dump of loaded php modules and PHP's ini values and compare the two, in that case I'll get back to you tomorrow.
But I highly doubt the underlying issue stems from phpcs, I think it's just exhibiting symptoms, the fix seemed small enough to report here.

Funny how this uncovered another issue 😅 I was fairly positive that I changed the file names appropriately.

[jrfnl]

@NanoSector I look forward to seeing the more detailed environment info tomorrow. I kind of doubt it will be related to Xdebug, as Xdebug is used a lot and I've not seen this issue reported before, but definitely curious to see what is causing it.

[NanoSector]

I dug a little deeper, turns out it is not a PHP environment issue but a side effect of a helper command I use to talk to the Vagrant VM from the host, written in Fish.

It eventually executes the following command:

ssh -o LogLevel=QUIET -t vagrant@192.168.56.56 -p 22 "bash --init-file <(echo \"cd /home/vagrant/projects/my-project\") -t -c \"vendor/bin/phpcbf --parallel=8\""

Running the following in the VM itself does not exhibit this behaviour:

bash --init-file <(echo "cd /home/vagrant/projects/my-project") -i -t -c "vendor/bin/phpcbf --parallel=8"

And moving the cd call into the -c argument also does not exhibit this behaviour:

ssh -o LogLevel=QUIET -t vagrant@192.168.56.56 -p 22 "bash -t -c \"cd /home/vagrant/projects/my-project && vendor/bin/phpcbf --parallel=8\""

What I think is happening is that Bash is somehow putting the file redirection into the same process group as phpcbf and this trips up the Runner. The in-VM implementation does file redirection before invoking bash with phpcbf.

This bug can be triggered from Bash without SSH by the following command:

bash -c 'bash --init-file <(echo "cd /home/vagrant/projects/my-project") -i -t -c "vendor/bin/phpcbf --parallel=8 -p"'

I do feel like phpcs/phpcbf should at least check that it's monitoring its own processes / ignore foreign processes, regardless of this unusual use case.

[jrfnl]

@NanoSector Interesting case for sure. Would there be any way this can be triggered without your custom Vagrant VM set up though ?

[NanoSector]

@jrfnl Yes, you should be able to trigger it locally using the last command I provided (replace the path with something pointing to a project with phpcs in it). This works on my MacBook without any VMs involved.