On Tue, May 27, 2014 at 12:52 PM, Ben Pfaff <b...@nicira.com> wrote: > On Tue, May 27, 2014 at 12:32:50PM -0700, Ansis Atteka wrote: >> Between fork() and execvp() calls in the process_start() >> function both child and parent processes share the same >> file descriptors. This means that, if a child process >> received a signal during this time interval, then it could >> potentially write data to a shared file descriptor. >> >> One such example is fatal signal handler, where, if >> child process received SIGTERM signal, then it would >> write data into pipe. Then a read event would occur >> on the other end of the pipe where parent process is >> listening and this would make parent process to incorrectly >> believe that it was the one who received SIGTERM. >> Also, since parent process never reads data from this >> pipe, then this bug would make parent process to consume >> 100% CPU by immediately waking up from the event loop. >> >> This patch will help to avoid this problem by blocking >> signals until child closes all its file descriptors. >> >> Signed-off-by: Ansis Atteka <aatt...@nicira.com> >> Reported-by: Suganya Ramachandran <sugan...@vmware.com> >> Issue: 1255110 > > I agree that this fixes a problem. Thank you for tracking it down and > coming up with a good fix. > > I am nervous about blocking all signals indiscriminately. Sometimes > libraries use signals (e.g. pthreads implementations sometimes do) and > I don't know whether it's a good idea to block them. PSPP used to > have functions fatal_signal_block() and fatal_signal_unblock() to > block the fatal signal handlers, before commit d8b30702057c > (fatal-signal: Run signal hooks outside of actual signal handlers.). > We could reintroduce these, or just add a function to inquire of the > signals that fatal-signal handles (in the form of a signal mask).
I sent PATCHv2 that addresses your concern. The new patch blocks only fatal signals. Just curious. Do you think it would make sense to use ppoll() or signal_fd() instead of "self pipe trick" to handle signals? Not sure about portability, but it seems that ppoll() at least is POSIX compliant and was introduced in Linux 2.6.16. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
