d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

1. Move thread list flags into new separate member, and atomically

Browse Source 
put DEAD thread on GC list, this closes a race between pthread_join
   and thr_cleanup.
2. Introduce a mutex to protect tcb initialization, tls allocation and
   deallocation code in rtld seems no lock protection or it is broken,
   under stress testing, memory is corrupted.

Reviewed by: deischen
patch partly provided by: deischen
This commit is contained in:
David Xu 2004-10-23 23:28:36 +00:00
parent 1be7387c03
commit b4f9f84b96
10 changed files with 90 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
lib/libkse/thread/thr_create.c
View File

@ -234,6 +234,7 @@ _pthread_create(pthread_t * thread, const pthread_attr_t * attr,
new_thread->specific_data_count = 0;
new_thread->cleanup = NULL;
new_thread->flags = 0;
new_thread->tlflags = 0;
new_thread->continuation = NULL;
new_thread->wakeup_time.tv_sec = -1;
new_thread->lock_switch = 0;

2
lib/libkse/thread/thr_find_thread.c
View File

@ -90,7 +90,7 @@ _thr_ref_delete(struct pthread *curthread, struct pthread *thread)
if (curthread != NULL)
curthread->critical_count--;
if ((thread->refcount == 0) &&
(thread->flags & THR_FLAGS_GC_SAFE) != 0)
(thread->tlflags & TLFLAGS_GC_SAFE) != 0)
THR_GCLIST_ADD(thread);
KSE_LOCK_RELEASE(curkse, &_thread_list_lock);
_kse_critical_leave(crit);

37
lib/libkse/thread/thr_kern.c
View File

