diff options
author | patacongo <patacongo@42af7a65-404d-4744-a932-0658087f49c3> | 2007-03-30 13:11:19 +0000 |
---|---|---|
committer | patacongo <patacongo@42af7a65-404d-4744-a932-0658087f49c3> | 2007-03-30 13:11:19 +0000 |
commit | 303a7c688b072a0a674bd5a8c55019b56b7db79a (patch) | |
tree | d699368973f43600ab2c29b398bc5922bb77532c | |
parent | 0bd4fc137b7ba0218159b8ded673efcb9c404ce1 (diff) | |
download | px4-nuttx-303a7c688b072a0a674bd5a8c55019b56b7db79a.tar.gz px4-nuttx-303a7c688b072a0a674bd5a8c55019b56b7db79a.tar.bz2 px4-nuttx-303a7c688b072a0a674bd5a8c55019b56b7db79a.zip |
Fix another potential pthread_join race condition
git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@181 42af7a65-404d-4744-a932-0658087f49c3
-rw-r--r-- | nuttx/sched/pthread_join.c | 116 |
1 files changed, 70 insertions, 46 deletions
diff --git a/nuttx/sched/pthread_join.c b/nuttx/sched/pthread_join.c index 71ea7e9f0..37908291b 100644 --- a/nuttx/sched/pthread_join.c +++ b/nuttx/sched/pthread_join.c @@ -82,12 +82,13 @@ * * Return Value: * 0 if successful. Otherwise, one of the following error codes: - * EINVAL The value specified by thread does not refer to a - * joinable thread. - * ESRCH No thread could be found corresponding to that - * specified by the given thread ID. - * EDEADLK A deadlock was detected or the value of thread - * specifies the calling thread. + * + * EINVAL The value specified by thread does not refer to a + * joinable thread. + * ESRCH No thread could be found corresponding to that + * specified by the given thread ID. + * EDEADLK A deadlock was detected or the value of thread + * specifies the calling thread. * * Assumptions: * @@ -144,76 +145,99 @@ int pthread_join(pthread_t thread, pthread_addr_t *pexit_value) * case, it must be a task and not a pthread. */ - else /* if ((tcb->flags & EDEADLK) == 0) */ + else { ret = EINVAL; } (void)pthread_givesemaphore(&g_join_semaphore); } - else if (pjoin->terminated) + else { - dbg("Thread has terminated\n"); + /* We found the join info structure. Increment for the reference + * to the join structure that we have. This will keep things + * stable for we have to do + */ - /* Get the thread exit value from the terminated thread. */ + sched_lock(); + pjoin->crefs++; + + /* Check if the thread is still running. If not, then things are + * simpler. There are still race conditions to be concerned with. + * For example, there could be multiple threads executing in the + * 'else' block below when we enter! + */ - if (pexit_value) + if (pjoin->terminated) { - dbg("exit_value=0x%p\n", pjoin->exit_value); - *pexit_value = pjoin->exit_value; + dbg("Thread has terminated\n"); + + /* Get the thread exit value from the terminated thread. */ + + if (pexit_value) + { + dbg("exit_value=0x%p\n", pjoin->exit_value); + *pexit_value = pjoin->exit_value; + } } + else + { + dbg("Thread is still running\n"); - /* Then remove and deallocate the thread entry. */ + /* Relinquish the data set semaphore. Since pre-emption is + * disabled, we can be certain that no task has the + * opportunity to run between the time we relinquish the + * join semaphore and the time that we wait on the thread exit + * semaphore. + */ - (void)pthread_removejoininfo((pid_t)thread); - (void)pthread_givesemaphore(&g_join_semaphore); - sched_free(pjoin); - ret = OK; - } - else - { - dbg("Thread is still running\n"); + (void)pthread_givesemaphore(&g_join_semaphore); - /* Relinquish the data set semaphore, making certain that - * no task has the opportunity to run between the time - * we relinquish the data set semaphore and the time that - * we wait on the join semaphore. - */ + /* Take the thread's thread exit semaphore. We will sleep here + * until the thread exits. We need to exercise caution because + * there could be multiple threads waiting here for the same + * pthread to exit. + */ - sched_lock(); - pjoin->crefs++; - (void)pthread_givesemaphore(&g_join_semaphore); + (void)pthread_takesemaphore(&pjoin->exit_sem); - /* Take the thread's join semaphore */ + /* The thread has exited! Get the thread exit value */ - (void)pthread_takesemaphore(&pjoin->exit_sem); + if (pexit_value) + { + *pexit_value = pjoin->exit_value; + dbg("exit_value=0x%p\n", pjoin->exit_value); + } - /* Get the thread exit value */ + /* Post the thread's data semaphore so that the exitting thread + * will know that we have received the data. + */ - if (pexit_value) - { - *pexit_value = pjoin->exit_value; - dbg("exit_value=0x%p\n", pjoin->exit_value); - } + (void)pthread_givesemaphore(&pjoin->data_sem); - /* Post the thread's join semaphore so that exitting thread - * will know that we have received the data. - */ + /* Retake the join semaphore, we need to hold this when + * pthread_destroyjoin is called. + */ - (void)pthread_givesemaphore(&pjoin->data_sem); + (void)pthread_takesemaphore(&g_join_semaphore); + } - /* Pre-emption is okay now. */ + /* Pre-emption is okay now. The logic still cannot be re-entered + * because we hold the join semaphore + */ sched_unlock(); - /* Deallocate the thread entry */ + /* Release our reference to the join structure and, if the reference + * count decrements to zero, deallocate the join structure. + */ - (void)pthread_takesemaphore(&g_join_semaphore); if (--pjoin->crefs <= 0) { - (void)pthread_destroyjoin(pjoin); + (void)pthread_destroyjoin(pjoin); } (void)pthread_givesemaphore(&g_join_semaphore); + ret = OK; } |