Fix a race condition in pthread_join(). All of the following must occur

atomically:

1) Search _thread_list for the thread to join.
2) Search _dead_list for the thread to join.
3) Set the running thread as the joiner.

While we're at it, fix a race in the case where multiple threads try to
join on the same thread.  POSIX says that the behavior of multiple joiners
is undefined, but the fix is cheap as a result of the other fix.
This commit is contained in:
Jason Evans 2001-06-27 11:41:15 +00:00
parent 5f36700a32
commit 651974ee92
9 changed files with 159 additions and 144 deletions

View File

@ -1207,7 +1207,6 @@ char *__ttyname_r_basic(int, char *, size_t);
char *ttyname_r(int, char *, size_t);
void _cond_wait_backout(pthread_t);
void _fd_lock_backout(pthread_t);
int _find_dead_thread(pthread_t);
int _find_thread(pthread_t);
struct pthread *_get_curthread(void);
void _set_curthread(struct pthread *);

View File

@ -64,35 +64,3 @@ _find_thread(pthread_t pthread)
/* Return zero if the thread exists: */
return ((pthread1 != NULL) ? 0:ESRCH);
}
/* Find a thread in the linked list of dead threads: */
int
_find_dead_thread(pthread_t pthread)
{
pthread_t pthread1;
/* Check if the caller has specified an invalid thread: */
if (pthread == NULL || pthread->magic != PTHREAD_MAGIC)
/* Invalid thread: */
return(EINVAL);
/*
* Lock the garbage collector mutex to ensure that the garbage
* collector is not using the dead thread list.
*/
if (pthread_mutex_lock(&_gc_mutex) != 0)
PANIC("Cannot lock gc mutex");
/* Search for the specified thread: */
TAILQ_FOREACH(pthread1, &_dead_list, dle) {
if (pthread1 == pthread)
break;
}
/* Unlock the garbage collector mutex: */
if (pthread_mutex_unlock(&_gc_mutex) != 0)
PANIC("Cannot lock gc mutex");
/* Return zero if the thread exists: */
return ((pthread1 != NULL) ? 0:ESRCH);
}

View File

@ -42,6 +42,7 @@ _pthread_join(pthread_t pthread, void **thread_return)
{
struct pthread *curthread = _get_curthread();
int ret = 0;
pthread_t thread;
_thread_enter_cancellation_point();
@ -60,24 +61,63 @@ _pthread_join(pthread_t pthread, void **thread_return)
}
/*
* Find the thread in the list of active threads or in the
* list of dead threads:
* Lock the garbage collector mutex to ensure that the garbage
* collector is not using the dead thread list.
*/
if ((_find_thread(pthread) != 0) && (_find_dead_thread(pthread) != 0))
if (pthread_mutex_lock(&_gc_mutex) != 0)
PANIC("Cannot lock gc mutex");
/*
* Defer signals to protect the thread list from access
* by the signal handler:
*/
_thread_kern_sig_defer();
/*
* Unlock the garbage collector mutex, now that the garbage collector
* can't be run:
*/
if (pthread_mutex_unlock(&_gc_mutex) != 0)
PANIC("Cannot lock gc mutex");
/*
* Search for the specified thread in the list of active threads. This
* is done manually here rather than calling _find_thread() because
* the searches in _thread_list and _dead_list (as well as setting up
* join/detach state) have to be done atomically.
*/
TAILQ_FOREACH(thread, &_thread_list, tle) {
if (thread == pthread)
break;
}
if (thread == NULL) {
/*
* Search for the specified thread in the list of dead threads:
*/
TAILQ_FOREACH(thread, &_dead_list, dle) {
if (thread == pthread)
break;
}
}
/* Check if the thread was not found or has been detached: */
if (thread == NULL ||
((pthread->attr.flags & PTHREAD_DETACHED) != 0)) {
/* Undefer and handle pending signals, yielding if necessary: */
_thread_kern_sig_undefer();
/* Return an error: */
ret = ESRCH;
/* Check if this thread has been detached: */
else if ((pthread->attr.flags & PTHREAD_DETACHED) != 0)
/* Return an error: */
ret = ESRCH;
} else if (pthread->joiner != NULL) {
/* Undefer and handle pending signals, yielding if necessary: */
_thread_kern_sig_undefer();
else if (pthread->joiner != NULL)
/* Multiple joiners are not supported. */
ret = ENOTSUP;
/* Check if the thread is not dead: */
else if (pthread->state != PS_DEAD) {
} else if (pthread->state != PS_DEAD) {
/* Set the running thread to be the joiner: */
pthread->joiner = curthread;
@ -103,13 +143,11 @@ _pthread_join(pthread_t pthread, void **thread_return)
*thread_return = pthread->ret;
}
/*
* Make the thread collectable by the garbage collector. There
* is a race here with the garbage collector if multiple threads
* try to join the thread, but the behavior of multiple joiners
* is undefined, so don't bother protecting against the race.
*/
/* Make the thread collectable by the garbage collector. */
pthread->attr.flags |= PTHREAD_DETACHED;
/* Undefer and handle pending signals, yielding if necessary: */
_thread_kern_sig_undefer();
}
_thread_leave_cancellation_point();

