Re: Patch to add g_child_watch_source

["Alexis S. L. Carvalho" <alexis cecm usp br>]

Thus spake Jonathan Blandford:
> "Alexis S. L. Carvalho" <alexis cecm usp br> writes:
> > I don't think I understand what you're trying to protect with
> > child_watch_count...
> 
> If child_watch_count gets incremented in between the waitpid and setting
> child_watch_source->child, I could miss the child exiting.  So I make a
> copy of child_watch_count to use in check_for_child_exited to keep it
> consistent throughout.  I can make the comment more obvious.

Actually I hadn't understood why you were using child_watch_count in the
first place.  Now I see that you're trying to avoid calling waitpid when
there was no SIGCHLD.

Here are two other races:

	parent runs child
	child exits; SIGCHLD is delivered to the parent
	parent calls g_child_watch_source_new

You'll end up calling waitpid for this child only when another SIGCHLD
arrives.  One way to handle this race is to call waitpid in
g_child_watch_source_new.

The other race is the same one, but for the first child:  the signal
handler is still not installed.  Usually, this isn't a problem - the
SIGCHLD will be ignored and you'll be able to call waitpid.  But if the
application had a signal handler installed or had SIGCHLD blocked, you
may have some problems.  You may argue that an application that has a
handler for SIGCHLD and uses GChildWatch sources is broken, but since
signal masks are inherited through fork/exec I think you should unblock
SIGCHLD (with sigprocmask).

The easiest way to deal with this is probably an initialization function
that has to be called before forking the first child.

Also, you may want to save errno before the call to write in the signal
handler and restore it later, just in case.

Finally, it would be nice if the documentation mentioned that since
you're dealing with signal handlers, some system calls may fail, setting
errno to EINTR.

Alexis