From c9ad22033781a01c4cada45ad44482be2fc5e467 Mon Sep 17 00:00:00 2001 From: David Xu Date: Sun, 12 Mar 2006 11:28:06 +0000 Subject: [PATCH] Close a race for thread detach. --- lib/libthread_xu/thread/thr_create.c | 45 +++++++++++---------------- lib/libthread_xu/thread/thr_exit.c | 8 +++-- lib/libthread_xu/thread/thr_list.c | 21 +++++++------ lib/libthread_xu/thread/thr_private.h | 3 +- 4 files changed, 38 insertions(+), 39 deletions(-) diff --git a/lib/libthread_xu/thread/thr_create.c b/lib/libthread_xu/thread/thr_create.c index 3be88a723e..1f8e4a8099 100644 --- a/lib/libthread_xu/thread/thr_create.c +++ b/lib/libthread_xu/thread/thr_create.c @@ -32,7 +32,7 @@ * SUCH DAMAGE. * * $FreeBSD: src/lib/libpthread/thread/thr_create.c,v 1.58 2004/10/23 23:28:36 davidxu Exp $ - * $DragonFly: src/lib/libthread_xu/thread/thr_create.c,v 1.5 2005/10/10 13:45:57 davidxu Exp $ + * $DragonFly: src/lib/libthread_xu/thread/thr_create.c,v 1.6 2006/03/12 11:28:06 davidxu Exp $ */ #include #include @@ -55,16 +55,14 @@ struct start_arg { volatile umtx_t started; }; -static void free_thread(struct pthread *curthread, struct pthread *thread); static int create_stack(struct pthread_attr *pattr); -static void free_stack(struct pthread *curthread, struct pthread_attr *pattr); static void thread_start(void *); __weak_reference(_pthread_create, pthread_create); int _pthread_create(pthread_t * thread, const pthread_attr_t * attr, - void *(*start_routine) (void *), void *arg) + void *(*start_routine) (void *), void *arg) { struct start_arg start_arg; void *stack; @@ -152,7 +150,12 @@ _pthread_create(pthread_t * thread, const pthread_attr_t * attr, /* Initialise hooks in the thread structure: */ if (new_thread->attr.suspend == THR_CREATE_SUSPENDED) new_thread->flags = THR_FLAGS_NEED_SUSPEND; + new_thread->state = PS_RUNNING; + + if (new_thread->attr.flags & PTHREAD_CREATE_DETACHED) + new_thread->tlflags |= TLFLAGS_DETACHED; + /* * Thread created by thr_create() inherits currrent thread * sigmask, however, before new thread setup itself correctly, @@ -162,6 +165,7 @@ _pthread_create(pthread_t * thread, const pthread_attr_t * attr, __sys_sigprocmask(SIG_SETMASK, &sigmask, &oldsigmask); new_thread->sigmask = oldsigmask; /* Add the new thread. */ + new_thread->refcount = 1; _thr_link(curthread, new_thread); /* Return thread pointer eariler so that new thread can use it. */ (*thread) = new_thread; @@ -190,10 +194,16 @@ _pthread_create(pthread_t * thread, const pthread_attr_t * attr, ret = EAGAIN; } if (ret != 0) { - if (locked) - THR_THREAD_UNLOCK(curthread, new_thread); - _thr_unlink(curthread, new_thread); - free_thread(curthread, new_thread); + if (!locked) + THR_THREAD_LOCK(curthread, new_thread); + new_thread->state = PS_DEAD; + new_thread->terminated = 1; + THR_THREAD_UNLOCK(curthread, new_thread); + THREAD_LIST_LOCK(curthread); + _thread_active_threads--; + new_thread->tlflags |= TLFLAGS_DETACHED; + _thr_ref_delete_unlocked(curthread, new_thread); + THREAD_LIST_UNLOCK(curthread); (*thread) = 0; ret = EAGAIN; } else if (locked) { @@ -203,14 +213,6 @@ _pthread_create(pthread_t * thread, const pthread_attr_t * attr, return (ret); } -static void -free_thread(struct pthread *curthread, struct pthread *thread) -{ - free_stack(curthread, &thread->attr); - curthread->terminated = 1; - _thr_free(curthread, thread); -} - static int create_stack(struct pthread_attr *pattr) { @@ -227,17 +229,6 @@ create_stack(struct pthread_attr *pattr) return (ret); } -static void -free_stack(struct pthread *curthread, struct pthread_attr *pattr) -{ - if ((pattr->flags & THR_STACK_USER) == 0) { - THREAD_LIST_LOCK(curthread); - /* Stack routines don't use malloc/free. */ - _thr_stack_free(pattr); - THREAD_LIST_UNLOCK(curthread); - } -} - static void thread_start(void *arg) { diff --git a/lib/libthread_xu/thread/thr_exit.c b/lib/libthread_xu/thread/thr_exit.c index c7a5b8d237..8dc43bd5eb 100644 --- a/lib/libthread_xu/thread/thr_exit.c +++ b/lib/libthread_xu/thread/thr_exit.c @@ -30,7 +30,7 @@ * SUCH DAMAGE. * * $FreeBSD: src/lib/libpthread/thread/thr_exit.c,v 1.39 2004/10/23 23:37:54 davidxu Exp $ - * $DragonFly: src/lib/libthread_xu/thread/thr_exit.c,v 1.3 2005/05/07 09:29:46 davidxu Exp $ + * $DragonFly: src/lib/libthread_xu/thread/thr_exit.c,v 1.4 2006/03/12 11:28:06 davidxu Exp $ */ #include @@ -129,12 +129,16 @@ _pthread_exit(void *status) exit(0); /* Never reach! */ } + THR_LOCK(curthread); + curthread->state = PS_DEAD; + THR_UNLOCK(curthread); + curthread->refcount--; if (curthread->tlflags & TLFLAGS_DETACHED) THR_GCLIST_ADD(curthread); - curthread->state = PS_DEAD; THREAD_LIST_UNLOCK(curthread); if (curthread->joiner) _thr_umtx_wake(&curthread->state, INT_MAX); + /* XXX * All the code is incorrect on DragonFly. * DragonFly has race condition here. it should provide diff --git a/lib/libthread_xu/thread/thr_list.c b/lib/libthread_xu/thread/thr_list.c index 40957c4752..41f4580cf2 100644 --- a/lib/libthread_xu/thread/thr_list.c +++ b/lib/libthread_xu/thread/thr_list.c @@ -24,7 +24,7 @@ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * - * $DragonFly: src/lib/libthread_xu/thread/thr_list.c,v 1.3 2005/03/29 19:26:20 joerg Exp $ + * $DragonFly: src/lib/libthread_xu/thread/thr_list.c,v 1.4 2006/03/12 11:28:06 davidxu Exp $ */ #include @@ -38,7 +38,7 @@ #include "thr_private.h" #include "libc_private.h" -/*#define DEBUG_THREAD_LIST */ +#define DEBUG_THREAD_LIST #ifdef DEBUG_THREAD_LIST #define DBG_MSG stdout_debug #else @@ -131,7 +131,6 @@ _thr_gc(struct pthread *curthread) continue; } - DBG_MSG("Freeing thread %p\n", td); _thr_free(curthread, td); } } @@ -236,8 +235,6 @@ _thr_link(struct pthread *curthread, struct pthread *thread) */ thread->uniqueid = next_uniqueid++; THR_LIST_ADD(thread); - if (thread->attr.flags & PTHREAD_DETACHED) - thread->tlflags |= TLFLAGS_DETACHED; _thread_active_threads++; THREAD_LIST_UNLOCK(curthread); } @@ -310,14 +307,20 @@ _thr_ref_add(struct pthread *curthread, struct pthread *thread, void _thr_ref_delete(struct pthread *curthread, struct pthread *thread) +{ + THREAD_LIST_LOCK(curthread); + _thr_ref_delete_unlocked(curthread, thread); + THREAD_LIST_UNLOCK(curthread); +} + +void +_thr_ref_delete_unlocked(struct pthread *curthread, struct pthread *thread) { if (thread != NULL) { - THREAD_LIST_LOCK(curthread); thread->refcount--; - if ((thread->refcount == 0) && - (thread->tlflags & TLFLAGS_GC_SAFE) != 0) + if ((thread->refcount == 0) && thread->state == PS_DEAD && + (thread->tlflags & TLFLAGS_DETACHED) != 0) THR_GCLIST_ADD(thread); - THREAD_LIST_UNLOCK(curthread); } } diff --git a/lib/libthread_xu/thread/thr_private.h b/lib/libthread_xu/thread/thr_private.h index 13f715824a..19633ec860 100644 --- a/lib/libthread_xu/thread/thr_private.h +++ b/lib/libthread_xu/thread/thr_private.h @@ -32,7 +32,7 @@ * Private thread definitions for the uthread kernel. * * $FreeBSD: src/lib/libpthread/thread/thr_private.h,v 1.120 2004/11/01 10:49:34 davidxu Exp $ - * $DragonFly: src/lib/libthread_xu/thread/thr_private.h,v 1.7 2005/06/21 07:47:01 davidxu Exp $ + * $DragonFly: src/lib/libthread_xu/thread/thr_private.h,v 1.8 2006/03/12 11:28:06 davidxu Exp $ */ #ifndef _THR_PRIVATE_H @@ -721,6 +721,7 @@ void _thread_exit(char *, int, char *) __dead2; void _thr_exit_cleanup(void); int _thr_ref_add(struct pthread *, struct pthread *, int); void _thr_ref_delete(struct pthread *, struct pthread *); +void _thr_ref_delete_unlocked(struct pthread *, struct pthread *); int _thr_find_thread(struct pthread *, struct pthread *, int); void _thr_rtld_init(void); void _thr_rtld_fini(void); -- 2.41.0