Re: [gpsd-dev] [PATCH] Use pselect() to reduce wakeups and avoid race conditions

[Mike Frysinger]

On Friday 13 January 2012 17:55:37 Eckhart Wörner wrote:
> Am Freitag, 13. Januar 2012, 17:24:20 schrieb Mike Frysinger:
> > the idle issue doesn't require pselect() to address.  you can do that
> > already by replacing "tv" with NULL in the select() call.  but the
> > current main logic seems to imply that this behavior is somewhat
> > desirable (at least when debugging).  perhaps the tv argument should be
> > modified that way ?  if people are running in debug mode, then poll
> > every second, otherwise wait in definitely.  note: i'm certainly not a
> > gpsd expert, so my quick analysis might be off ...
> 
> I don't see how tv is used in debugging at all. See also the comment before
> the select().

true ... i misread the select() loop incorrectly.  in this case, tv is used to 
mitigate the race condition you highlight below ...

> > as for the race condition, a little explanation might be in order for
> > people not versed on esoteric signal behavior.  signal() has the
> > property that when the user specified handler is called, the handler is
> > automatically reset to SIG_DFL.  thus there is a race condition from the
> > time someone does `killall - HUP gpsd` to the time where gpsd's main
> > loop notices and actually processes the request.  this window cannot be
> > fully closed even if the user signal handler itself calls signal() to
> > setup the handler for future signals.  if, in that window of time where
> > the handler has been reset to SIG_DFL, the user resends with `killall
> > -HUP gpsd`, then the kernel will go ahead and terminate gpsd.
> 
> As you said, the problem is that signal handling using select() is racy. If
> gpsd is signalled directly before entering the select(), we're stuck in it
> until something else happens, at which point the signal is indeed
> processed.

true ... that race your pselect() change fixes -- if the signal comes in after 
the "while (0 == signalled)" check and before the select() syscall, the 
response to the signal is delayed by 1 second (the timeout specified by tv).

> You were saying that a second HUP will fix the first HUP if it indeed
> occurs directly before entering select(). This is true, but not the point.

it's worse than that.  you cannot send any signal a 2nd time once the first has 
been delivered.  the relationship to where the checking occurs doesn't matter.  
at any point before the app exits, sending the same signal a 2nd time will 
cause the kernel to immediately terminate it.

your original changelog is too vague to figure out which race and which signals 
you're referring to.  i'd suggest reposting with clarification and real details 
as to what it is fixing.

> From an outside perspective, a signal has to reliably trigger an action,
> not only when it is sent twice.

of course

> > the 3rd arg to sigprocmask() is not an emptyset, it's the old sigset, so
> > the variable name needs tweaking.
> 
> The old sigset is the emptyset.

there is no guarantee this is true.  signal masks are inherited across forks 
and execs.
-mike

signature.asc
Description: This is a digitally signed message part.