reapAllProcesses occassionally, as apparently signal delivery is unreliable

[jknight]

Has this actually been verified to be true?

[glyph]

It doesn't matter; see comments on #1997. We need to do this anyway.

[jknight]

Implemented fix in source:branches/sigchld-791/

Technique: a C module which installs a signal handler for SIGCHLD, with SA_RESTART, and a C function to write to a given fd. The reactor listens on that fd, and calls reapAllProcesses when there's activity.

However, if the C module is not present, it instead starts a LoopingCall? running whenever there are processes waiting to be reaped, and doesn't install a SIGCHLD handler at all.

[jknight]

Also related to and should fix #1430 #1997

[jknight]

And also #733

[therve]

It seems it would need review ?

[exarkun]

It doesn't have sufficient unit tests.

[itamarst]

The LoopingCall? should also be used if someone does reactor.run(installSignalHandlers=False), if it's not already. And probably should stop running if there's no Process instances and restart if necessary (so trial is happy), again, if it's not already got that behavior in the branch.

[exarkun]

In addition to #733 and #1997, see #2535.

[therve]

(In [23160]) Branching to 'sigchld-791-2'

[therve]

I had the opportunity to look at this a bit more. I think the looping call solution is wrong. Do we know exactly what platform doesn't have SA_RESTART? I think I'd prefer the current behavior with SIGCHLD than a polling system that browses every processes launched 10 times per second. This conflicts pretty badly with #2967.

[glyph]

(In [23726]) Branching to 'sigchld-791-3'

[exarkun]

SIGCHLD delivery is not unreliable. What is meant by "unreliable" when people talk about signal delivery is that multiple instances of a particular signal may be coalesced into one call to a signal handler in a process. So polling isn't necessary to deal with "unreliable signal delivery".

However, some of the code in this branch is still probably useful for resolving some Twisted process issues. For example, sigaction SA_RESTART prevents us from breaking os.popen. So some of this code should be used to resolve some tickets.

I'm closing this ticket because the problem it describes does not exist and the solution it suggests is therefore unnecessary. Since there is useful code in the branch, I will re-associate it with the ticket it's most directed at fixing, #733 (the branch conflicts with trunk, so it needed to be merged forward anyway).