[Twisted-Python] spawnProcess - reapProcess not retrying on failures

[Adi Roiban]

On 3 September 2014 18:55,  <exarkun at twistedmatrix.com> wrote:
> On 03:27 pm, adi at roiban.ro wrote:
>>
>> On 3 September 2014 14:39,  <exarkun at 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