Skip to content

--parallel option fails if pcntl_waitpid returns non-managed process ID #1112

Open
@NanoSector

Description

@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.

Activity

rodrigoprimo

rodrigoprimo commented on Jun 4, 2025

@rodrigoprimo
Contributor

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

NanoSector commented on Jun 4, 2025

@NanoSector
Author

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

jrfnl commented on Jun 4, 2025

@jrfnl
Member

@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

NanoSector commented on Jun 5, 2025

@NanoSector
Author

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

jrfnl commented on Jun 5, 2025

@jrfnl
Member

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

NanoSector

NanoSector commented on Jun 5, 2025

@NanoSector
Author

@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.

added a commit that references this issue on Jun 6, 2025
f379b6b
added 13 commits that reference this issue on Jun 6, 2025
bf71509
4ed90f8
5391aaf
b35d0bb
1836b8b
6dd074a
6d89e6d
9f81441
6ea0ffb
4356e93
4732822
8df50e3
60b9ac1
added 3 commits that reference this issue on Jun 28, 2025
1408155
aad4288
054315d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Participants

      @rodrigoprimo@jrfnl@NanoSector

      Issue actions

        `--parallel` option fails if pcntl_waitpid returns non-managed process ID · Issue #1112 · PHPCSStandards/PHP_CodeSniffer