Re: Is this pthread_cond_wait() race possible

[Sebastian Andrzej Siewior <bigeasy at linutronix dot de>]

On 2017-06-07 21:41:38 [+0200], Torvald Riegel wrote:
> On Wed, 2017-06-07 at 18:48 +0200, Sebastian Andrzej Siewior wrote:
> > Hi Torvald,
> > 
> > I've been staring at the new pthread_cond_* code. Could you please tell
> > if this race possible (two wait threads looking at the same __g_signals
> > value while a third thread signals a wake before going to futex_wait()):
> > 
> > T1                               T2                       T3
> > 
> > __pthread_cond_wait_common()                            
> >  __condvar_fetch_add_wseq_acquire (cond, 2);
> >  __pthread_mutex_unlock_usercnt();                       
> > 
> >                                 __pthread_cond_wait_common()
> >                                  __condvar_fetch_add_wseq_acquire (cond, 2);
> >                                  __pthread_mutex_unlock_usercnt();
> > 
> >                                                         __pthread_cond_signal()
> >                                                           if ((cond->__data.__g_size[g1] != 0) …
> > 
> >                                                            /* Add a signal. */
> >                                                            atomic_fetch_add_relaxed (cond->__data.__g_signals + g1, 2);
> > 
> >  signals = atomic_load_acquire (cond->__data.__g_signals + g);
> >                                  signals = atomic_load_acquire (cond->__data.__g_signals + g);
> > 
> >   signals = 2                     signals = 2
> > 
> >           /* If there is an available signal, don't block.  */
> >   if (signals != 0)               if (signals != 0)
> >       break;                         break;
> > 
> >   while (!atomic_compare_exchange_weak_acquire (cond->__data.__g_signals + g,
> >                                                  &signals, signals - 2));
> >   signals = 0
> >                                   while (!atomic_compare_exchange_weak_acquire (cond->__data.__g_signals + g,
> >                                                 &signals, signals - 2));
> >                                    signals = (int) -2
> 
> The second CAS would expect a value of 2 in *(cond->__data.__g_signals +
> g).  Because the first CAS succeeded in your example, the second CAS
> will fail, and T2 will update it's view of __g_signals and store that in
> signals.

So T1 stores 2 -> 0 in __g_signals + g as expected.
T2 tries to store 2 -> 0 in __g_signals + g which fails because
__g_signals + g != signals (0 != 2). However
atomic_compare_exchange_weak_acquire() will update signals to 0 right?
So the second iteration of the while loop will update __g_signals + g to
0 -> -2. T2 won't loop until __g_signals becomes 2 again, right?

Sebastian