> I will try your patch and post back here. I discussed the patch with Tobias and it is not enough. The barrier pointer could become NULL during _destroy.
I'm currently working on a better diff which I'll post once I'm confident it's good enough. > > > On 15 Mar 2016, at 01:30, Tobias Ulmer <[email protected]> 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. > > > > Index: rthread_barrier.c > > =================================================================== > > RCS file: /home/vcs/cvs/openbsd/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 15 Mar 2016 01:10:25 -0000 > > @@ -70,16 +70,23 @@ int > > pthread_barrier_destroy(pthread_barrier_t *barrier) > > { > > pthread_barrier_t b; > > + int rc; > > > > if (barrier == NULL || *barrier == NULL) > > return (EINVAL); > > > > b = *barrier; > > > > - if (b->sofar > 0) > > + if ((rc = pthread_mutex_trylock(&b->mutex))) > > + return rc; > > + > > + if (b->sofar > 0) { > > + pthread_mutex_unlock(&b->mutex); > > return (EBUSY); > > + } > > > > *barrier = NULL; > > + pthread_mutex_unlock(&b->mutex); > > pthread_mutex_destroy(&b->mutex); > > pthread_cond_destroy(&b->cond); > > free(b); > > > > > > --8<-- > > /* modified test program */ > > #include <stdio.h> > > #include <pthread.h> > > #include <stdlib.h> > > #include <unistd.h> > > > > pthread_barrier_t barrier; > > > > void *threadfn(void *arg) { > > int num = (int)arg; > > fprintf(stderr, "Thread %d started\n", num); > > srandom(num); > > sleep(random() % 5 + 1); > > pthread_barrier_wait(&barrier); > > fprintf(stderr, "Thread %d ended\n", num); > > } > > > > > > int main(int argc, char *argv[]) { > > int numThreads = 4; > > int i, rc; > > pthread_barrier_init(&barrier, NULL, numThreads+1); > > pthread_t *thread = malloc(sizeof(pthread_t) * numThreads); > > for(i = 0; i < numThreads; i++) { > > pthread_create(&thread[i], NULL, threadfn, (void*)i); > > } > > fprintf(stderr, "Main thread done spawning threads\n"); > > pthread_barrier_wait(&barrier); > > fprintf(stderr, "Main thread done waiting\n"); > > > > do { > > rc = pthread_barrier_destroy(&barrier); > > fprintf(stderr, "pthread_barrier_destroy returned %d\n", rc); > > } while (rc != 0); > > return 0; > > } > > --8<-- >
