Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [RFC] Additional context in pcntl_signal handler (was Re: pcntl_signal & sa_siginfo) »

[RFC] Additional context in pcntl_signal handler (was Re: pcntl_signal & sa_siginfo)

php.internals

Bishop Bettini

10 years ago
Hi All, David and I would like to propose a second array argument be added to signal handlers registered with pcntl_signal <http://php.net/manual/en/function.pcntl-signal.php>. The array passes through kernel-provided signal context like the process ID sending the signal. https://wiki.php.net/rfc/additional-context-in-pcntl-signal-handler RFC links to an implementation. Please try it out and let us know your thoughts. This only adds an argument, so no BC break. Maybe too late, but it'd be great to get this in 7.1. Thanks! bishop On Mon, Jun 13, 2016 at 2:18 PM, David Walker <dave@mudsite.com> wrote:

Dmitry Stogov

10 years ago
Hi, To keep maximum compatibility and eliminate unnecessary additional overhead, I would keep pcntl_signal() unchanged, but add pcntl_sigaction() with the ability to specify the need for the second argument (In the same way as POSIX does). Joe, Davey, when we stop targeting new RFCs for 7.1? Thanks. Dmitry. ________________________________ From: bishop.bettini@gmail.com <bishop.bettini@gmail.com> on behalf of Bishop Bettini <bishop@php.net> Sent: Thursday, June 23, 2016 8:18:58 PM To: PHP internals Cc: David Walker Subject: [PHP-DEV] [RFC] Additional context in pcntl_signal handler (was Re: [PHP-DEV] pcntl_signal & sa_siginfo) Hi All, David and I would like to propose a second array argument be added to signal handlers registered with pcntl_signal <http://php.net/manual/en/function.pcntl-signal.php>. The array passes through kernel-provided signal context like the process ID sending the signal. https://wiki.php.net/rfc/additional-context-in-pcntl-signal-handler RFC links to an implementation. Please try it out and let us know your thoughts. This only adds an argument, so no BC break. Maybe too late, but it'd be great to get this in 7.1. Thanks! bishop On Mon, Jun 13, 2016 at 2:18 PM, David Walker <dave@mudsite.com> wrote:

Dmitry Stogov

10 years ago
BTW: I'm not sure what pcntl_sigaction() could return as the "oldact" argument..., so may be the original proposal is good enough. ________________________________ From: Dmitry Stogov <dmitry@zend.com> Sent: Thursday, June 23, 2016 9:02:55 PM To: PHP internals; bishop@php.net; Joe Watkins; davey@php.net Cc: David Walker Subject: Re: [PHP-DEV] [RFC] Additional context in pcntl_signal handler (was Re: [PHP-DEV] pcntl_signal & sa_siginfo) Hi, To keep maximum compatibility and eliminate unnecessary additional overhead, I would keep pcntl_signal() unchanged, but add pcntl_sigaction() with the ability to specify the need for the second argument (In the same way as POSIX does). Joe, Davey, when we stop targeting new RFCs for 7.1? Thanks. Dmitry. ________________________________ From: bishop.bettini@gmail.com <bishop.bettini@gmail.com> on behalf of Bishop Bettini <bishop@php.net> Sent: Thursday, June 23, 2016 8:18:58 PM To: PHP internals Cc: David Walker Subject: [PHP-DEV] [RFC] Additional context in pcntl_signal handler (was Re: [PHP-DEV] pcntl_signal & sa_siginfo) Hi All, David and I would like to propose a second array argument be added to signal handlers registered with pcntl_signal <http://php.net/manual/en/function.pcntl-signal.php>. The array passes through kernel-provided signal context like the process ID sending the signal. https://wiki.php.net/rfc/additional-context-in-pcntl-signal-handler RFC links to an implementation. Please try it out and let us know your thoughts. This only adds an argument, so no BC break. Maybe too late, but it'd be great to get this in 7.1. Thanks! bishop On Mon, Jun 13, 2016 at 2:18 PM, David Walker <dave@mudsite.com> wrote:

David Walker

