Quoting Samuel Thibault (2013-11-16 11:16:02)
> > - pthread_spin_lock (&lock);
> > - if (nreqthreads == 1)
> > + __atomic_sub_fetch (&totalthreads, 1, __ATOMIC_RELAXED);
> > + if (__atomic_sub_fetch (&nreqthreads, 1, __ATOMIC_RELAXED) == 0)
> > {
> > /* No other thread is listening for requests, continue. */
> > - pthread_spin_unlock (&lock);
> > + __atomic_add_fetch (&totalthreads, 1, __ATOMIC_RELAXED);
> > + __atomic_add_fetch (&nreqthreads, 1, __ATOMIC_RELAXED);
> > goto startover;
> > }
> > - nreqthreads--;
> > - totalthreads--;
> > - pthread_spin_unlock (&lock);
> > }
>
> Here the totalthreads update should be done after decrementing and
> testing nreqthreads (and thus no reincrement if that gives 0). Otherwise
> master might see totalthreads being 1 although there is actually another
> thread which will happen to realize it'll actually continue. Put
> another way, a thread mustn't decrement totalthreads unless it is
> absolutely sure it will terminate.
Dooh, yes, of course >,<
> With that fixed, Ack.
I just realized that I tested these changes *with* the
libports_stability.patch, so the threads do not actually time out and
so this particular code is never reached. Maybe we should defer this
patch until Richards patch destruction rework is finished and we got
rid of that patch?
Justus
