| Q | A |
|---|
| Branch? | 5.4, 6.4, 7.0 |
| Bug fix? | yes |
| New feature? | no |
| Deprecations? | no |
| Issues | N/A |
| License | MIT |
This PR addresses a bug in Symfony's Process component affecting PHP versions prior to 8.3. In these versions, callingproc_get_status multiple times on the same process resource only returns the correct exit status on the first call, with subsequent calls returning -1 due to the process being discarded. This behavior can lead to race conditions and incorrect process status reporting in Symfony applications.
To resolve this, the PR introduces a workaround that caches the first retrieved exit status. This cached status is then used for subsequent calls, ensuring consistent and accurate exit status reporting. This change aims to align the behavior with PHP 8.3, where this issue has been resolved natively.
Changes:
- Added caching mechanism for the exit status in the Process component.
- Ensured the cached status is used for subsequent
proc_get_status calls if PHP version is below 8.3.
Proof of Concept:
To demonstrate the issue and the effectiveness of the fix, a proof of concept script is included below. This script uses Symfony's Process component to start a subprocess that outputs a message and exits with a specific status code. The script then attempts to retrieve the exit status of the process using getExitCode(). In PHP versions prior to 8.3, without the proposed fix, this script will often incorrectly report an exit code of -1 due to the race condition described earlier.
<?phpif (getenv('OUTPUT' ) ) {#echo 'This should fail when "wait" gets large';sleep(2 );echo'This should fail even when "wait" is small';die(123 );}useSymfony\Component\Process\Process;require_once__DIR__ .'/vendor/autoload.php';do {// Increasingly bigger waits.static$wait =0;$wait +=100000;echosprintf('Wait: %s',$wait ) .PHP_EOL;$p =newProcess( ['php',__FILE__ ],__DIR__, ['OUTPUT' =>1,] );$p->start(function (string$type,string$out )use ($p ) {echo$out .PHP_EOL;/** * Calling most methods in Symfony Process that triggers * updateStatus() can potentially trigger the -1 bug. * * @see Process::updateStatus() */echosprintf('Is Running: %s',$p->isRunning() ?'Yes' :'No' ) .PHP_EOL;echosprintf('Exit Code: %s',$p->getExitCode() ) .PHP_EOL;} );while ($p->isRunning() ) {usleep($wait );}if ( !$p->isSuccessful() ) {break;}}while (true );$is_started =$p->isStarted();$is_running =$p->isRunning();$exit_code =$p->getExitCode();echosprintf('Started: %s, Running: %s, Exit code: %s',$is_started,$is_running,$exit_code ) .PHP_EOL;Impact:
- This change only affects Symfony applications running on PHP versions below 8.3.
- It improves the reliability of process status reporting in these environments.
- No breaking changes or backward compatibility issues are introduced.
Testing:
I haven't added tests to this PR because:
- This bug involves racing conditions, which is hard to replicate.
- It's past 1 AM local time right now, and I've been debugging this for the past 6 hours.
- The provided "Proof of Concept" can serve as an initial check to confirm the bug.
I'm really not the type of developer that does not write tests, but I beg my pardon for the reasons above.
Considerations
- Based on theproc_open implementation, this fix might not be needed on Windows.
- You can find additional context for this bug in thisStack Overflow answer.
Uh oh!
There was an error while loading.Please reload this page.
This PR addresses a bug in Symfony's Process component affecting PHP versions prior to 8.3. In these versions, calling
proc_get_statusmultiple times on the same process resource only returns the correct exit status on the first call, with subsequent calls returning -1 due to the process being discarded. This behavior can lead to race conditions and incorrect process status reporting in Symfony applications.To resolve this, the PR introduces a workaround that caches the first retrieved exit status. This cached status is then used for subsequent calls, ensuring consistent and accurate exit status reporting. This change aims to align the behavior with PHP 8.3, where this issue has been resolved natively.
Changes:
proc_get_statuscalls if PHP version is below 8.3.Proof of Concept:
To demonstrate the issue and the effectiveness of the fix, a proof of concept script is included below. This script uses Symfony's Process component to start a subprocess that outputs a message and exits with a specific status code. The script then attempts to retrieve the exit status of the process using getExitCode(). In PHP versions prior to 8.3, without the proposed fix, this script will often incorrectly report an exit code of -1 due to the race condition described earlier.
Impact:
Testing:
I haven't added tests to this PR because:
I'm really not the type of developer that does not write tests, but I beg my pardon for the reasons above.
Considerations