View File

@ -64,35 +64,3 @@ _find_thread(pthread_t pthread)
/* Return zero if the thread exists: */
return ((pthread1 != NULL) ? 0:ESRCH);
}
/* Find a thread in the linked list of dead threads: */
int
_find_dead_thread(pthread_t pthread)
{
pthread_t pthread1;
/* Check if the caller has specified an invalid thread: */
if (pthread == NULL || pthread->magic != PTHREAD_MAGIC)
/* Invalid thread: */
return(EINVAL);
/*
* Lock the garbage collector mutex to ensure that the garbage
* collector is not using the dead thread list.
*/
if (pthread_mutex_lock(&_gc_mutex) != 0)
PANIC("Cannot lock gc mutex");
/* Search for the specified thread: */
TAILQ_FOREACH(pthread1, &_dead_list, dle) {
if (pthread1 == pthread)
break;
}
/* Unlock the garbage collector mutex: */
if (pthread_mutex_unlock(&_gc_mutex) != 0)
PANIC("Cannot lock gc mutex");
/* Return zero if the thread exists: */
return ((pthread1 != NULL) ? 0:ESRCH);
}

View File

@ -42,6 +42,7 @@ _pthread_join(pthread_t pthread, void **thread_return)
{
struct pthread *curthread = _get_curthread();
int ret = 0;
pthread_t thread;
_thread_enter_cancellation_point();
@ -60,24 +61,63 @@ _pthread_join(pthread_t pthread, void **thread_return)
}
/*
* Find the thread in the list of active threads or in the
* list of dead threads:
* Lock the garbage collector mutex to ensure that the garbage
* collector is not using the dead thread list.
*/
if ((_find_thread(pthread) != 0) && (_find_dead_thread(pthread) != 0))
if (pthread_mutex_lock(&_gc_mutex) != 0)
PANIC("Cannot lock gc mutex");
/*
* Defer signals to protect the thread list from access
* by the signal handler:
*/
_thread_kern_sig_defer();
/*
* Unlock the garbage collector mutex, now that the garbage collector
* can't be run:
*/
if (pthread_mutex_unlock(&_gc_mutex) != 0)
PANIC("Cannot lock gc mutex");
/*
* Search for the specified thread in the list of active threads. This
* is done manually here rather than calling _find_thread() because
* the searches in _thread_list and _dead_list (as well as setting up
* join/detach state) have to be done atomically.
*/
TAILQ_FOREACH(thread, &_thread_list, tle) {
if (thread == pthread)
break;
}
if (thread == NULL) {
/*
* Search for the specified thread in the list of dead threads:
*/
TAILQ_FOREACH(thread, &_dead_list, dle) {
if (thread == pthread)
break;
}
}
/* Check if the thread was not found or has been detached: */
if (thread == NULL ||
((pthread->attr.flags & PTHREAD_DETACHED) != 0)) {
/* Undefer and handle pending signals, yielding if necessary: */
_thread_kern_sig_undefer();
/* Return an error: */
ret = ESRCH;
/* Check if this thread has been detached: */
else if ((pthread->attr.flags & PTHREAD_DETACHED) != 0)
/* Return an error: */
ret = ESRCH;
} else if (pthread->joiner != NULL) {
/* Undefer and handle pending signals, yielding if necessary: */
_thread_kern_sig_undefer();
else if (pthread->joiner != NULL)
/* Multiple joiners are not supported. */
ret = ENOTSUP;
/* Check if the thread is not dead: */
else if (pthread->state != PS_DEAD) {
} else if (pthread->state != PS_DEAD) {
/* Set the running thread to be the joiner: */
pthread->joiner = curthread;
@ -103,13 +143,11 @@ _pthread_join(pthread_t pthread, void **thread_return)
*thread_return = pthread->ret;
}
/*
* Make the thread collectable by the garbage collector. There
* is a race here with the garbage collector if multiple threads
* try to join the thread, but the behavior of multiple joiners
* is undefined, so don't bother protecting against the race.
*/
/* Make the thread collectable by the garbage collector. */
pthread->attr.flags |= PTHREAD_DETACHED;
/* Undefer and handle pending signals, yielding if necessary: */
_thread_kern_sig_undefer();
}
_thread_leave_cancellation_point();

View File

@ -1207,7 +1207,6 @@ char *__ttyname_r_basic(int, char *, size_t);
char *ttyname_r(int, char *, size_t);
void _cond_wait_backout(pthread_t);
void _fd_lock_backout(pthread_t);
int _find_dead_thread(pthread_t);
int _find_thread(pthread_t);
struct pthread *_get_curthread(void);
void _set_curthread(struct pthread *);

