From 531ca4db4794aab863a898b4d079ccd59b424b25 Mon Sep 17 00:00:00 2001 From: rpj Date: Mon, 13 Sep 2004 04:32:16 +0000 Subject: Clarify behaviour and remove some redundant code - see ChangeLogs --- pthread_join.c | 163 +++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 112 insertions(+), 51 deletions(-) (limited to 'pthread_join.c') diff --git a/pthread_join.c b/pthread_join.c index 97cbd51..89ee1a5 100644 --- a/pthread_join.c +++ b/pthread_join.c @@ -83,22 +83,62 @@ pthread_join (pthread_t thread, void **value_ptr) int result; pthread_t self; - /* This is the proper way to test for a valid thread ID */ - result = pthread_kill (thread, 0); - if (0 != result) + /* + * Possibilities for the target thread on entry to pthread_join(): + * + * 1) the target thread is detached, in which case it could be destroyed and + * it's thread id (struct) reused before we get the reuse lock; + * 2) the target thread is joinable, in which case it will not be destroyed + * until it has been joined, and it's thread id cannot be reused before we + * get access to it. + * + * Only (1) is a potential race condition. + * While (1) is possibly an application programming error, pthread_join is + * required to handle it with an error. + * If an application attempts to join a detached thread that exits before we + * get the reuse lock, it's thread struct could be reused for a new joinable + * thread before we get the reuse_lock and we will then be joining the wrong + * thread. + * + * To fix this properly would require a reuse count as part of the thread id + * so that we could confirm that we are working with the same thread. This + * option will require a major change to the API, forcing recompilation of + * applications that use the library. + * + * Cheaper alternatives that do not change the API are: + * - separate reuse stacks could be used for detached and joinable + * threads. Threads which detach themselves will not present a race condition + * because they will be placed on a different stack to that which provides + * reusable structs to new joinable threads. + * The problem with this solution is that, an application that creates + * joinable threads which then detach themselves can cause the detach reuse + * stack to grow indefinitely. + * + * - maintain a sufficiently large reuse pool and manage it as a FIFO + * queue. This would prevent the immediate reuse of thread structs, but this + * only provides a statistical safeguard, not a deterministic one. + * + * - include an entry point array at the start of each thread struct so that + * each reuse (modulo N) returns a different thread id (pointer to the thread + * struct). Offset 0 from this pointer will contain a validity field and an + * offset value field (subtracted from the pointer to obtain the address of + * thread struct). This option is similar to including a reuse counter with + * the thread id but maintains the thread id as a simple pointer. + * + * As at 27/08/2004, none of the above have been implemented. + */ + + EnterCriticalSection (&ptw32_thread_reuse_lock); + /* + * The first test is the same as pthread_kill(thread, 0), duplicated here + * so that we can use the reuse_lock to ensure the thread isn't destroyed + * and reused before we've finished with the POSIX thread struct. + */ + if (NULL == thread + || NULL == thread->threadH + || THREAD_PRIORITY_ERROR_RETURN == GetThreadPriority (thread->threadH)) { - return result; - } - - self = pthread_self (); - if (NULL == self) - { - return ENOENT; - } - - if (0 != pthread_equal (self, thread)) - { - result = EDEADLK; + result = ESRCH; } else if (PTHREAD_CREATE_DETACHED == thread->detachState) { @@ -106,60 +146,81 @@ pthread_join (pthread_t thread, void **value_ptr) } else { - /* - * Pthread_join is a cancelation point. - * If we are canceled then our target thread must not be - * detached (destroyed). This is guarranteed because - * pthreadCancelableWait will not return if we - * are canceled. + /* + * The target thread is joinable and can't be reused before we join it. */ - result = pthreadCancelableWait (thread->threadH); - if (result == 0) + LeaveCriticalSection (&ptw32_thread_reuse_lock); + + if (NULL == (self = pthread_self ())) + { + result = ENOENT; + } + else if (0 != pthread_equal (self, thread)) + { + result = EDEADLK; + } + else { + /* + * Pthread_join is a cancelation point. + * If we are canceled then our target thread must not be + * detached (destroyed). This is guarranteed because + * pthreadCancelableWait will not return if we + * are canceled. + */ + result = pthreadCancelableWait (thread->threadH); + + if (0 == result) + { #if ! defined (__MINGW32__) || defined (__MSVCRT__) || defined (__DMC__) - if (value_ptr != NULL - && !GetExitCodeThread (thread->threadH, (LPDWORD) value_ptr)) - { - result = ESRCH; - } - else - { + if (value_ptr != NULL + && !GetExitCodeThread (thread->threadH, (LPDWORD) value_ptr)) + { + result = ESRCH; + } + else + { + /* + * The result of making multiple simultaneous calls to + * pthread_join() specifying the same target is undefined. + */ + ptw32_threadDestroy (thread); + } + +#else /* __MINGW32__ && ! __MSVCRT__ */ + + /* + * If using CRTDLL, the thread may have exited, and endthread + * will have closed the handle. + */ + if (value_ptr != NULL) + { + *value_ptr = thread->exitStatus; + } + /* * The result of making multiple simultaneous calls to * pthread_join() specifying the same target is undefined. */ ptw32_threadDestroy (thread); - } -#else /* __MINGW32__ && ! __MSVCRT__ */ +#endif /* __MINGW32__ && ! __MSVCRT__ */ - /* - * If using CRTDLL, the thread may have exited, and endthread - * will have closed the handle. - */ - if (value_ptr != NULL) + } + else { - *value_ptr = thread->exitStatus; + result = ESRCH; } - - /* - * The result of making multiple simultaneous calls to - * pthread_join() specifying the same target is undefined. - */ - ptw32_threadDestroy (thread); - -#endif /* __MINGW32__ && ! __MSVCRT__ */ - - } - else - { - result = ESRCH; } + + goto FAIL0; } + LeaveCriticalSection (&ptw32_thread_reuse_lock); +FAIL0: return (result); } /* pthread_join */ -- cgit v1.2.3