On 3 September 2014 18:55, <exar...@twistedmatrix.com> wrote:
> On 03:27 pm, a...@roiban.ro wrote:
>>
>> On 3 September 2014 14:39, <exar...@twistedmatrix.com> wrote:
[snip]
>> Do you have any suggestion for how the calls should be made?
>>
>> reactor.run(installSignalHandlers=True, installStopHandlers=False)
>
>
> Perhaps.
[snip]
> It might be nice to try to be somewhat flexible - in case there's some
> reason to change what signals the reactor wants to handle in the future.
> Perhaps:
>
> reactor.run(installSignalHandlers={SIGCHLD})
>
> An entirely different direction could be to make this bit of configuration
> into initialization for the reactor.
>
> from twisted.internet.epollreactor import install
> install(installSignalHandlers={SIGCHLD})
>
> from twisted.internet import reactor
> ...
> reactor.run()
>
> By keeping these details away from `IReactorCore.run`, that method remains
> maximally useful. For example, if you could set up the reactor this way, a
> normal `twistd` plugin would still be able to benefit from your choice, even
> with twistd's naive call of `reactor.run()` with no extra arguments.
>
> Application code calling these `install` functions is already supported
> (it's how you select a specific reactor, after all). Some of the install
> functions even accept arguments already.
>
> This would actually eliminate another existing issue - `IReactorCore.run` is
> actually defined to take no arguments. The implementations ignore this
> because someone thought it was important to be able to disable installation
> of signal handlers.
I am happy to have a simple reactor.run() and move
installSignalHandlers somewhere else.
working with install(installSignalHandlers={SIGCHLD}) seems a bit complicated,
as I assume that many developers rely on the automatic reactor installation.
In the same time, I assume that 'installSignalHandlers' argument would
be supported by all reactors this is why maybe we can
have something like:
from twisted.internet import reactor
def customHandler(signum, frame):
pass
reactor.installSignalHandlers(
SIGCHLD=True, # Install default handler
SIGTERM=None, # Don't install handler
SIGINT=customHandler, # Install custom handler
# SIGBREAK is not request so that default handler is installed.
)
# reactor.installSignalHandlers() installs all default handlers.
reactor.run()
----
reactor.run(InstallSignalHandlers=True|False) would be deprecated.
In case reactor.installSignalHandlers is not called before run(), all
default handlers will be installed.
[snip]
> The sidecar process is an example of a general fix, though. The idea there
> is that Twisted itself runs a private child process (perhaps only when the
> first call to spawnProcess is made). It talks to that process using a file
> descriptor. That process can install a SIGCHLD handler (because Twisted
> owns it, application developers don't get to say they don't want one
> installed) or use another more invasive strategy for child process
> management. When you want to spawn a process, the main process tells the
> sidecar to do it. The sidecar relays traffic between the child and the
> original parent (or does something involving passing file descriptors across
> processes).
>
> This removes the need to ever install a SIGCHLD handler in the main process.
> It also probably enables some optimizations (reapProcesses is O(N!) on the
> number of child processes right now) that are very tricky or impossible
> otherwise.
>
> Jean-Paul
Thanks for the details regarding the side-process dedicated to child
process management.
Not sure if we need a separate ticket for that, or add it as a comment
to https://twistedmatrix.com/trac/ticket/5710
Thanks!
--
Adi Roiban
_______________________________________________
Twisted-Python mailing list
Twisted-Python@twistedmatrix.com
http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
