[2.2] [Process] Add getPid and sendSignal methods to Process #5391
Conversation
|
IMO, this quick fix adds an important missing feature for 2.1, and I propose to add a signal emitter method in 2.2 |
|
scheduled for 2.2. |
|
I've added support for posix signals |
| */ | ||
| public function sendSignal($sig) | ||
| { | ||
| if (true !== $this->isRunning()) { |
|
Do you really need posix to send signals? |
|
Do you mean to send signal via another |
|
If I understand correctly, yes. posix_kill() does nothing more then a |
|
posix extension is very common on *nix PHP packages. Using Sending a signal with an |
|
Is there a reason not to use proc_terminate() since it does not require an additional extension? |
|
For example, this code does not stop the process but just send the SIGCONT posix signal : posix_kill($pid, SIGCONT); |
|
My bad, you are right, |
|
By the way, I realized that after having realized multiple tests, both methods are wrong because the command pid and signal are returned against the immediate process whereas the real command is a child of the process. it's due to the line : $this->commandline = '('.$this->commandline.') 3>/dev/null; code=$?; echo $code >&3; exit $code';I think it's related to #5430 |
|
Since talking about this: Two things, 1) this makes it impossible to get back the actual command the process was constructed with which seems poor (could keep two variables), and 2) this generated another sh process that gets orphaned quite easily if the actual process is killed which just creates a memory leak. Do we actually need that and is it better than just dealing with the bug? Seems like it creates a new problem. |
|
This is a hacky patch for getting the exitcode when PHP is compiled with the --enable-sigchild option. This option seems required when dealing with Oracle driver (see this article). When this option is enabled you can not retrieve the exitcode. I have work on a dirty hack (thanks @Seldaek ) which results in this commandline : $this->commandline = '(' . $this->commandline . '&) 3>/dev/null; code=$?; echo $code >&3 & pid=$! && echo $pid >&4; wait $pid; exit $code';This allows us to retrieve the latest pid, which can be signaled with We can't just get rid off dealing with bug or Windows behavior, because this class would be unusable in many environments. |
|
You can also do it like this: if ($notWindows) {
$cmd = 'exec '.$cmd;
}
$proc = proc_open($cmd);
proc_terminate($proc, $signal);This will remove PHP's wrapper, and send signals directly to the process we are interested in. I don't know if |
|
Hello @schmittjoh The workaround introduced in symfony/process@86c20b9cab is required to get the exitcode which is required to know is the process was successful. I don't think your code will give access to it in case we're in a |
|
This code does not look very sane :) but anyway, could we somehow detect whether we are in an environment which On Sat, Sep 8, 2012 at 9:34 AM, Romain Neutron notifications@github.comwrote:
|
|
For reference, it was added in #5353 |
|
it does not seem easy to find out if the --enable-sigchild option was used at compilation, and the only solution I found is to use the output of phpinfo(INFO_GENERAL); |
|
Which output something like |
|
That's also what Jordi mentioned on the PR. In my benchmarks it takes about 50 microseconds which seems acceptable if On Sat, Sep 8, 2012 at 9:47 AM, Romain Neutron notifications@github.comwrote:
|
|
thanks for the quick benchmark @schmittjoh :) I've to go right now, I'm gonna implement this afternoon |
|
Indeed at this point given the amount of hackery needed for the sigchild support it might be best to only enable it when needed. On the other hand, it is more likely we'll have regressions since not many people will test it if it's only enabled sometimes. It might be good to have a switch like the enableWindowsCompatbilitylala() so that you can run tests with and without the sigchild workarounds at least, to see if the result is consistent even on builds without sigchild. |
|
I understand that the stuff is a workaround, but it introduces more weird behavior than it's worth in my opinion...or at the least only enable will absolutely required/requested. |
|
This PR is replaced by #5476, feel free to comment |
This PR was squashed before being merged into the master branch (closes #5476). Discussion ---------- [Process] Add signal and getPid methods Bug fix: no Feature addition: yes Backwards compatibility break: no Symfony2 tests pass: yes License of the code: MIT This PR replaces #5391 ; it adds : - sigchild compatibility mode. This means that if you activate it you have access to this exitcode of the process in case your php has been compiled with --enable-sigchild. This can happen when you deal with Oracle databases. ```php $process->setEnhanceSigchildCompatibility(true); ``` - `getPid` method to get the actual process identifier of the process ```php $process->getPid(); ``` - `signal` method to send Posix signal to the process ```php $process->signal(SIGHUP); ``` - Add optionnal `$signal` as second argument to `stop`method. If provided, the signal is sent at timeout to the process. Example of use : ```php $process->stop(5, SIGKILL); ``` Tests have been enhanced and now run both sigchild / non-sigchild mode. By the way, tests are successful with a PHP compiled with the --enable-sigchild option ;) Commits ------- 5ed2737 [Process] Add signal and getPid methods
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
This method is particularly nice for sending posix signal to a process
example of use :
I've not implement the signal sender in the Process API because it requires posix environment, but I think could be nice to have it and throw an exception in case it's not supported.
What do you think about it ?