[RFC] Process and chained commands in symfony 2.2

[romainneutron]

Hello,

I wanted to implement signal and getPid methods for 2.2 (see #5476).

• Posix signal are very interesting in case of timeout ; when you ask your process to stop, you could specify a timeout when a SIGKILL could be sent. (And I'm sure you got lots of ideas to use them other ways)
• Pid can be useful for interactions with other programs.

The Story :

There are two functions to implement these methods : proc_get_status and proc_terminate. Actually, these methods might not work as expected inside the Process component because PHP might not run the command directly, but wraps it with sh (described in #5030).

Reminder

Chained commands

The Process component can handle such command :

new Process('git branch -a && git fetch upstream');

Inside Process, it can be summarized as :

$descriptorspec = array(array("pipe", "r"), array("pipe", "w"), array("pipe", "a"));
$process = proc_open('git branch -a && git fetch upstream', $descriptorspec, $pipes);

The sh wrapper

Let's run a command and let's ask for the Pid

$descriptorspec = array(array("pipe", "r"), array("pipe", "w"), array("pipe", "a"));
$process = proc_open('sleep 3', $descriptorspec, $pipes);
$status = proc_get_status($process);
echo $status['pid'];

will result in :

15888

That can be monitored :

grosroro 15886  1.0  0.0 254140 16164 pts/0    S+   23:19   0:00  | \_ php proc.php     
grosroro 15888  0.0  0.0   3940   580 pts/0    S+   23:19   0:00  |     \_ sh -c sleep 3
grosroro 15889  0.0  0.0   6748   568 pts/0    S+   23:19   0:00  |         \_ sleep 3

The actual Pid is 15889, not 15888 ; The status returned by proc_get_status is not the one we expected (the status of our command), but the one of the sh wrapper. It is the same behavior with proc_terminate ; the signal is sent to the sh wrapper instead of the command.

The problem

This leads us to two conclusions :

• All the methods that returns results or use results probed by proc_get_status might be wrong (hasBeenSignaled, getTermSignal, hasBeenStopped, getStopSignal, stop).
• We can not currently easily signal a process or retrieve the pid in the current design.

  The solution

The only solution found is to prepend the command with exec :

$descriptorspec = array(array("pipe", "r"), array("pipe", "w"), array("pipe", "a"));
$process = proc_open('exec sleep 3', $descriptorspec, $pipes);
$status = proc_get_status($process);
echo $status['pid'];

will result in :

9441

That can be monitored :

grosroro  9440  1.0  0.0 254140 16188 pts/0    S+   23:48   0:00  |   \_ php proc.php
grosroro  9441  0.0  0.0   6748   564 pts/0    S+   23:48   0:00  |       \_ sleep 3

proc_terminate and proc_get_status works as expected in this case.

Backward Compatibility

This solution is quite nice, but it leads to another issue :

This was working well :

$descriptorspec = array(array("pipe", "r"), array("pipe", "w"), array("pipe", "a"));
$process = proc_open('echo "hello" && echo "world"', $descriptorspec, $pipes);
// echoes "hello\nworld";
echo fgets($pipes[1], 4096);

This is not working as expected :

$descriptorspec = array(array("pipe", "r"), array("pipe", "w"), array("pipe", "a"));
$process = proc_open('exec echo "hello" && echo "world"', $descriptorspec, $pipes);
// echoes "hello";
echo fgets($pipes[1], 4096);

Proposals

Parse command line and allow pid/signal access to non-chained commands

this would prepend command line with "exec" only for atomic commands.

• pros : preserve BC 2.1
• cons : Lots of methods might not work as expected (hasBeenSignaled, getTermSignal, hasBeenStopped, getStopSignal, stop)

  Split commands in atomic subprocesses / add a chain method / enforce use of ProcessBuilder

• pros : guarantee the control control over the processes, adds multiple possibilities for chaining strategies), fixes
• cons : breaks the BC with 2.1, disable the ability to run chained commands in the background (There is no way to add a callback when a process finish to start the next one)

Do you have any thoughts about this ?

[stof]

@romainneutron I don't see why you say that getExitCode does not return the right result. It does, according to the same rule than used by sh. ; and && are not equivalent (otherwise, we would not have 2 syntaxes)

[romainneutron]

You are correct.

[romainneutron]

I removed this block :

Chained command interpretation

In the following example we have a second problem due to chained commands. Both commands are legals with the Process component but not interpreted the same way by a shell. In bash, the exit code is 127 for $process1 and 0 for $process2.

$process1 = new Process('nonExistingCommand && ls');
$process1->run();

$process2 = new Process('nonExistingCommand ; ls');
$process2->run();

assert($process1->getExitCode() !== $process2->getExitCode());

[Seldaek]

How about a ProcessChain class that would take a list of Process/ProcessChain instances and run them one by one, it could run in AND or OR mode - so in AND mode it would stop as soon as something exits with non-zero. It would be stoppable at any time and return the current Pid of whichever subprocess it's currently running, all other calls could be forwarded to the current subprocess as well.

Nesting of ProcessChain allows you to build AND/OR combinations if really needed.

[romainneutron]

Quick summary of IRC conversation :

@schmittjoh propose to parse commands and run them atomically with a regexp like preg_split('#("(?:[^"]|"(?<=\\\\))*"|'(?:[^']|'(?<=\\\\))*'|[^\s]+)#', $cmd, null, PREG_SPLIT_*)

@Seldaek said that with such commands for i in ls foo; do echo $i && cat $i; done there would be less fun, propose to reduce magic that could leads us to weird bugs

Seldaek: the ProcessChain isn't soo complex and it could also just accept an array of commands that it turns into Process instances itself

With jordi proposal, BC is preserved

[fabpot]

I agree with @Seldaek

[jmikola]

@romainneutron: Are there implications here for the command wrapping already done for Sigchild compatibility?

See: https://github.com/symfony/Process/blob/master/Process.php#L1117

if ($this->enhanceSigchildCompatibility && $this->isSigchildEnabled()) {
    // last exit code is output on the fourth pipe and caught to work around --enable-sigchild
    $descriptors = array_merge($descriptors, array(array('pipe', 'w')));

    $this->commandline = '('.$this->commandline.') 3>/dev/null; code=$?; echo $code >&3; exit $code';
}

[romainneutron]

This commandline is a hack to get the exitcode of a process in a sigchild environment (otherwise it returns -1). Is this the answer you were looking for ?

[jmikola]

No, I was curious if using exec to effectively replace the sh process (instead of having our process launched as a child of sh) would still work if we wrap $this->commandline in parentheses.

[romainneutron]

In the case of sigchild environment, it won't work. Have a look at the output of :

use Symfony\Component\Process\Process;

$process = new Process('echo "a"; echo "b"');
$process->run();
var_dump($process->getOutput());

$process = new Process('exec (echo "a"; echo "b")');
$process->run();
var_dump($process->getOutput());

$process = new Process('exec echo "a"; echo "b"');
$process->run();
var_dump($process->getOutput());

string(4) "a\nb\n"
string(0) ""
string(2) "a\n"