The test suite for lang/sbcl has uncovered a race between
pthread_join() and the exiting thread: it may return before it's safe
to unmap a custom thread stack.

In the exiting thread:
  1) pthread_exit() calls _thread_release() via the tc_thread_release
      pointer.
  2) _thread_release() eventually calls _sem_post(&thread->donesem),
     and then returns.
  3) pthread_exit() then calls __threxit(&tib->tib_tid).

In the joining thread, _sem_wait(&thread->donesem, ...) then returns,
possibly before the exiting thread has reached __threxit().

I don't really know what the best fix would be, but waiting for
__threxit() to zero out tib_tid seems to work for me:

diff --git a/lib/librthread/rthread.c b/lib/librthread/rthread.c
index 8825e7844af..1fd1b5ee217 100644
--- a/lib/librthread/rthread.c
+++ b/lib/librthread/rthread.c
@@ -306,6 +306,9 @@ pthread_join(pthread_t thread, void **retval)
                if (retval)
                        *retval = thread->retval;
 
+               while (thread->tib->tib_tid)
+                       pthread_yield();
+
                /*
                 * We should be the last having a ref to this thread,
                 * but someone stupid or evil might haved detached it;


Following is a test program which exposes the race. You might need to
run it in a loop to see a segfault, especially on a uniprocessor
machine.


#include <sys/mman.h>
#include <pthread.h>
#include <err.h>

#define NTHREADS 100

pthread_mutex_t exitmtx = PTHREAD_MUTEX_INITIALIZER;

struct thrstk {
        pthread_t pth;
        void *stack;
} threads[NTHREADS];

void *
thrmain(void *arg)
{
        struct thrstk *thrstk = arg;

        if (thrstk == NULL)
                pthread_mutex_lock(&exitmtx);
        else {
                pthread_join(thrstk->pth, NULL);
                munmap(thrstk->stack, PTHREAD_STACK_MIN);
        }

        return NULL;
}

int
main()
{
        pthread_attr_t attr;
        struct thrstk *prev;
        int i;

        pthread_mutex_lock(&exitmtx);
        prev = NULL;
        for (i = 0; i < NTHREADS; i++) {
                threads[i].stack = mmap(NULL, PTHREAD_STACK_MIN,
                    PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANON|MAP_STACK, -1, 
0);
                if (threads[i].stack == NULL)
                        err(1, "mmap");
                pthread_attr_init(&attr);
                pthread_attr_setstack(&attr, threads[i].stack, 
PTHREAD_STACK_MIN);
                if (pthread_create(&threads[i].pth, &attr, thrmain, prev) != 0)
                        err(1, "pthread_create");
                pthread_attr_destroy(&attr);
                prev = &threads[i];
        }

        pthread_mutex_unlock(&exitmtx);
        thrmain(prev);

        return 0;
}

Reply via email to