Re: Checked in Matt Welsh's patch

[Matt Welsh <mdw@cs.berkeley.edu>]

Tom,

Actually, your change to check "if (r && errno == EINTR)" is not correct.
You actually must check that "r == EINTR" as I had it originally. This is
because pthread_cond_timedwait *returns* EINTR if it was interrupted by
a signal. In other cases it may return other values (whether or not errno
is set to EINTR). 

The man pages for pthread_cond_timedwait say this as well:

RETURN VALUE
       All condition variable functions return 0 on success and a
       non-zero error code on error.


ERRORS
       pthread_cond_init,                    pthread_cond_signal,
       pthread_cond_broadcast, and pthread_cond_wait never return
       an error code.

       The pthread_cond_timedwait function returns the  following
       error codes on error:

              ETIMEDOUT
                     the  condition  variable  was  not  signaled
                     until the timeout specified by abstime


              EINTR  pthread_cond_timedwait was interrupted by  a
                     signal

       The  pthread_cond_destroy  function  returns the following
       error code on error:

              EBUSY  some threads are currently waiting on  cond.


My tests show that most of the time, pthread_cond_timedwait is returning
110 (which is ETIMEDOUT) and other times is returning EINTR (which is 4).
In some cases errno is 0 and in other cases it is EINTR. errno is never
ETIMEDOUT. This also means that the last check (that errno == ETIMEDOUT)
is a bug; do the test against r instead.

This is probably because pthread_cond_timedwait is implemented using 
several system calls which may independently set errno to EINTR. 
pthread_cond_timedwait probably does some checking and returns either
EINTR or ETIMEDOUT based on what actually happened (i.e. a timeout signal
versus some other signal). 

I do not have a copy of the POSIX thread spec with me, so it's possible
that this is non-standard behavior under Linux, in which case the test 
should be wrapped around an #ifdef LINUX_THREADS or something. Can 
you check into that?

Can you please make this change, and put a comment to this effect 
before the test? This is a strange enough race condition that if we
have the wrong code there things will subtly start to break.

Thanks much!
Matt Welsh