@ -139,6 +139,9 @@ LIST_HEAD(thread_hash_head, pthread);
static struct thread_hash_head thr_hashtable[THREAD_HASH_QUEUES];
#define THREAD_HASH(thrd) ((unsigned long)thrd % THREAD_HASH_QUEUES)
/* Lock for thread tcb constructor/destructor */
static pthread_mutex_t _tcb_mutex;
#ifdef DEBUG_THREAD_KERN
static void dump_queues(struct kse *curkse);
#endif
@ -166,7 +169,7 @@ static void thr_resume_check(struct pthread *curthread, ucontext_t *ucp,
struct pthread_sigframe *psf);
static int thr_timedout(struct pthread *thread, struct timespec *curtime);
static void thr_unlink(struct pthread *thread);
static void thr_destroy(struct pthread *thread);
static void thr_destroy(struct pthread *curthread, struct pthread *thread);
static void thread_gc(struct pthread *thread);
static void kse_gc(struct pthread *thread);
static void kseg_gc(struct pthread *thread);
@ -240,7 +243,7 @@ _kse_single_thread(struct pthread *curthread)
_thr_stack_free(&thread->attr);
if (thread->specific != NULL)
free(thread->specific);
thr_destroy(thread);
thr_destroy(curthread, thread);
}
}
@ -285,14 +288,14 @@ _kse_single_thread(struct pthread *curthread)
/* Free the free threads. */
while ((thread = TAILQ_FIRST(&free_threadq)) != NULL) {
TAILQ_REMOVE(&free_threadq, thread, tle);
thr_destroy(thread);
thr_destroy(curthread, thread);
}
free_thread_count = 0;
/* Free the to-be-gc'd threads. */
while ((thread = TAILQ_FIRST(&_thread_gc_list)) != NULL) {
TAILQ_REMOVE(&_thread_gc_list, thread, gcle);
thr_destroy(thread);
thr_destroy(curthread, thread);
}
TAILQ_INIT(&gc_ksegq);
_gc_count = 0;
@ -381,6 +384,7 @@ _kse_init(void)
if (_lock_init(&_thread_list_lock, LCK_ADAPTIVE,
_kse_lock_wait, _kse_lock_wakeup) != 0)
PANIC("Unable to initialize thread list lock");
_pthread_mutex_init(&_tcb_mutex, NULL);
active_kse_count = 0;
active_kseg_count = 0;
_gc_count = 0;
@ -1207,7 +1211,6 @@ thr_cleanup(struct kse *curkse, struct pthread *thread)
thread->kseg = _kse_initial->k_kseg;
thread->kse = _kse_initial;
}
thread->flags |= THR_FLAGS_GC_SAFE;
/*
* We can't hold the thread list lock while holding the
@ -1216,6 +1219,7 @@ thr_cleanup(struct kse *curkse, struct pthread *thread)
KSE_SCHED_UNLOCK(curkse, curkse->k_kseg);
DBG_MSG("Adding thread %p to GC list\n", thread);
KSE_LOCK_ACQUIRE(curkse, &_thread_list_lock);
thread->tlflags |= TLFLAGS_GC_SAFE;
THR_GCLIST_ADD(thread);
KSE_LOCK_RELEASE(curkse, &_thread_list_lock);
if (sys_scope) {
@ -1255,7 +1259,7 @@ thread_gc(struct pthread *curthread)
/* Check the threads waiting for GC. */
for (td = TAILQ_FIRST(&_thread_gc_list); td != NULL; td = td_next) {
td_next = TAILQ_NEXT(td, gcle);
if ((td->flags & THR_FLAGS_GC_SAFE) == 0)
if ((td->tlflags & TLFLAGS_GC_SAFE) == 0)
continue;
else if (((td->attr.flags & PTHREAD_SCOPE_SYSTEM) != 0) &&
((td->kse->k_kcb->kcb_kmbx.km_flags & KMF_DONE) == 0)) {
@ -2384,7 +2388,14 @@ _thr_alloc(struct pthread *curthread)
if ((thread == NULL) &&
((thread = malloc(sizeof(struct pthread))) != NULL)) {
bzero(thread, sizeof(struct pthread));
if ((thread->tcb = _tcb_ctor(thread, curthread == NULL)) == NULL) {
if (curthread) {
_pthread_mutex_lock(&_tcb_mutex);
thread->tcb = _tcb_ctor(thread, 0 /* not initial tls */);
_pthread_mutex_unlock(&_tcb_mutex);
} else {
thread->tcb = _tcb_ctor(thread, 1 /* initial tls */);
}
if (thread->tcb == NULL) {
free(thread);
thread = NULL;
} else {
@ -2420,7 +2431,7 @@ _thr_free(struct pthread *curthread, struct pthread *thread)
thread->name = NULL;
}
if ((curthread == NULL) || (free_thread_count >= MAX_CACHED_THREADS)) {
thr_destroy(thread);
thr_destroy(curthread, thread);
} else {
/* Add the thread to the free thread list. */
crit = _kse_critical_enter();
@ -2433,14 +2444,20 @@ _thr_free(struct pthread *curthread, struct pthread *thread)
}
static void
thr_destroy(struct pthread *thread)
thr_destroy(struct pthread *curthread, struct pthread *thread)
{
int i;
for (i = 0; i < MAX_THR_LOCKLEVEL; i++)
_lockuser_destroy(&thread->lockusers[i]);
_lock_destroy(&thread->lock);
_tcb_dtor(thread->tcb);
if (curthread) {
_pthread_mutex_lock(&_tcb_mutex);
_tcb_dtor(thread->tcb);
_pthread_mutex_unlock(&_tcb_mutex);
} else {
_tcb_dtor(thread->tcb);
}
free(thread->siginfo);
free(thread);
}

26
lib/libkse/thread/thr_private.h
View File

@ -753,9 +753,13 @@ struct pthread {
#define THR_FLAGS_IN_RUNQ 0x0004 /* in run queue using pqe link */
#define THR_FLAGS_EXITING 0x0008 /* thread is exiting */
#define THR_FLAGS_SUSPENDED 0x0010 /* thread is suspended */
#define THR_FLAGS_GC_SAFE 0x0020 /* thread safe for cleaning */
#define THR_FLAGS_IN_TDLIST 0x0040 /* thread in all thread list */
#define THR_FLAGS_IN_GCLIST 0x0080 /* thread in gc list */
/* Thread list flags; only set with thread list lock held. */
#define TLFLAGS_GC_SAFE 0x0001 /* thread safe for cleaning */
#define TLFLAGS_IN_TDLIST 0x0002 /* thread in all thread list */
#define TLFLAGS_IN_GCLIST 0x0004 /* thread in gc list */
int tlflags;
/*
* Base priority is the user setable and retrievable priority
* of the thread. It is only affected by explicit calls to
@ -897,30 +901,30 @@ do { \
* the gc list.
*/
#define THR_LIST_ADD(thrd) do { \
if (((thrd)->flags & THR_FLAGS_IN_TDLIST) == 0) { \
if (((thrd)->tlflags & TLFLAGS_IN_TDLIST) == 0) { \
TAILQ_INSERT_HEAD(&_thread_list, thrd, tle); \
_thr_hash_add(thrd); \
(thrd)->flags |= THR_FLAGS_IN_TDLIST; \
(thrd)->tlflags |= TLFLAGS_IN_TDLIST; \
} \
} while (0)
#define THR_LIST_REMOVE(thrd) do { \
if (((thrd)->flags & THR_FLAGS_IN_TDLIST) != 0) { \
if (((thrd)->tlflags & TLFLAGS_IN_TDLIST) != 0) { \
TAILQ_REMOVE(&_thread_list, thrd, tle); \
_thr_hash_remove(thrd); \
(thrd)->flags &= ~THR_FLAGS_IN_TDLIST; \
(thrd)->tlflags &= ~TLFLAGS_IN_TDLIST; \
} \
} while (0)
#define THR_GCLIST_ADD(thrd) do { \
if (((thrd)->flags & THR_FLAGS_IN_GCLIST) == 0) { \
if (((thrd)->tlflags & TLFLAGS_IN_GCLIST) == 0) { \
TAILQ_INSERT_HEAD(&_thread_gc_list, thrd, gcle);\
(thrd)->flags |= THR_FLAGS_IN_GCLIST; \
(thrd)->tlflags |= TLFLAGS_IN_GCLIST; \
_gc_count++; \
} \
} while (0)
#define THR_GCLIST_REMOVE(thrd) do { \
if (((thrd)->flags & THR_FLAGS_IN_GCLIST) != 0) { \
if (((thrd)->tlflags & TLFLAGS_IN_GCLIST) != 0) { \
TAILQ_REMOVE(&_thread_gc_list, thrd, gcle); \
(thrd)->flags &= ~THR_FLAGS_IN_GCLIST; \
(thrd)->tlflags &= ~TLFLAGS_IN_GCLIST; \
_gc_count--; \
} \
} while (0)

3
lib/libkse/thread/thr_sig.c
View File

@ -1199,8 +1199,7 @@ static void
thr_sigframe_save(struct pthread *thread, struct pthread_sigframe *psf)
{
/* This has to initialize all members of the sigframe. */
psf->psf_flags =
thread->flags & (THR_FLAGS_PRIVATE | THR_FLAGS_IN_TDLIST);
psf->psf_flags = thread->flags & THR_FLAGS_PRIVATE;
psf->psf_interrupted = thread->interrupted;
psf->psf_timeout = thread->timeout;
psf->psf_state = thread->state;

1
lib/libpthread/thread/thr_create.c
View File

@ -234,6 +234,7 @@ _pthread_create(pthread_t * thread, const pthread_attr_t * attr,
new_thread->specific_data_count = 0;
new_thread->cleanup = NULL;
new_thread->flags = 0;
new_thread->tlflags = 0;
new_thread->continuation = NULL;
new_thread->wakeup_time.tv_sec = -1;
new_thread->lock_switch = 0;

2
lib/libpthread/thread/thr_find_thread.c
View File

@ -90,7 +90,7 @@ _thr_ref_delete(struct pthread *curthread, struct pthread *thread)
if (curthread != NULL)
curthread->critical_count--;
if ((thread->refcount == 0) &&
(thread->flags & THR_FLAGS_GC_SAFE) != 0)
(thread->tlflags & TLFLAGS_GC_SAFE) != 0)
THR_GCLIST_ADD(thread);
KSE_LOCK_RELEASE(curkse, &_thread_list_lock);
_kse_critical_leave(crit);

37
lib/libpthread/thread/thr_kern.c
View File

@ -139,6 +139,9 @@ LIST_HEAD(thread_hash_head, pthread);
static struct thread_hash_head thr_hashtable[THREAD_HASH_QUEUES];
#define THREAD_HASH(thrd) ((unsigned long)thrd % THREAD_HASH_QUEUES)
/* Lock for thread tcb constructor/destructor */
static pthread_mutex_t _tcb_mutex;
#ifdef DEBUG_THREAD_KERN
static void dump_queues(struct kse *curkse);
#endif
@ -166,7 +169,7 @@ static void thr_resume_check(struct pthread *curthread, ucontext_t *ucp,
struct pthread_sigframe *psf);
static int thr_timedout(struct pthread *thread, struct timespec *curtime);
static void thr_unlink(struct pthread *thread);
static void thr_destroy(struct pthread *thread);
static void thr_destroy(struct pthread *curthread, struct pthread *thread);
static void thread_gc(struct pthread *thread);
static void kse_gc(struct pthread *thread);
static void kseg_gc(struct pthread *thread);
@ -240,7 +243,7 @@ _kse_single_thread(struct pthread *curthread)
_thr_stack_free(&thread->attr);
if (thread->specific != NULL)
free(thread->specific);
thr_destroy(thread);
thr_destroy(curthread, thread);
}
}
@ -285,14 +288,14 @@ _kse_single_thread(struct pthread *curthread)
/* Free the free threads. */
while ((thread = TAILQ_FIRST(&free_threadq)) != NULL) {
TAILQ_REMOVE(&free_threadq, thread, tle);
thr_destroy(thread);
thr_destroy(curthread, thread);
}
free_thread_count = 0;
/* Free the to-be-gc'd threads. */
while ((thread = TAILQ_FIRST(&_thread_gc_list)) != NULL) {
TAILQ_REMOVE(&_thread_gc_list, thread, gcle);
thr_destroy(thread);
thr_destroy(curthread, thread);
}
TAILQ_INIT(&gc_ksegq);
_gc_count = 0;
@ -381,6 +384,7 @@ _kse_init(void)
if (_lock_init(&_thread_list_lock, LCK_ADAPTIVE,
_kse_lock_wait, _kse_lock_wakeup) != 0)
PANIC("Unable to initialize thread list lock");
_pthread_mutex_init(&_tcb_mutex, NULL);
active_kse_count = 0;
active_kseg_count = 0;
_gc_count = 0;
@ -1207,7 +1211,6 @@ thr_cleanup(struct kse *curkse, struct pthread *thread)
thread->kseg = _kse_initial->k_kseg;
thread->kse = _kse_initial;
}
thread->flags |= THR_FLAGS_GC_SAFE;
/*
* We can't hold the thread list lock while holding the
@ -1216,6 +1219,7 @@ thr_cleanup(struct kse *curkse, struct pthread *thread)
KSE_SCHED_UNLOCK(curkse, curkse->k_kseg);
DBG_MSG("Adding thread %p to GC list\n", thread);
KSE_LOCK_ACQUIRE(curkse, &_thread_list_lock);
thread->tlflags |= TLFLAGS_GC_SAFE;
THR_GCLIST_ADD(thread);
KSE_LOCK_RELEASE(curkse, &_thread_list_lock);
if (sys_scope) {
@ -1255,7 +1259,7 @@ thread_gc(struct pthread *curthread)
/* Check the threads waiting for GC. */
for (td = TAILQ_FIRST(&_thread_gc_list); td != NULL; td = td_next) {
td_next = TAILQ_NEXT(td, gcle);
if ((td->flags & THR_FLAGS_GC_SAFE) == 0)
if ((td->tlflags & TLFLAGS_GC_SAFE) == 0)
continue;
else if (((td->attr.flags & PTHREAD_SCOPE_SYSTEM) != 0) &&
((td->kse->k_kcb->kcb_kmbx.km_flags & KMF_DONE) == 0)) {
@ -2384,7 +2388,14 @@ _thr_alloc(struct pthread *curthread)
if ((thread == NULL) &&
((thread = malloc(sizeof(struct pthread))) != NULL)) {
bzero(thread, sizeof(struct pthread));
if ((thread->tcb = _tcb_ctor(thread, curthread == NULL)) == NULL) {
if (curthread) {
_pthread_mutex_lock(&_tcb_mutex);
thread->tcb = _tcb_ctor(thread, 0 /* not initial tls */);
_pthread_mutex_unlock(&_tcb_mutex);
} else {
thread->tcb = _tcb_ctor(thread, 1 /* initial tls */);
}
if (thread->tcb == NULL) {
free(thread);
thread = NULL;
} else {
@ -2420,7 +2431,7 @@ _thr_free(struct pthread *curthread, struct pthread *thread)
thread->name = NULL;
}
if ((curthread == NULL) || (free_thread_count >= MAX_CACHED_THREADS)) {
thr_destroy(thread);
thr_destroy(curthread, thread);
} else {
/* Add the thread to the free thread list. */
crit = _kse_critical_enter();
@ -2433,14 +2444,20 @@ _thr_free(struct pthread *curthread, struct pthread *thread)
}
static void
thr_destroy(struct pthread *thread)
thr_destroy(struct pthread *curthread, struct pthread *thread)
{
int i;
for (i = 0; i < MAX_THR_LOCKLEVEL; i++)
_lockuser_destroy(&thread->lockusers[i]);
_lock_destroy(&thread->lock);
_tcb_dtor(thread->tcb);
if (curthread) {
_pthread_mutex_lock(&_tcb_mutex);
_tcb_dtor(thread->tcb);
_pthread_mutex_unlock(&_tcb_mutex);
} else {
_tcb_dtor(thread->tcb);
}
free(thread->siginfo);
free(thread);
}

26
lib/libpthread/thread/thr_private.h
View File

@ -753,9 +753,13 @@ struct pthread {
#define THR_FLAGS_IN_RUNQ 0x0004 /* in run queue using pqe link */
#define THR_FLAGS_EXITING 0x0008 /* thread is exiting */
#define THR_FLAGS_SUSPENDED 0x0010 /* thread is suspended */
#define THR_FLAGS_GC_SAFE 0x0020 /* thread safe for cleaning */
#define THR_FLAGS_IN_TDLIST 0x0040 /* thread in all thread list */
#define THR_FLAGS_IN_GCLIST 0x0080 /* thread in gc list */
/* Thread list flags; only set with thread list lock held. */
#define TLFLAGS_GC_SAFE 0x0001 /* thread safe for cleaning */
#define TLFLAGS_IN_TDLIST 0x0002 /* thread in all thread list */
#define TLFLAGS_IN_GCLIST 0x0004 /* thread in gc list */
int tlflags;
/*
* Base priority is the user setable and retrievable priority
* of the thread. It is only affected by explicit calls to
@ -897,30 +901,30 @@ do { \
* the gc list.
*/
#define THR_LIST_ADD(thrd) do { \
if (((thrd)->flags & THR_FLAGS_IN_TDLIST) == 0) { \
if (((thrd)->tlflags & TLFLAGS_IN_TDLIST) == 0) { \
TAILQ_INSERT_HEAD(&_thread_list, thrd, tle); \
_thr_hash_add(thrd); \
(thrd)->flags |= THR_FLAGS_IN_TDLIST; \
(thrd)->tlflags |= TLFLAGS_IN_TDLIST; \
} \
} while (0)
#define THR_LIST_REMOVE(thrd) do { \
if (((thrd)->flags & THR_FLAGS_IN_TDLIST) != 0) { \
if (((thrd)->tlflags & TLFLAGS_IN_TDLIST) != 0) { \
TAILQ_REMOVE(&_thread_list, thrd, tle); \
_thr_hash_remove(thrd); \
(thrd)->flags &= ~THR_FLAGS_IN_TDLIST; \
(thrd)->tlflags &= ~TLFLAGS_IN_TDLIST; \
} \
} while (0)
#define THR_GCLIST_ADD(thrd) do { \
if (((thrd)->flags & THR_FLAGS_IN_GCLIST) == 0) { \
if (((thrd)->tlflags & TLFLAGS_IN_GCLIST) == 0) { \
TAILQ_INSERT_HEAD(&_thread_gc_list, thrd, gcle);\
(thrd)->flags |= THR_FLAGS_IN_GCLIST; \
(thrd)->tlflags |= TLFLAGS_IN_GCLIST; \
_gc_count++; \
} \
} while (0)
#define THR_GCLIST_REMOVE(thrd) do { \
if (((thrd)->flags & THR_FLAGS_IN_GCLIST) != 0) { \
if (((thrd)->tlflags & TLFLAGS_IN_GCLIST) != 0) { \
TAILQ_REMOVE(&_thread_gc_list, thrd, gcle); \
(thrd)->flags &= ~THR_FLAGS_IN_GCLIST; \
(thrd)->tlflags &= ~TLFLAGS_IN_GCLIST; \
_gc_count--; \
} \
} while (0)

3
lib/libpthread/thread/thr_sig.c
View File

@ -1199,8 +1199,7 @@ static void
thr_sigframe_save(struct pthread *thread, struct pthread_sigframe *psf)
{
/* This has to initialize all members of the sigframe. */
psf->psf_flags =
thread->flags & (THR_FLAGS_PRIVATE | THR_FLAGS_IN_TDLIST);
psf->psf_flags = thread->flags & THR_FLAGS_PRIVATE;
psf->psf_interrupted = thread->interrupted;
psf->psf_timeout = thread->timeout;
psf->psf_state = thread->state;