Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove legacy sigchild compatibility layer #103

Merged
merged 1 commit into from
Feb 10, 2023
Merged

Remove legacy sigchild compatibility layer #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 9 additions & 40 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ as [Streams](https://github.com/reactphp/stream).
* [Command](#command)
* [Termination](#termination)
* [Custom pipes](#custom-pipes)
* [Sigchild Compatibility](#sigchild-compatibility)
* [Windows Compatibility](#windows-compatibility)
* [Install](#install)
* [Tests](#tests)
Expand Down Expand Up @@ -262,8 +261,7 @@ $process->on('exit', function ($code, $term) {
```

Note that `$code` is `null` if the process has terminated, but the exit
code could not be determined (for example
[sigchild compatibility](#sigchild-compatibility) was disabled).
code could not be determined.
Similarly, `$term` is `null` unless the process has terminated in response to
an uncaught signal sent to it.
This is not a limitation of this project, but actual how exit codes and signals
Expand Down Expand Up @@ -387,43 +385,6 @@ number of pipes and additional file descriptors, but many common applications
being run as a child process will expect that the parent process properly
assigns these file descriptors.

### Sigchild Compatibility

Internally, this project uses a work-around to improve compatibility when PHP
has been compiled with the `--enable-sigchild` option. This should not affect most
installations as this configure option is not used by default and many
distributions (such as Debian and Ubuntu) are known to not use this by default.
Some installations that use [Oracle OCI8](http://php.net/manual/en/book.oci8.php)
may use this configure option to circumvent `defunct` processes.

When PHP has been compiled with the `--enable-sigchild` option, a child process'
exit code cannot be reliably determined via `proc_close()` or `proc_get_status()`.
To work around this, we execute the child process with an additional pipe and
use that to retrieve its exit code.

This work-around incurs some overhead, so we only trigger this when necessary
and when we detect that PHP has been compiled with the `--enable-sigchild` option.
Because PHP does not provide a way to reliably detect this option, we try to
inspect output of PHP's configure options from the `phpinfo()` function.

The static `setSigchildEnabled(bool $sigchild): void` method can be used to
explicitly enable or disable this behavior like this:

```php
// advanced: not recommended by default
Process::setSigchildEnabled(true);
```

Note that all processes instantiated after this method call will be affected.
If this work-around is disabled on an affected PHP installation, the `exit`
event may receive `null` instead of the actual exit code as described above.
Similarly, some distributions are known to omit the configure options from
`phpinfo()`, so automatic detection may fail to enable this work-around in some
cases. You may then enable this explicitly as given above.

**Note:** The original functionality was taken from Symfony's
[Process](https://github.com/symfony/process) compoment.

### Windows Compatibility

Due to platform constraints, this library provides only limited support for
Expand Down Expand Up @@ -607,6 +568,14 @@ This project aims to run on any platform and thus does not require any PHP
extensions and supports running on legacy PHP 5.3 through current PHP 8+ and HHVM.
It's *highly recommended to use the latest supported PHP version* for this project.

Note that legacy platforms that use PHP compiled with the legacy `--enable-sigchild`
option may not reliably determine the child process' exit code in some cases.
This should not affect most installations as this configure option is not used
by default and most distributions (such as Debian and Ubuntu) are known to not
use this by default. This option may be used on some installations that use
[Oracle OCI8](https://www.php.net/manual/en/book.oci8.php), see `phpinfo()` output
to check if your installation might be affected.

See above note for limited [Windows Compatibility](#windows-compatibility).

## Tests
Expand Down
120 changes: 2 additions & 118 deletions src/Process.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@
/**
* Process component.
*
* This class borrows logic from Symfony's Process component for ensuring
* compatibility when PHP is compiled with the --enable-sigchild option.
*
* This class also implements the `EventEmitterInterface`
* which allows you to react to certain events:
*
Expand All @@ -40,8 +37,7 @@
* ```
*
* Note that `$code` is `null` if the process has terminated, but the exit
* code could not be determined (for example
* [sigchild compatibility](#sigchild-compatibility) was disabled).
* code could not be determined.
* Similarly, `$term` is `null` unless the process has terminated in response to
* an uncaught signal sent to it.
* This is not a limitation of this project, but actual how exit codes and signals
Expand Down Expand Up @@ -91,18 +87,13 @@ class Process extends EventEmitter
private $env;
private $fds;

private $enhanceSigchildCompatibility;
private $sigchildPipe;

private $process;
private $status;
private $exitCode;
private $fallbackExitCode;
private $stopSignal;
private $termSignal;

private static $sigchild;

/**
* Constructor.
*
Expand Down Expand Up @@ -145,7 +136,6 @@ public function __construct($cmd, $cwd = null, array $env = null, array $fds = n
}

$this->fds = $fds;
$this->enhanceSigchildCompatibility = self::isSigchildEnabled();
}

/**
Expand Down Expand Up @@ -173,23 +163,6 @@ public function start(LoopInterface $loop = null, $interval = 0.1)
$loop = $loop ?: Loop::get();
$cmd = $this->cmd;
$fdSpec = $this->fds;
$sigchild = null;

// Read exit code through fourth pipe to work around --enable-sigchild
if ($this->enhanceSigchildCompatibility) {
$fdSpec[] = array('pipe', 'w');
\end($fdSpec);
$sigchild = \key($fdSpec);

// make sure this is fourth or higher (do not mess with STDIO)
if ($sigchild < 3) {
$fdSpec[3] = $fdSpec[$sigchild];
unset($fdSpec[$sigchild]);
$sigchild = 3;
}

$cmd = \sprintf('(%s) ' . $sigchild . '>/dev/null; code=$?; echo $code >&' . $sigchild . '; exit $code', $cmd);
}

// on Windows, we do not launch the given command line in a shell (cmd.exe) by default and omit any error dialogs
// the cmd.exe shell can explicitly be given as part of the command as detailed in both documentation and tests
Expand Down Expand Up @@ -242,11 +215,6 @@ public function start(LoopInterface $loop = null, $interval = 0.1)
});
};

if ($sigchild !== null) {
$this->sigchildPipe = $pipes[$sigchild];
unset($pipes[$sigchild]);
}

foreach ($pipes as $n => $fd) {
// use open mode from stream meta data or fall back to pipe open mode for legacy HHVM
$meta = \stream_get_meta_data($fd);
Expand Down Expand Up @@ -292,11 +260,6 @@ public function close()
$pipe->close();
}

if ($this->enhanceSigchildCompatibility) {
$this->pollExitCodePipe();
$this->closeExitCodePipe();
}

$exitCode = \proc_close($this->process);
$this->process = null;

Expand Down Expand Up @@ -350,7 +313,7 @@ public function getCommand()
* will be returned if the process is still running.
*
* Null may also be returned if the process has terminated, but the exit
* code could not be determined (e.g. sigchild compatibility was disabled).
* code could not be determined.
*
* @return int|null
*/
Expand Down Expand Up @@ -437,85 +400,6 @@ public function isTerminated()
return $status !== null ? $status['signaled'] : false;
}

/**
* Return whether PHP has been compiled with the '--enable-sigchild' option.
*
* @see \Symfony\Component\Process\Process::isSigchildEnabled()
* @return bool
*/
public final static function isSigchildEnabled()
{
if (null !== self::$sigchild) {
return self::$sigchild;
}

if (!\function_exists('phpinfo')) {
return self::$sigchild = false; // @codeCoverageIgnore
}

\ob_start();
\phpinfo(INFO_GENERAL);

return self::$sigchild = false !== \strpos(\ob_get_clean(), '--enable-sigchild');
}

/**
* Enable or disable sigchild compatibility mode.
*
* Sigchild compatibility mode is required to get the exit code and
* determine the success of a process when PHP has been compiled with
* the --enable-sigchild option.
*
* @param bool $sigchild
* @return void
*/
public final static function setSigchildEnabled($sigchild)
{
self::$sigchild = (bool) $sigchild;
}

/**
* Check the fourth pipe for an exit code.
*
* This should only be used if --enable-sigchild compatibility was enabled.
*/
private function pollExitCodePipe()
{
if ($this->sigchildPipe === null) {
return;
}

$r = array($this->sigchildPipe);
$w = $e = null;

$n = @\stream_select($r, $w, $e, 0);

if (1 !== $n) {
return;
}

$data = \fread($r[0], 8192);

if (\strlen($data) > 0) {
$this->fallbackExitCode = (int) $data;
}
}

/**
* Close the fourth pipe used to relay an exit code.
*
* This should only be used if --enable-sigchild compatibility was enabled.
*/
private function closeExitCodePipe()
{
if ($this->sigchildPipe === null) {
return;
}

\fclose($this->sigchildPipe);
$this->sigchildPipe = null;
}

/**
* Return the cached process status.
*
Expand Down
18 changes: 0 additions & 18 deletions tests/AbstractProcessTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,24 +246,6 @@ public function testGetTermSignalWhenRunning($process)
$this->assertNull($process->getTermSignal());
}

public function testCommandWithEnhancedSigchildCompatibilityReceivesExitCode()
{
if (DIRECTORY_SEPARATOR === '\\') {
$this->markTestSkipped('Process pipes not supported on Windows');
}

$loop = $this->createLoop();
$old = Process::isSigchildEnabled();
Process::setSigchildEnabled(true);
$process = new Process('echo foo');
$process->start($loop);
Process::setSigchildEnabled($old);

$loop->run();

$this->assertEquals(0, $process->getExitCode());
}

public function testReceivesProcessStdoutFromEcho()
{
if (DIRECTORY_SEPARATOR === '\\') {
Expand Down