Repository navigation
Possible of dealing with SignalEvent #8
Description
Usually a script using event loop should be able to deal with signals. Instead of using pcntl functions, can we open up the support for SignalEvent?
However as the loop is declared as private, there is no way of implement that by using either timer or tick queue.
Any thought?
Activity
cboden commentedon Aug 4, 2014
That's interesting and something to be further looked in to. I would think we'd alter the
EventLoopInterfaceand support it in each of the implementations (if they're able to). TheExtEventLoopcould useSignalEventand the other implementations could use the pcntl functions (if pcntl is loaded).steverhoades commentedon Sep 8, 2014
@cboden EvLoop can also use SignalEvent see example #9 http://us1.php.net/manual/en/ev.examples.php
clue commentedon Sep 19, 2015
See also #20 for a first starting point.
jmalloc commentedon Oct 20, 2015
Hey all, I'll probably make an attempt at adding signal support to loop implementations as a matter of necessity. I was considering adding a
SignallingLoopInterfacewith methods to register/unregister signal handlers. This interface would extendLoopInterface, as I'm not sure that all implementations can support signals in a useful way. If it turns out that they can, we could roll the interfaces together. Thoughts @cboden / @clue / anyone?mikegioia commentedon Mar 1, 2016
@jmalloc Did you make any progress with that Interface? I'm in a hard place right now needing signal support for a React loop >_<
cebe commentedon Mar 1, 2016
@mikegioia I am currently using https://github.com/mkraemer/react-pcntl in combination with this patch: #44. Would love to have some feedback in the PR.
mikegioia commentedon Mar 1, 2016
@cebe I just tested out react-pcntl and applied your patch. It works :D :D However, we're still left with that unsuppressed warning from the
stream_selectfailing. While I haven't written your suggested test case, I'm pretty confident the performance penalty (if there even is one) is negligible for StreamSelectLoop users. I just wonder what the preferred method of dealing with this is. I'd hate to have to fork the repo to add my own @.Has there been any movement on incorporating your patch?
cebe commentedon Mar 1, 2016
I'd vote to add the
@onstream_selectbut I'd also like to hear project maintainers opinion on this matter. @cboden @WyriHaximus what needs to be done to get #44 merged? Could you please take a look?jmalloc commentedon Mar 1, 2016
@mikegioia I havent had a chance to address it in any way I'd want to share, just some hacks to get us going at the moment :/
We use an error handler called Asplode, which produces
ErrorExceptionsfrom old-style PHP errors. The solution for us was to catch anErrorExceptionfromstream_selectand callpcntl_dispatchif the error message describes a signal event. There's probably some overhead involved there, but we use signals in pretty standard ways (to trigger config reloads or to kill gracefully) so it hasn't been a problem.I'm still keen to attempt a more general solution that doesn't use a timer, but I think it would require the okay to use
@onstream_selectas @cebe suggests, and simply callpcntl_dispatchif it's available when the return value isfalse. It's a bit more complex, but it looks like Icicle.io does something similar.mikegioia commentedon Mar 1, 2016
Thanks @jmalloc. I ended up doing something similar with a custom error and exception handler. I'd be curious to see what you did with
pcntl_signal_dispatchand why it was needed -- I didn't run into any issues with signal sending once I caught the warning, but I'm unfamiliar with what that function does 😞jmalloc commentedon Mar 1, 2016
The handlers registered with
pcntl_signalaren't actually invoked when the signal is received, they're only invoked whenpcntl_signal_dispatchis called. I think it's somewhat common to register this function as a tick handler, but I haven't tried that.cebe commentedon Mar 2, 2016
yes, you register it as tick handler in normal php or in a timer in the eventloop.
cboden commentedon Mar 8, 2016
Supported now via #44