Re: GChildWatchSource take three
- From: Owen Taylor <otaylor redhat com>
- To: Jonathan Blandford <jrb redhat com>
- Cc: gtk-devel-list gnome org
- Subject: Re: GChildWatchSource take three
- Date: Tue, 16 Sep 2003 16:10:18 -0400
On Tue, 2003-09-16 at 01:06, Jonathan Blandford wrote:
> Owen Taylor <otaylor redhat com> writes:
>
> > On Wed, 2003-07-30 at 14:16, Jonathan Blandford wrote:
> > > Hi Owen,
> > >
> > > Here's the latest version of my patch. I added more to the docs as well
> > > as fixing other things discussed with Alex. The big question I have is
> > > how best to handle ECHILD situations. Currently, I'm passing -1 as the
> > > status. As an alternative, I can just not dispatch those sources as
> > > well.
> >
> > Commented on below with a few other things; despite the length of
> > the comments, it looks basically pretty solid; most of this stuff
> > is details, and the stuff that isn't details shouldn't be hard
> > to fix.
>
> I made most of the changes you suggested. There were two I had problems
> with. Also, I'm assuming this won't work with win32. Should I go ahead
> and add #ifdef G_OS_UNIX guards to gmain.[ch]?
>
> Some comments on the comments.
> > ====
> > +void
> > +_g_main_thread_init ()
> > +{
> > + add_pipes_to_main_contexts ();
> > +}
> > ====
> >
> > Don't you need to do the SINGLE => THREADED initialization
> > here? If you don't do it here, then a child watched
> > added before g_thread_init() won't fire correctly
> > after it.
>
> I don't do SINGLE => THREADED initialization here as we may not want to
> use the child_watch stuff.
If you are in SINGLE, you are using the child_watch stuff, no?
> > ECHILD indicates a programmatic error, and you should g_warning()
> > and probably avoid dispatching. Anyways, I believe a status of -1
> > could be a legitimate exit status, so you can't use it as a flag value.
>
> I wonder if there are ever legitimate reasons to want to get ECHILD?
If you don't know that there is a child (possibly zombie) with a given
PID that you created, then you have no business passing that ID
to waitpid.
> > This doens't work on several aspects:
> >
> > A) g_main_context_wakeup_unlocked() is a noop
> > ifndef G_THREADS_ENABLED
> >
> > B) g_main_context_wakeup_unlocked() checks
> > if (g_thread_supported() && context->poll_waiting)
> > so it's a no-op when you call it here in the
> > single threaded case.
> >
> > C) g_main_context_wakeup_unlocked() isn't signal safe. In the
> > sequence
> >
> > ===
> > ifdef G_THREADS_ENABLED
> > if (!context->poll_waiting) <= A
> > {
> > #ifndef G_OS_WIN32
> > gchar c;
> > read (context->wake_up_pipe[0], &c, 1);
> > #endif
> > }
> > else
> > context->poll_waiting = FALSE; <= B
> > ===
> >
> > if your signal handler runs between A and B, you
> > leak a byte into the pipe, and your main loop will
> > spin at 100% cpu because it gets left there. I think you
> > can fix this by doing:
> >
> > if (context->wake_up_rec.revents & G_IO_IN)
> > {
> > /* Read a bunch of bytes from the pipe */
> > }
> > context->poll_waiting = FALSE;
> >
> > And then use a custom wake up function in your
> > signal handler that doesn't look at g_thread_supported().
>
> I added the custom wake up function, but I'm not sure what you were
> proposing for g_main_context_check. I tried replacing the
> (!context->poll_waiting) test but that seemed to break timeouts.
I don't think it should. I'm not really sure how to explain
further ... context->wake_up_rec.revents should have G_IO_IN if there
is stuff waiting to read out of the wake-up pipe.
Regards,
Owen
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]