View File

@ -64,35 +64,3 @@ _find_thread(pthread_t pthread)
/* Return zero if the thread exists: */
return ((pthread1 != NULL) ? 0:ESRCH);
}
/* Find a thread in the linked list of dead threads: */
int
_find_dead_thread(pthread_t pthread)
{
pthread_t pthread1;
/* Check if the caller has specified an invalid thread: */
if (pthread == NULL || pthread->magic != PTHREAD_MAGIC)
/* Invalid thread: */
return(EINVAL);
/*
* Lock the garbage collector mutex to ensure that the garbage
* collector is not using the dead thread list.
*/
if (pthread_mutex_lock(&_gc_mutex) != 0)
PANIC("Cannot lock gc mutex");
/* Search for the specified thread: */
TAILQ_FOREACH(pthread1, &_dead_list, dle) {
if (pthread1 == pthread)
break;
}
/* Unlock the garbage collector mutex: */
if (pthread_mutex_unlock(&_gc_mutex) != 0)
PANIC("Cannot lock gc mutex");
/* Return zero if the thread exists: */
return ((pthread1 != NULL) ? 0:ESRCH);
}

View File

@ -42,6 +42,7 @@ _pthread_join(pthread_t pthread, void **thread_return)
{
struct pthread *curthread = _get_curthread();
int ret = 0;
pthread_t thread;
_thread_enter_cancellation_point();
@ -60,24 +61,63 @@ _pthread_join(pthread_t pthread, void **thread_return)
}
/*
* Find the thread in the list of active threads or in the
* list of dead threads:
* Lock the garbage collector mutex to ensure that the garbage
* collector is not using the dead thread list.
*/
if ((_find_thread(pthread) != 0) && (_find_dead_thread(pthread) != 0))
if (pthread_mutex_lock(&_gc_mutex) != 0)
PANIC("Cannot lock gc mutex");
/*
* Defer signals to protect the thread list from access
* by the signal handler:
*/
_thread_kern_sig_defer();
/*
* Unlock the garbage collector mutex, now that the garbage collector
* can't be run:
*/
if (pthread_mutex_unlock(&_gc_mutex) != 0)
PANIC("Cannot lock gc mutex");
/*
* Search for the specified thread in the list of active threads. This
* is done manually here rather than calling _find_thread() because
* the searches in _thread_list and _dead_list (as well as setting up
* join/detach state) have to be done atomically.
*/
TAILQ_FOREACH(thread, &_thread_list, tle) {
if (thread == pthread)
break;
}
if (thread == NULL) {
/*
* Search for the specified thread in the list of dead threads:
*/
TAILQ_FOREACH(thread, &_dead_list, dle) {
if (thread == pthread)
break;
}
}
/* Check if the thread was not found or has been detached: */
if (thread == NULL ||
((pthread->attr.flags & PTHREAD_DETACHED) != 0)) {
/* Undefer and handle pending signals, yielding if necessary: */
_thread_kern_sig_undefer();
/* Return an error: */
ret = ESRCH;
/* Check if this thread has been detached: */
else if ((pthread->attr.flags & PTHREAD_DETACHED) != 0)
/* Return an error: */
ret = ESRCH;
} else if (pthread->joiner != NULL) {
/* Undefer and handle pending signals, yielding if necessary: */
_thread_kern_sig_undefer();
else if (pthread->joiner != NULL)
/* Multiple joiners are not supported. */
ret = ENOTSUP;
/* Check if the thread is not dead: */
else if (pthread->state != PS_DEAD) {
} else if (pthread->state != PS_DEAD) {
/* Set the running thread to be the joiner: */
pthread->joiner = curthread;
@ -103,13 +143,11 @@ _pthread_join(pthread_t pthread, void **thread_return)
*thread_return = pthread->ret;
}
/*
* Make the thread collectable by the garbage collector. There
* is a race here with the garbage collector if multiple threads
* try to join the thread, but the behavior of multiple joiners
* is undefined, so don't bother protecting against the race.
*/
/* Make the thread collectable by the garbage collector. */
pthread->attr.flags |= PTHREAD_DETACHED;
/* Undefer and handle pending signals, yielding if necessary: */
_thread_kern_sig_undefer();
}
_thread_leave_cancellation_point();

View File

@ -1207,7 +1207,6 @@ char *__ttyname_r_basic(int, char *, size_t);
char *ttyname_r(int, char *, size_t);
void _cond_wait_backout(pthread_t);
void _fd_lock_backout(pthread_t);
int _find_dead_thread(pthread_t);
int _find_thread(pthread_t);
struct pthread *_get_curthread(void);
void _set_curthread(struct pthread *);