[Valgrind-developers] detection of inter-thread dependencies through condition variables

[ERSEK L. <la...@ca...>]

Hi,

[0] states:


----------
     7.5. Hints and Tips for Effective Use of Helgrind

       3. Avoid POSIX condition variables

       [...]

       a solution to this problem that does not require source-level
       annotation of condition-variable wait loops is beyond the current
       state of the art.
----------


I respectfully ask if the following passage could be recommended for the 
Waiter as a code transformation that pessimizes, but does not invalidate 
the code. If this qualifies as "source-level annotation of 
condition-variable wait loops", then I apologize for the noise. (I 
interpret "source-level annotation" as specially formatted comments, or 
#pragma's, or Valgrind-specific API calls etc.)


----------
     If your original code looks (as it should look) like

         pthread_mutex_lock(mx);
         while (!b) {
           pthread_cond_wait(cv, mx);
         }
         pthread_mutex_unlock(mx);

     Then consider inserting the following COMPILING_FOR_VALGRIND block:

         pthread_mutex_lock(mx);

         #ifdef COMPILING_FOR_VALGRIND
         {
           struct timespec epoch;
           int ret;

           epoch.tv_sec = 0;
           epoch.tv_nsec = 0;

           ret = pthread_cond_timedwait(cv, mx, &epoch);
           assert(ETIMEDOUT == ret || 0 == ret);
         }
         #endif

         while (!b) {
           pthread_cond_wait(cv, mx);
         }
         pthread_mutex_unlock(mx);
----------

1. The dependency on the condition variable is now unavoidable, no matter 
the initial value of "b" right after acquiring "mx"; so Valgrind can see 
the dependency unconditionally.

2. No matter the value of "ret" (0 or ETIMEDOUT), pthread_cond_timedwait() 
will have released and re-acquired mutex "mx"; see [1]. This is new 
mutex/condvar activity, but it shouldn't hurt too much performance-wise.

3. The clock selection (see [2]) of the condition variable shouldn't 
matter with an absolute time barrier set to 0. The epoch of whichever 
clock is selected happens in the past, thus pthread_cond_timedwait() 
should return immediately.

4. pthread_cond_timedwait() introduces a cancellation point. This is 
nothing new if (!b) holds, because pthread_cond_wait() is a cancellation 
point anyway. If "b" is true, however, this cancellation point is new. 
Shouldn't hurt too much, since the original waiter code can't predict the 
value of "b" either, so it must be prepared for cancellation anyway.

5. If "ret" is ETIMEDOUT, then we've received no signal/broadcast for 
sure; back to business as usual. If "ret" is 0, we have received a 
signal/broadcast (or a spurious wakeup), but we'll still check "b" first, 
so the code stays valid.


As said above, I apologize if this is obvious and already covered by the 
exclusion of "source-level annotation".

Thanks,
lacos

[0] http://valgrind.org/docs/manual/hg-manual.html#hg-manual.effective-use
[1] http://www.opengroup.org/onlinepubs/000095399/functions/pthread_cond_timedwait.html
[2] http://valgrind.org/docs/manual/drd-manual.html#drd-manual.pctw