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

[exarkun at twistedmatrix.com]

On 03:27 pm, adi at roiban.ro wrote:
>On 3 September 2014 14:39,  <exarkun at twistedmatrix.com> wrote:
>>
>>Yes.  Providing more fine-grain control over signal handlers would be 
>>a fine
>>improvement.
>
>Do you have any suggestion for how the calls should be made?
>
>reactor.run(installSignalHandlers=True,  installStopHandlers=False)

Perhaps.
>or
>
>reactor.installStopHandlers = False
>reactor.run()

Probably not this one.  Setting attributes on random things is kind of 
sad. :)

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.
>
>>Another fine improvement would be making child processes work even if 
>>a
>>SIGCHLD handler cannot be installed (for example, by polling children
>>periodically, by using wait() in a sidecar process, or by using a 
>>better
>>system-specific child process monitoring API (eg kqueue's 
>>EVFILT_PROC)).
>
>I see that GlibReactorBase inherits from PosixReactorBase so it should
>install the SIGCHLD handler... this should not be the reason why gtk2
>reactor don't work.

Gtk messes with signals too.  There are confusing order-of-execution 
dependencies and Gtk changes subtly from release to release, re-breaking 
things after we fix them or changing them to be broken in a different 
way.

So that's *why* it probably doesn't work with Gtk2 reactor - if not 
*how*. ;)

I think I missed the explanation of what in particular wasn't working 
with Gtk2 reactor though.
>As a poor man's fix and Unix independent fix maybe we can call
>reapAllProcess in a task.LoopingCall...
>What are the reasons why SIGCHLD handler cannot be installed?

Either because you want to run the reactor in a non-main thread (where 
Python won't let you install signal handlers for the good reason that 
mixing signals and threads has crazy behavior) or because you want to 
use a different library that depends on having its own SIGCHLD handler 
and you're not interested in Twisted's process support.
>I am asking since I hope it could help me understant where and how to
>enable the poor man's fix... and how to fill the bug report.
>
>kqueue's EVFILT_PROC would be great, but I guess that we still need a
>general fix

Perhaps, perhaps not.  A general fix might be less code but having half 
a dozen specialized fixes, one for each reactor, would also still fix 
the problem.  The different reactor implementations are essentially the 
big piles of specialized fixes needed to do non-blocking I/O on 
different platforms.  This would just be a little more of the same.

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
>---------
>
>For the record: Right now, to ignore SIGINT, SIGTERM, SIGBREAK handles
>but keep SIGCHLD I do:
>
># Patch base reactor to not install SIGINT, SIGTERM and SIGBREAK 
>handlers
>_SignalReactorMixin._handleSignals = lambda self: None
>reactor.run()
>
>
>--
>Adi Roiban