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

[Eckhart Wörner]

Hi Mike,

[I'm subscribed to the ML]

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().

> 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.

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. From an 
outside perspective, a signal has to reliably trigger an action, not only when 
it is sent twice.

> there is no race issue between
> different signals (e.g. `killall -HUP gpsd` && `killall -INT gpsd`) since
> each signal has its own state.

Oh sure, all of the signal handlers write their code to "signalled", which 
contains the code of the latest signal only. Processing signals therefore only 
works correctly if we block and only let the signals in one by one, which is 
exactly what is achieved by the pselect().

> 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.

Eckhart