Date: Sat, 8 Jan 2005 00:15:58 GMT From: David Xu <davidxu@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 68509 for review Message-ID: <200501080015.j080Fwkx078032@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=68509 Change 68509 by davidxu@davidxu_tiger on 2005/01/08 00:15:00 Link owned simple mutex, so after fork, we can fix the ownership of the mutex, for simple mutex, umtx owner id is thread id, but after fork, in child process, current thread is a new kernel thread, its thread id is different than it is in parent process. POSIX spec about pthread_atfork implies that mutex may be inherited from parent process, but it is not said in fork() spec, indeed, David R.Butenhof's book has such example program does call pthread_unlock after fork() in child process. Broken POSIX Specification! Affected files ... .. //depot/projects/davidxu_thread/src/lib/libthread/thread/Makefile.inc#5 edit .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_atfork.c#5 edit .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_cond.c#10 edit .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_create.c#5 edit .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_fork.c#7 edit .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_init.c#8 edit .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_mutex.c#13 edit .. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_private.h#17 edit Differences ... ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/Makefile.inc#5 (text+ko) ==== @@ -4,7 +4,6 @@ .PATH: ${.CURDIR}/thread SRCS+= \ - thr_atfork.c \ thr_attr.c \ thr_autoinit.c \ thr_barrier.c \ ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_atfork.c#5 (text+ko) ==== @@ -37,6 +37,7 @@ _pthread_atfork(void (*prepare)(void), void (*parent)(void), void (*child)(void)) { + struct pthread *curthread; struct pthread_atfork *af; _thr_check_init(); @@ -44,12 +45,12 @@ if ((af = malloc(sizeof(struct pthread_atfork))) == NULL) return (ENOMEM); + curthread = _get_curthread(); af->prepare = prepare; af->parent = parent; af->child = child; - _pthread_mutex_lock(&_thr_atfork_mutex); + THR_UMTX_LOCK(curthread, &_thr_atfork_lock); TAILQ_INSERT_TAIL(&_thr_atfork_list, af, qe); - _pthread_mutex_unlock(&_thr_atfork_mutex); + THR_UMTX_UNLOCK(curthread, &_thr_atfork_lock); return (0); } - ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_cond.c#10 (text+ko) ==== @@ -98,9 +98,9 @@ rval = EINVAL; else { /* Lock the condition variable structure: */ - THR_LOCK_ACQUIRE(curthread, &(*cond)->c_lock); + THR_UMTX_LOCK(curthread, &(*cond)->c_lock); if ((*cond)->c_waiters + (*cond)->c_wakeups != 0) { - THR_LOCK_RELEASE(curthread, &(*cond)->c_lock); + THR_UMTX_UNLOCK(curthread, &(*cond)->c_lock); return (EBUSY); } @@ -112,7 +112,7 @@ *cond = NULL; /* Unlock the condition variable structure: */ - THR_LOCK_RELEASE(curthread, &cv->c_lock); + THR_UMTX_UNLOCK(curthread, &cv->c_lock); /* Free the cond lock structure: */ @@ -142,7 +142,7 @@ pthread_cond_t cv; cv = *(cci->cond); - THR_LOCK_ACQUIRE(curthread, &cv->c_lock); + THR_UMTX_LOCK(curthread, &cv->c_lock); if (cv->c_seqno != cci->seqno && cv->c_wakeups != 0) { if (cv->c_waiters > 0) { cv->c_seqno++; @@ -152,7 +152,7 @@ } else { cv->c_waiters--; } - THR_LOCK_RELEASE(curthread, &cv->c_lock); + THR_UMTX_UNLOCK(curthread, &cv->c_lock); _mutex_cv_lock(cci->mutex); } @@ -177,10 +177,10 @@ return (ret); cv = *cond; - THR_LOCK_ACQUIRE(curthread, &cv->c_lock); + THR_UMTX_LOCK(curthread, &cv->c_lock); ret = _mutex_cv_unlock(mutex); if (ret) { - THR_LOCK_RELEASE(curthread, &cv->c_lock); + THR_UMTX_UNLOCK(curthread, &cv->c_lock); return (ret); } oldseq = seq = cv->c_seqno; @@ -192,18 +192,18 @@ do { if (cancel) { THR_CLEANUP_PUSH(curthread, cond_cancel_handler, &cci); - THR_LOCK_RELEASE(curthread, &cv->c_lock); + THR_UMTX_UNLOCK(curthread, &cv->c_lock); oldcancel = _thr_cancel_enter(curthread); ret = umtx_timedwait((struct umtx *)&cv->c_seqno, seq, abstime); _thr_cancel_leave(curthread, oldcancel); THR_CLEANUP_POP(curthread, 0); } else { - THR_LOCK_RELEASE(curthread, &cv->c_lock); + THR_UMTX_UNLOCK(curthread, &cv->c_lock); ret = umtx_timedwait((struct umtx *)&cv->c_seqno, seq, abstime); } - THR_LOCK_ACQUIRE(curthread, &cv->c_lock); + THR_UMTX_LOCK(curthread, &cv->c_lock); seq = cv->c_seqno; if (abstime != NULL && ret != 0) { if (ret == EINTR) @@ -222,7 +222,7 @@ } else { cv->c_waiters--; } - THR_LOCK_RELEASE(curthread, &cv->c_lock); + THR_UMTX_UNLOCK(curthread, &cv->c_lock); _mutex_cv_lock(mutex); return (ret); } @@ -285,7 +285,7 @@ cv = *cond; /* Lock the condition variable structure. */ - THR_LOCK_ACQUIRE(curthread, &cv->c_lock); + THR_UMTX_LOCK(curthread, &cv->c_lock); if (cv->c_waiters) { if (!broadcast) { cv->c_wakeups++; @@ -299,7 +299,7 @@ umtx_wake((struct umtx *)&cv->c_seqno, INT_MAX); } } - THR_LOCK_RELEASE(curthread, &cv->c_lock); + THR_UMTX_UNLOCK(curthread, &cv->c_lock); return (ret); } ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_create.c#5 (text+ko) ==== @@ -173,6 +173,7 @@ /* Initialize the mutex queue: */ TAILQ_INIT(&new_thread->mutexq); + TAILQ_INIT(&new_thread->pri_mutexq); /* Initialise hooks in the thread structure: */ new_thread->specific = NULL; ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_fork.c#7 (text+ko) ==== @@ -43,6 +43,30 @@ #include "libc_private.h" #include "thr_private.h" +__weak_reference(_pthread_atfork, pthread_atfork); + +int +_pthread_atfork(void (*prepare)(void), void (*parent)(void), + void (*child)(void)) +{ + struct pthread *curthread; + struct pthread_atfork *af; + + _thr_check_init(); + + if ((af = malloc(sizeof(struct pthread_atfork))) == NULL) + return (ENOMEM); + + curthread = _get_curthread(); + af->prepare = prepare; + af->parent = parent; + af->child = child; + THR_UMTX_LOCK(curthread, &_thr_atfork_lock); + TAILQ_INSERT_TAIL(&_thr_atfork_list, af, qe); + THR_UMTX_UNLOCK(curthread, &_thr_atfork_lock); + return (0); +} + /* * For a while, allow libpthread to work with a libc that doesn't * export the malloc lock. @@ -54,11 +78,14 @@ pid_t _fork(void) { + static long inprogress, waiters; + sigset_t sigset, oldset; struct pthread *curthread; struct pthread_atfork *af; pid_t ret; - int errsave; + long tmp; + int errsave, unlock_malloc; if (!_thr_is_inited()) return (__sys_fork()); @@ -71,7 +98,19 @@ SIGFILLSET(sigset); __sys_sigprocmask(SIG_SETMASK, &sigset, &oldset); - _pthread_mutex_lock(&_thr_atfork_mutex); + /* We allow new hook to be added when executing hooks. */ + THR_UMTX_LOCK(curthread, &_thr_atfork_lock); + tmp = inprogress; + while (tmp) { + waiters++; + THR_UMTX_UNLOCK(curthread, &_thr_atfork_lock); + umtx_wait((struct umtx *)&inprogress, tmp); + THR_UMTX_LOCK(curthread, &_thr_atfork_lock); + waiters--; + tmp = inprogress; + } + inprogress = 1; + THR_UMTX_UNLOCK(curthread, &_thr_atfork_lock); /* Run down atfork prepare handlers. */ TAILQ_FOREACH_REVERSE(af, &_thr_atfork_list, atfork_head, qe) { @@ -79,28 +118,39 @@ af->prepare(); } - /* Fork a new process: */ - if ((_thr_isthreaded() != 0) && (__malloc_lock != NULL)) + /* + * Try our best to protect memory from being corrupted in + * child process because another thread in malloc code will + * simply be kill by fork(). + */ + if ((_thr_isthreaded() != 0) && (__malloc_lock != NULL)) { + unlock_malloc = 1; _spinlock(__malloc_lock); + } else { + unlock_malloc = 0; + } + /* Fork a new process: */ if ((ret = __sys_fork()) == 0) { /* Child process */ errsave = errno; + inprogress = 0; curthread->cancelflags &= ~THR_CANCEL_NEEDED; curthread->critical_count = 0; curthread->locklevel = 0; + thr_self(&curthread->tid); + /* clear other threads locked us. */ umtx_init(&curthread->lock); - thr_self(&curthread->tid); + umtx_init(&_thr_atfork_lock); _thr_setthreaded(0); - _mutex_reinit(&_thr_atfork_mutex); - /* reinitialize libc spinlocks, this includes __malloc_lock. */ _thr_spinlock_init(); - TAILQ_INIT(&curthread->mutexq); + _mutex_fork(curthread); + curthread->priority_mutex_count = 0; /* reinit library. */ @@ -118,7 +168,7 @@ /* Parent process */ errsave = errno; - if ((_thr_isthreaded() != 0) && (__malloc_lock != NULL)) + if (unlock_malloc) _spinunlock(__malloc_lock); /* Restore signal mask. */ @@ -129,7 +179,12 @@ if (af->parent != NULL) af->parent(); } - _pthread_mutex_unlock(&_thr_atfork_mutex); + + THR_UMTX_LOCK(curthread, &_thr_atfork_lock); + inprogress = 0; + if (waiters) + umtx_wake((struct umtx *)&inprogress, waiters); + THR_UMTX_UNLOCK(curthread, &_thr_atfork_lock); } errno = errsave; ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_init.c#8 (text+ko) ==== @@ -348,6 +348,7 @@ /* Initialize the mutex queue: */ TAILQ_INIT(&thread->mutexq); + TAILQ_INIT(&thread->pri_mutexq); /* Initialize hooks in the thread structure: */ thread->specific = NULL; @@ -365,6 +366,10 @@ size_t len; int mib[2]; + umtx_init(&_thread_signal_lock); + umtx_init(&_keytable_lock); + umtx_init(&_thr_atfork_lock); + _thr_spinlock_init(); _thr_list_init(); /* @@ -383,25 +388,6 @@ _pthread_attr_default.guardsize_attr = _thr_guard_default; TAILQ_INIT(&_thr_atfork_list); - - umtx_init(&_thread_signal_lock); - umtx_init(&_mutex_static_lock); - umtx_init(&_cond_static_lock); - umtx_init(&_rwlock_static_lock); - umtx_init(&_keytable_lock); - _thr_spinlock_init(); - _pthread_mutex_init(&_thr_atfork_mutex, NULL); - } else { - umtx_init(&_thread_signal_lock); - umtx_init(&_mutex_static_lock); - umtx_init(&_cond_static_lock); - umtx_init(&_rwlock_static_lock); - umtx_init(&_keytable_lock); - /* reinitialized in thr_fork.c */ -#if 0 - _thr_spinlock_init(); - _mutex_reinit(&_thr_atfork_mutex); -#endif } #ifdef SYSTEM_SCOPE_ONLY ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_mutex.c#13 (text+ko) ==== @@ -86,10 +86,6 @@ static inline void mutex_queue_enq(pthread_mutex_t, pthread_t); static void mutex_lock_backout(void *arg); -static struct pthread_mutex_attr static_mutex_attr = - PTHREAD_MUTEXATTR_STATIC_INITIALIZER; -static pthread_mutexattr_t static_mattr = &static_mutex_attr; - __weak_reference(__pthread_mutex_init, pthread_mutex_init); __weak_reference(__pthread_mutex_lock, pthread_mutex_lock); __weak_reference(__pthread_mutex_timedlock, pthread_mutex_timedlock); @@ -215,6 +211,7 @@ return mutex_init(mutex, mutex_attr, 0); } +/* used to reinitialize mutex after fork() */ int _mutex_reinit(pthread_mutex_t *mutex) { @@ -229,6 +226,24 @@ return (0); } +void +_mutex_fork(struct pthread *curthread) +{ + struct pthread_mutex *m; + + /* After fork, tid was changed, fix ownership. */ + TAILQ_FOREACH(m, &curthread->mutexq, m_qe) { + m->m_lock.u_owner = (void *)curthread->tid; + } + + /* Clear contender for priority mutexes */ + TAILQ_FOREACH(m, &curthread->pri_mutexq, m_qe) { + /* clear another thread locked us */ + umtx_init(&m->m_lock); + TAILQ_INIT(&m->m_queue); + } +} + int _pthread_mutex_destroy(pthread_mutex_t *mutex) { @@ -299,12 +314,10 @@ * owned mutex, separated it from priority * mutex list */ -#if 0 /* Add to the list of owned mutexes: */ MUTEX_ASSERT_NOT_OWNED(*mutex); TAILQ_INSERT_TAIL(&curthread->mutexq, (*mutex), m_qe); -#endif } else if (umtx_owner(&(*mutex)->m_lock) == curthread->tid) { ret = mutex_self_trylock(curthread, *mutex); } /* else {} */ @@ -352,7 +365,7 @@ /* Add to the list of owned mutexes: */ MUTEX_ASSERT_NOT_OWNED(*mutex); - TAILQ_INSERT_TAIL(&curthread->mutexq, + TAILQ_INSERT_TAIL(&curthread->pri_mutexq, (*mutex), m_qe); } else if ((*mutex)->m_owner == curthread) ret = mutex_self_trylock(curthread, *mutex); @@ -389,7 +402,7 @@ THR_UNLOCK(curthread); /* Add to the list of owned mutexes: */ MUTEX_ASSERT_NOT_OWNED(*mutex); - TAILQ_INSERT_TAIL(&curthread->mutexq, + TAILQ_INSERT_TAIL(&curthread->pri_mutexq, (*mutex), m_qe); } else if ((*mutex)->m_owner == curthread) ret = mutex_self_trylock(curthread, *mutex); @@ -474,12 +487,10 @@ * owned mutex, separated it from priority * mutex list */ -#if 0 /* Add to the list of owned mutexes: */ MUTEX_ASSERT_NOT_OWNED(*m); TAILQ_INSERT_TAIL(&curthread->mutexq, (*m), m_qe); -#endif } else if (umtx_owner(&(*m)->m_lock) == curthread->tid) { ret = mutex_self_lock(curthread, *m, abstime); } else { @@ -498,12 +509,10 @@ } if (ret == 0) { (*m)->m_owner = curthread; -#if 0 /* Add to the list of owned mutexes: */ MUTEX_ASSERT_NOT_OWNED(*m); TAILQ_INSERT_TAIL(&curthread->mutexq, (*m), m_qe); -#endif } } return (ret); @@ -564,7 +573,7 @@ /* Add to the list of owned mutexes: */ MUTEX_ASSERT_NOT_OWNED(*m); - TAILQ_INSERT_TAIL(&curthread->mutexq, + TAILQ_INSERT_TAIL(&curthread->pri_mutexq, (*m), m_qe); /* Unlock the mutex structure: */ @@ -652,7 +661,7 @@ /* Add to the list of owned mutexes: */ MUTEX_ASSERT_NOT_OWNED(*m); - TAILQ_INSERT_TAIL(&curthread->mutexq, + TAILQ_INSERT_TAIL(&curthread->pri_mutexq, (*m), m_qe); /* Unlock the mutex structure: */ @@ -976,13 +985,10 @@ * for owned mutex, separated it from * priority mutex list */ -#if 0 /* Remove the mutex from the threads queue. */ MUTEX_ASSERT_IS_OWNED(*m); - TAILQ_REMOVE(&(*m)->m_owner->mutexq, - (*m), m_qe); + TAILQ_REMOVE(&curthread->mutexq, (*m), m_qe); MUTEX_INIT_LINK(*m); -#endif if (add_reference) (*m)->m_refcount++; /* @@ -1041,7 +1047,7 @@ /* Remove the mutex from the threads queue. */ MUTEX_ASSERT_IS_OWNED(*m); - TAILQ_REMOVE(&(*m)->m_owner->mutexq, + TAILQ_REMOVE(&(*m)->m_owner->pri_mutexq, (*m), m_qe); MUTEX_INIT_LINK(*m); @@ -1093,7 +1099,7 @@ /* Remove the mutex from the threads queue. */ MUTEX_ASSERT_IS_OWNED(*m); - TAILQ_REMOVE(&(*m)->m_owner->mutexq, + TAILQ_REMOVE(&(*m)->m_owner->pri_mutexq, (*m), m_qe); MUTEX_INIT_LINK(*m); @@ -1153,13 +1159,13 @@ * simultaneous call to change the threads priority * and from the owning thread releasing the mutex. */ - m = TAILQ_FIRST(&pthread->mutexq); + m = TAILQ_FIRST(&pthread->pri_mutexq); if (m != NULL) { THR_LOCK_ACQUIRE(curthread, &m->m_lock); /* * Make sure the thread still owns the lock. */ - if (m == TAILQ_FIRST(&pthread->mutexq)) + if (m == TAILQ_FIRST(&pthread->pri_mutexq)) mutex_rescan_owned(curthread, pthread, /* rescan all owned */ NULL); THR_LOCK_RELEASE(curthread, &m->m_lock); @@ -1379,7 +1385,7 @@ * A null mutex means start at the beginning of the owned * mutex list. */ - m = TAILQ_FIRST(&pthread->mutexq); + m = TAILQ_FIRST(&pthread->pri_mutexq); /* There is no inherited priority yet. */ inherited_prio = 0; @@ -1477,7 +1483,7 @@ { struct pthread_mutex *m, *m_next; - for (m = TAILQ_FIRST(&pthread->mutexq); m != NULL; m = m_next) { + for (m = TAILQ_FIRST(&pthread->pri_mutexq); m != NULL; m = m_next) { m_next = TAILQ_NEXT(m, m_qe); if ((m->m_flags & MUTEX_FLAGS_PRIVATE) != 0) pthread_mutex_unlock(&m); @@ -1573,7 +1579,7 @@ * thread's list of owned mutexes. */ mutex->m_owner = pthread; - TAILQ_INSERT_TAIL(&pthread->mutexq, mutex, m_qe); + TAILQ_INSERT_TAIL(&pthread->pri_mutexq, mutex, m_qe); break; case PTHREAD_PRIO_INHERIT: @@ -1582,7 +1588,7 @@ * thread's list of owned mutexes. */ mutex->m_owner = pthread; - TAILQ_INSERT_TAIL(&pthread->mutexq, mutex, m_qe); + TAILQ_INSERT_TAIL(&pthread->pri_mutexq, mutex, m_qe); /* Track number of priority mutexes owned: */ pthread->priority_mutex_count++; @@ -1621,7 +1627,7 @@ * to the thread's list of owned mutexes. */ mutex->m_owner = pthread; - TAILQ_INSERT_TAIL(&pthread->mutexq, + TAILQ_INSERT_TAIL(&pthread->pri_mutexq, mutex, m_qe); /* Track number of priority mutexes owned: */ ==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_private.h#17 (text+ko) ==== @@ -502,11 +502,12 @@ /* Number of priority ceiling or protection mutexes owned. */ int priority_mutex_count; - /* - * Queue of currently owned mutexes. - */ + /* Queue of currently owned simple type mutexes. */ TAILQ_HEAD(, pthread_mutex) mutexq; + /* Queue of currently owned priority type mutexs. */ + TAILQ_HEAD(, pthread_mutex) pri_mutexq; + void *ret; struct pthread_specific_elem *specific; int specific_data_count; @@ -531,6 +532,21 @@ ; \ } while (0) +#define THR_UMTX_TRYLOCK(thrd, lck) \ + umtx_trylock((struct umtx *)(lck), (thrd)->tid) + +#define THR_UMTX_LOCK(thrd, lck) \ + UMTX_LOCK((struct umtx *)(lck), (thrd)->tid) \ + +#define THR_UMTX_UNLOCK(thrd, lck) \ + umtx_unlock((struct umtx *)(lck), (thrd)->tid) + +#define THR_UMTX_TIMEDLOCK(thrd, lck, abstime) \ + umtx_timedlock((struct umtx *)(lck), (thrd)->tid, (abstime)) + +#define THR_UMTX_OWNED(thrd, lck) \ + (umtx_owner((struct umtx *)lck) == (thrd)->tid) + /* * Critical regions can also be detected by looking at the threads * current lock level. Ensure these macros increment and decrement @@ -552,13 +568,13 @@ #define THR_LOCK_ACQUIRE(thrd, lck) \ do { \ (thrd)->locklevel++; \ - UMTX_LOCK((lck), (thrd)->tid); \ + UMTX_LOCK((struct umtx *)(lck), (thrd)->tid); \ } while (0) #define THR_LOCK_RELEASE(thrd, lck) \ do { \ if ((thrd)->locklevel > 0) { \ - umtx_unlock((lck), (thrd)->tid); \ + umtx_unlock((struct umtx *)(lck), (thrd)->tid); \ (thrd)->locklevel--; \ THR_CRITICAL_CHECK(thrd); \ } else { \ @@ -641,7 +657,7 @@ SCLASS int _thread_active_threads SCLASS_PRESET(1); SCLASS TAILQ_HEAD(atfork_head, pthread_atfork) _thr_atfork_list; -SCLASS pthread_mutex_t _thr_atfork_mutex; +SCLASS struct umtx _thr_atfork_lock; /* Default thread attributes: */ SCLASS struct pthread_attr _pthread_attr_default @@ -668,9 +684,6 @@ /* Garbage thread count. */ SCLASS int _gc_count SCLASS_PRESET(0); -SCLASS struct umtx _mutex_static_lock; -SCLASS struct umtx _cond_static_lock; -SCLASS struct umtx _rwlock_static_lock; SCLASS struct umtx _keytable_lock; SCLASS struct umtx _thread_list_lock; SCLASS struct umtx _thread_signal_lock; @@ -690,6 +703,7 @@ int _mutex_cv_unlock(pthread_mutex_t *); void _mutex_notify_priochange(struct pthread *, struct pthread *, int); int _mutex_reinit(pthread_mutex_t *); +void _mutex_fork(struct pthread *curthread); void _mutex_unlock_private(struct pthread *); void _libpthread_init(struct pthread *); void *_pthread_getspecific(pthread_key_t);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200501080015.j080Fwkx078032>