Eric Blake wrote: > On 01/13/2012 12:15 PM, Luiz Capitulino wrote: > > This might look complex, but the final code is quite simple. The > > purpose of that approach is to allow qemu-ga to reap its children > > (semi-)automatically from its SIGCHLD handler. > > Yes, given your desire for the top-level qemu-ga signal handler to be > simple, I can see why you did a double fork, so that the intermediate > child can change the SIGCHLD behavior and actually do a blocking wait in > the case where status should not be ignored.
An alternative is for SIGCHLD to write a byte to a non-blocking pipe and do nothing else. A main loop outside signal context reads from the pipe, and on each read triggers a subloop of non-blocking waitpid() getting child statuses until there are no more. Because it's outside signal context, it's safe to do anything with the child statuses. (A long time ago, on other unixes, this wasn't possible because SIGCHLD would be retriggered until wait(), but it's not relevant on anything modern.) > > + execlp(pmutils_bin, pmutils_bin, arg, NULL); > > Do we really want to be relying on a PATH lookup, or should we be using > an absolute path in pmutils_bin? Since you mention async-signal-safety, execlp() isn't async-signal-safe! Last time I checked, in Glibc execlp() could call malloc(). Also reading PATH looks at the environment, which isn't always thread-safe either, depending on what else is going on. I'm not sure if it's relevant to the this code, but on Glibc fork() is not async-signal-safe and has been known to crash in signal handlers. This is why fork() was removed from SUS async-signal-safe functions. > I didn't check whether slog() is async-signal safe (probably not, since > even snprintf() is not async-signal safe, and you are passing a printf > style format string). But strerror() is not, so you shouldn't be using > it in the child if qemu-ga is multithreaded. In general, why is multithreadedness relevant to async-signal-safety here? Thanks, -- Jamie
