On Tue, Mar 15, 2016 at 02:30:26AM +0100, Tobias Ulmer wrote:
> On Tue, Mar 15, 2016 at 12:51:45AM +0100, Tobias Ulmer wrote:
> > On Thu, Mar 10, 2016 at 10:23:17PM +0000, Kári Tristan Helgason wrote:
> > > I think I may have come across a race in the implementation of 
> > > `pthread_barrier_wait` in librthread.
> 
> I'm pretty tired but I think I know what you're talking about. Can you
> try this diff?
> 
> We set 'sofar' to 0 first thing when the condition is met, allowing
> threads to destroy a barrier that's still in use.

I think this diff is a bit better because it maintains the current
behaviour and destroys the barrier properly without extra locks.

The idea is to count as threads exit the barrier and permit _destroy
execution only when that counter reaches 0. Otherwise return EBUSY as
before.

The ouptut from your test program varies but always returns 0 at the
end.

Example 1:
%------------------------------------------
$ ./test_destroy                                          
Main thread done spawning threads
Thread 0 started
Thread 1 started
Thread 2 started
Thread 3 started
Thread 2 ended
Main thread done waiting
pthread_barrier_destroy returned 16
pthread_barrier_destroy returned 16
...
pthread_barrier_destroy returned 16
Thread 1 ended
Thread 0 ended
Thread 3 ended
pthread_barrier_destroy returned 0
%------------------------------------------

Example 2:
%------------------------------------------
$ ./test_destroy
Thread 0 started
Thread 1 started
Main thread done spawning threads
Thread 3 started
Thread 2 started
Thread 1 ended
Main thread done waiting
Thread 3 ended
Thread 0 ended
Thread 2 ended
pthread_barrier_destroy returned 0
%------------------------------------------

What do you think?


Index: rthread.h
===================================================================
RCS file: /cvs/src/lib/librthread/rthread.h,v
retrieving revision 1.55
diff -u -p -r1.55 rthread.h
--- rthread.h   27 Jan 2016 08:40:05 -0000      1.55
+++ rthread.h   16 Mar 2016 09:50:58 -0000
@@ -142,6 +142,7 @@ struct pthread_barrier {
        pthread_cond_t cond;
        int threshold;
        int sofar;
+       int out;
        int generation;
 };
 
Index: rthread_barrier.c
===================================================================
RCS file: /cvs/src/lib/librthread/rthread_barrier.c,v
retrieving revision 1.2
diff -u -p -r1.2 rthread_barrier.c
--- rthread_barrier.c   23 Apr 2012 08:30:33 -0000      1.2
+++ rthread_barrier.c   16 Mar 2016 09:50:58 -0000
@@ -76,7 +76,7 @@ pthread_barrier_destroy(pthread_barrier_
 
        b = *barrier;
 
-       if (b->sofar > 0)
+       if (b->out > 0)
                return (EBUSY);
 
        *barrier = NULL;
@@ -105,6 +105,7 @@ pthread_barrier_wait(pthread_barrier_t *
 
        _rthread_debug(6, "sofar: %d, threshold: %d\n", b->sofar, b->threshold);
        if (++b->sofar == b->threshold) {
+               b->out = b->sofar - 1;  /* mark thread exit */
                b->sofar = 0;
                b->generation++;
                if ((rc = pthread_cond_broadcast(&b->cond)))
@@ -118,6 +119,7 @@ pthread_barrier_wait(pthread_barrier_t *
                        if ((rc = pthread_cond_wait(&b->cond, &b->mutex)))
                                goto err;
                } while (gen == b->generation);
+               b->out--;       /* mark thread exit */
        }
 
 err:

Reply via email to