10 years ago
On Thu, Jun 23, 2016 at 12:26 PM Dmitry Stogov <dmitry@zend.com> wrote:
> BTW: I'm not sure what pcntl_sigaction() could return as the "oldact" > argument..., so may be the original proposal is good enough. > ------------------------------ > *From:* Dmitry Stogov <dmitry@zend.com> > *Sent:* Thursday, June 23, 2016 9:02:55 PM > *To:* PHP internals; bishop@php.net; Joe Watkins; davey@php.net > *Cc:* David Walker > *Subject:* Re: [PHP-DEV] [RFC] Additional context in pcntl_signal handler > (was Re: [PHP-DEV] pcntl_signal & sa_siginfo) > > Hi, > > > To keep maximum compatibility and eliminate unnecessary additional > overhead, I would keep pcntl_signal() unchanged, but add pcntl_sigaction() > with the ability to specify the need for the second argument (In the same > way as POSIX does). > > > Joe, Davey, when we stop targeting new RFCs for 7.1? >
Dmitry, I was thinking about making a separate pcntl_* method to handle the differences, but decided against doing it as i assumed that the overhead involved in a siginfo_t allocation would be marginal. Having a separate call, if everyone would prefer, wouldn't be hard to implement. Now, this being my first attempt at contributing to internals, I'm not well versed on a best-practices on benchmarking to provide metrics to my assumption. (advice very welcomed) On your second point, it's interesting you discuss the *oact. As it is today, calls to pcntl_signal() define the C level signal_handler as pcntl_signal_handler. So the *oact is always going to be the same handler, since userland can't control the C level handler. However, a similar idea was brought up in 72409 <https://bugs.php.net/bug.php?id=72409> to have the userland callable returned. I took a stab at that bug with this pullreq <https://github.com/php/php-src/pull/1952> so that the callable a user sets to a signal would be returned. I wasn't sure if it was PR-able, since it changes a return value of a method, and would have some BC-problems.
-- Dave

David Walker

10 years ago
On Thu, Jun 23, 2016 at 1:49 PM David Walker <dave@mudsite.com> wrote:
> On Thu, Jun 23, 2016 at 12:26 PM Dmitry Stogov <dmitry@zend.com> wrote: > >> BTW: I'm not sure what pcntl_sigaction() could return as the "oldact" >> argument..., so may be the original proposal is good enough. >> ------------------------------ >> *From:* Dmitry Stogov <dmitry@zend.com> >> *Sent:* Thursday, June 23, 2016 9:02:55 PM >> *To:* PHP internals; bishop@php.net; Joe Watkins; davey@php.net >> *Cc:* David Walker >> *Subject:* Re: [PHP-DEV] [RFC] Additional context in pcntl_signal >> handler (was Re: [PHP-DEV] pcntl_signal & sa_siginfo) >> >> Hi, >> >> >> To keep maximum compatibility and eliminate unnecessary additional >> overhead, I would keep pcntl_signal() unchanged, but add pcntl_sigaction() >> with the ability to specify the need for the second argument (In the same >> way as POSIX does). >> >> >> Joe, Davey, when we stop targeting new RFCs for 7.1? >> > > > Now, this being my first attempt at contributing to internals, I'm not > well versed on a best-practices on benchmarking to provide metrics to my > assumption. (advice very welcomed) >
Having run tests through callgrind there is, as expected, a small bit of overhead. The question is, how much overhead can be safely deemed negligible for ease of the language? In my basic test wherein I just define an empty function, set the handler, and trigger the signal there is just over 13m instructions executed. This change increases the instruction count by about 2000, or 0.0001%. I would assume keeping a simple pcntl_signal() with a single handler is more desirable than mitigating the slight overhead this introduces.
-- Dave

Joe Watkins

10 years ago
Afternoon, The last alpha is going out today. There will be two weeks before the first beta, which is feature freeze. Cheers Joe On Thu, Jul 7, 2016 at 3:20 PM, David Walker <dave@mudsite.com> wrote:

Bishop Bettini

10 years ago
On Thu, Jun 23, 2016 at 1:18 PM, Bishop Bettini <bishop@php.net> wrote:
> Hi All, > > David and I would like to propose a second array argument be added to > signal handlers registered with pcntl_signal > <http://php.net/manual/en/function.pcntl-signal.php>. The array passes > through kernel-provided signal context like the process ID sending the > signal. > > https://wiki.php.net/rfc/additional-context-in-pcntl-signal-handler > > RFC links to an implementation. Please try it out and let us know your > thoughts. This only adds an argument, so no BC break. Maybe too late, but > it'd be great to get this in 7.1. > > On Mon, Jun 13, 2016 at 2:18 PM, David Walker <dave@mudsite.com> wrote: > >> Hi All, >> >> Long-time lurker, first time questioner here. I'm currently working on a >> project wherein I end up forking off many subprocesses, but am trying to >> keep tabs on the progress of individual forks by having the children send >> a >> signal back up to their parent every so often. >> >> pcntl_socket, allows me to listen for expected user signals from >> processes, >> however, the handler only allows a single argument the signalnumber. I >> would like to extend the handler to accept a possible second argument. If >> compiled on a system with __siginfo_t it would use SA_SIGINFO when setting >> up the signal listener, and populate a second argument to the >> user-callback >> which could be an array with at leas sa_pid and sa_uid (contents of >> _kill). >> >> So I'd like to source the list to see if this would be something that >> would >> be welcomed, or if there would be other ideas on how to get some more info >> of the signal back to the PHP-side of the pcntl_signal. > >
Following a suggestion made earlier, sending notice that voting on this RFC will open in a few days. The discussion so far has been recorded in the RFC. Thoughts appreciated!