From a8af2b6dda18b77d9bad0f0c32076ca066cf57f1 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Tue, 20 May 2008 20:43:28 +0000 Subject: [PATCH] Some work on pthreads: * Made the pthread_cond_t internals public. This is necessary to support process shared condition variables. Fixed initializer macro. * Made the pthread_rwlockattr_t structure opaque. * pthread_t is no longer typedef'ed to int. It's the pointer to the internal _pthread_thread structure. * Removed __get_pthread(). pthread_self() can be used instead. * No longer tunnel the pthread exit value through Haiku's thread exit value. We do have a separate field in the _pthread_thread structure for it, now. * Handle detaching of threads correctly. * pthread_rwlockattr_{g,s}etpshared() use the PTHREAD_PROCESS_{SHARED,PRIVATE} constants, now. * Commented out yet unsupported structures (barriers, spinlocks). * Rebuilt APR optional package. The pthread changes weren't binary compatible. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@25582 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- build/jam/OptionalPackages | 4 +- headers/posix/pthread.h | 27 +++--- src/system/libroot/posix/pthread/pthread.c | 83 +++++++++------- .../libroot/posix/pthread/pthread_cancel.cpp | 6 +- .../libroot/posix/pthread/pthread_cleanup.cpp | 4 +- .../libroot/posix/pthread/pthread_cond.c | 94 +++++++------------ .../libroot/posix/pthread/pthread_key.cpp | 4 +- .../libroot/posix/pthread/pthread_private.h | 23 ++--- .../libroot/posix/pthread/pthread_rwlock.cpp | 32 +++++-- 9 files changed, 143 insertions(+), 134 deletions(-) diff --git a/build/jam/OptionalPackages b/build/jam/OptionalPackages index ac8d6f6152..850d85e431 100644 --- a/build/jam/OptionalPackages +++ b/build/jam/OptionalPackages @@ -27,8 +27,8 @@ if [ IsOptionalHaikuImagePackageAdded APR ] { Echo "No optional package APR available for gcc4" ; } else { local baseURL = http://haiku-files.org/files/optional-packages ; - InstallOptionalHaikuImagePackage apr-0.9.17-gcc2-2008-05-13 - : $(baseURL)/apr-0.9.17-gcc2-2008-05-13.zip + InstallOptionalHaikuImagePackage apr-0.9.17-gcc2-2008-05-20 + : $(baseURL)/apr-0.9.17-gcc2-2008-05-20.zip : ; } diff --git a/headers/posix/pthread.h b/headers/posix/pthread.h index 4c31f56799..3b0ef6cf46 100644 --- a/headers/posix/pthread.h +++ b/headers/posix/pthread.h @@ -10,19 +10,21 @@ #include -typedef int pthread_t; +typedef struct _pthread_thread *pthread_t; typedef struct _pthread_attr *pthread_attr_t; typedef struct _pthread_mutex pthread_mutex_t; typedef struct _pthread_mutexattr *pthread_mutexattr_t; -typedef struct _pthread_cond *pthread_cond_t; +typedef struct _pthread_cond pthread_cond_t; typedef struct _pthread_condattr *pthread_condattr_t; typedef int pthread_key_t; typedef struct _pthread_once pthread_once_t; typedef struct _pthread_rwlock pthread_rwlock_t; -typedef struct _pthread_rwlockattr pthread_rwlockattr_t; +typedef struct _pthread_rwlockattr *pthread_rwlockattr_t; +/* typedef struct _pthread_barrier *pthread_barrier_t; typedef struct _pthread_barrierattr *pthread_barrierattr_t; typedef struct _pthread_spinlock *pthread_spinlock_t; +*/ struct _pthread_mutex { uint32_t flags; @@ -32,6 +34,14 @@ struct _pthread_mutex { int32_t owner_count; }; +struct _pthread_cond { + uint32_t flags; + int32_t sem; + pthread_mutex_t *mutex; + int32_t waiter_count; + int32_t event_counter; +}; + struct _pthread_once { int32_t state; }; @@ -53,10 +63,6 @@ struct _pthread_rwlock { }; }; -struct _pthread_rwlockattr { - uint32_t flags; -}; - enum pthread_mutex_type { PTHREAD_MUTEX_DEFAULT, PTHREAD_MUTEX_NORMAL, @@ -98,8 +104,6 @@ enum pthread_process_shared { #define PTHREAD_PRIO_INHERIT 1 #define PTHREAD_PRIO_PROTECT 2 -/* extern pthread_mutexattr_t pthread_mutexattr_default; */ - /* private structure */ struct __pthread_cleanup_handler { struct __pthread_cleanup_handler *previous; @@ -129,9 +133,8 @@ extern "C" { { PTHREAD_MUTEX_DEFAULT, 0, -42, -1, 0 } #define PTHREAD_RECURSIVE_MUTEX_INITIALIZER \ { PTHREAD_MUTEX_RECURSIVE, 0, -42, -1, 0 } - -extern pthread_cond_t _pthread_cond_static_initializer(void); -#define PTHREAD_COND_INITIALIZER _pthread_cond_static_initializer() +#define PTHREAD_COND_INITIALIZER \ + { 0, -42, NULL, 0, 0 } /* mutex functions */ extern int pthread_mutex_destroy(pthread_mutex_t *mutex); diff --git a/src/system/libroot/posix/pthread/pthread.c b/src/system/libroot/posix/pthread/pthread.c index fa8dd30d46..ef3a5c8780 100644 --- a/src/system/libroot/posix/pthread/pthread.c +++ b/src/system/libroot/posix/pthread/pthread.c @@ -15,6 +15,10 @@ #include +#define THREAD_DETACHED 0x01 +#define THREAD_DEAD 0x02 + + static const pthread_attr pthread_attr_default = { PTHREAD_CREATE_JOINABLE, B_NORMAL_PRIORITY, @@ -22,31 +26,15 @@ static const pthread_attr pthread_attr_default = { }; -static struct pthread_thread sMainThread; +static pthread_thread sMainThread; static int32 sPthreadSlot = -1; static int sConcurrencyLevel; -struct pthread_thread * -__get_pthread(void) -{ - struct pthread_thread *thread; - - if (sPthreadSlot == -1) - return &sMainThread; - - thread = (struct pthread_thread *)tls_get(sPthreadSlot); - if (thread == NULL) - return &sMainThread; - - return thread; -} - - static void pthread_destroy_thread(void *data) { - struct pthread_thread *thread = __get_pthread(); + pthread_thread *thread = pthread_self(); // call cleanup handlers while (true) { @@ -59,14 +47,16 @@ pthread_destroy_thread(void *data) } __pthread_key_call_destructors(thread); - free(thread); + + if (atomic_or(&thread->flags, THREAD_DEAD) & THREAD_DETACHED) + free(thread); } static int32 pthread_thread_entry(thread_func _unused, void *_thread) { - struct pthread_thread *thread = (struct pthread_thread *)_thread; + pthread_thread *thread = (pthread_thread *)_thread; // store thread data in TLS *tls_address(sPthreadSlot) = thread; @@ -86,8 +76,7 @@ pthread_create(pthread_t *_thread, const pthread_attr_t *_attr, void *(*startRoutine)(void*), void *arg) { const pthread_attr *attr = NULL; - struct pthread_thread *thread; - thread_id threadID; + pthread_thread *thread; struct thread_creation_attributes attributes; if (_thread == NULL) @@ -98,16 +87,21 @@ pthread_create(pthread_t *_thread, const pthread_attr_t *_attr, else attr = *_attr; - thread = (struct pthread_thread *)malloc(sizeof(struct pthread_thread)); + thread = (pthread_thread *)malloc(sizeof(pthread_thread)); if (thread == NULL) return B_WOULD_BLOCK; thread->entry = startRoutine; thread->entry_argument = arg; + thread->exit_value = NULL; thread->cancel_state = PTHREAD_CANCEL_ENABLE; thread->cancel_type = PTHREAD_CANCEL_DEFERRED; thread->cancelled = false; thread->cleanup_handlers = NULL; + thread->flags = 0; + + if (attr->detach_state == PTHREAD_CREATE_DETACHED) + thread->flags |= THREAD_DETACHED; if (sPthreadSlot == -1) { // In a clean pthread environment, this is even thread-safe! @@ -122,14 +116,15 @@ pthread_create(pthread_t *_thread, const pthread_attr_t *_attr, attributes.stack_address = NULL; attributes.stack_size = attr->stack_size; - threadID = _kern_spawn_thread(&attributes); - if (threadID < B_OK) { + thread->id = _kern_spawn_thread(&attributes); + if (thread->id < 0) { // stupid error code (EAGAIN) but demanded by POSIX + free(thread); return B_WOULD_BLOCK; } - resume_thread(threadID); - *_thread = threadID; + resume_thread(thread->id); + *_thread = thread; return B_OK; } @@ -138,22 +133,39 @@ pthread_create(pthread_t *_thread, const pthread_attr_t *_attr, pthread_t pthread_self(void) { - return find_thread(NULL); + pthread_thread *thread; + + if (sPthreadSlot == -1) + return &sMainThread; + + thread = (pthread_thread *)tls_get(sPthreadSlot); + if (thread == NULL) + return &sMainThread; + + return thread; } int pthread_equal(pthread_t t1, pthread_t t2) { - return t1 > 0 && t2 > 0 && t1 == t2; + return t1 != NULL && t2 != NULL && t1 == t2; } int -pthread_join(pthread_t thread, void **value_ptr) +pthread_join(pthread_t thread, void **_value) { - status_t error = wait_for_thread(thread, (status_t *)value_ptr); + status_t dummy; + status_t error = wait_for_thread(thread->id, &dummy); if (error == B_BAD_THREAD_ID) return ESRCH; + + if (_value != NULL) + *_value = thread->exit_value; + + if (atomic_or(&thread->flags, THREAD_DETACHED) & THREAD_DEAD) + free(thread); + return error; } @@ -161,14 +173,15 @@ pthread_join(pthread_t thread, void **value_ptr) void pthread_exit(void *value) { - exit_thread((status_t)value); + pthread_self()->exit_value = value; + exit_thread(B_OK); } int pthread_kill(pthread_t thread, int sig) { - status_t err = kill(thread, sig); + status_t err = kill(thread->id, sig); if (err == B_BAD_THREAD_ID) return ESRCH; return err; @@ -178,7 +191,9 @@ pthread_kill(pthread_t thread, int sig) int pthread_detach(pthread_t thread) { - // TODO: currently, all threads are detached in our implementation... + if (atomic_or(&thread->flags, THREAD_DETACHED) & THREAD_DEAD) + free(thread); + return 0; } diff --git a/src/system/libroot/posix/pthread/pthread_cancel.cpp b/src/system/libroot/posix/pthread/pthread_cancel.cpp index 924109033e..b52b9c6ac6 100644 --- a/src/system/libroot/posix/pthread/pthread_cancel.cpp +++ b/src/system/libroot/posix/pthread/pthread_cancel.cpp @@ -24,7 +24,7 @@ pthread_setcancelstate(int state, int *_oldState) if (state != PTHREAD_CANCEL_ENABLE && state != PTHREAD_CANCEL_DISABLE) return EINVAL; - pthread_thread* thread = __get_pthread(); + pthread_thread* thread = pthread_self(); if (thread == NULL) return EINVAL; @@ -42,7 +42,7 @@ pthread_setcanceltype(int type, int *_oldType) if (type != PTHREAD_CANCEL_DEFERRED && type != PTHREAD_CANCEL_ASYNCHRONOUS) return EINVAL; - pthread_thread* thread = __get_pthread(); + pthread_thread* thread = pthread_self(); if (thread == NULL) return EINVAL; @@ -57,7 +57,7 @@ pthread_setcanceltype(int type, int *_oldType) void pthread_testcancel(void) { - pthread_thread* thread = __get_pthread(); + pthread_thread* thread = pthread_self(); if (thread == NULL) return; diff --git a/src/system/libroot/posix/pthread/pthread_cleanup.cpp b/src/system/libroot/posix/pthread/pthread_cleanup.cpp index 36b86c0c8d..c3c0e69cbc 100644 --- a/src/system/libroot/posix/pthread/pthread_cleanup.cpp +++ b/src/system/libroot/posix/pthread/pthread_cleanup.cpp @@ -10,7 +10,7 @@ void __pthread_cleanup_push_handler(__pthread_cleanup_handler* handler) { - pthread_thread* thread = __get_pthread(); + pthread_thread* thread = pthread_self(); if (thread == NULL) return; @@ -22,7 +22,7 @@ __pthread_cleanup_push_handler(__pthread_cleanup_handler* handler) __pthread_cleanup_handler* __pthread_cleanup_pop_handler(void) { - pthread_thread* thread = __get_pthread(); + pthread_thread* thread = pthread_self(); if (thread == NULL) return NULL; diff --git a/src/system/libroot/posix/pthread/pthread_cond.c b/src/system/libroot/posix/pthread/pthread_cond.c index 2a36440eb7..259b6cfe31 100644 --- a/src/system/libroot/posix/pthread/pthread_cond.c +++ b/src/system/libroot/posix/pthread/pthread_cond.c @@ -12,44 +12,33 @@ #include +#define COND_FLAG_SHARED 0x01 + + static const pthread_condattr pthread_condattr_default = { false }; -pthread_cond_t -_pthread_cond_static_initializer(void) -{ - pthread_cond_t cond; - if (pthread_cond_init(&cond, NULL) == B_OK) - return cond; - - return NULL; -} - - int -pthread_cond_init(pthread_cond_t *_cond, const pthread_condattr_t *_attr) +pthread_cond_init(pthread_cond_t *cond, const pthread_condattr_t *_attr) { - pthread_cond *cond; const pthread_condattr *attr = NULL; - if (_cond == NULL) - return B_BAD_VALUE; - - cond = (pthread_cond *)malloc(sizeof(pthread_cond)); if (cond == NULL) - return B_NO_MEMORY; + return B_BAD_VALUE; if (_attr != NULL) attr = *_attr; else attr = &pthread_condattr_default; - // TODO: What about the process_shared attribute? + cond->flags = 0; + if (attr->process_shared) + cond->flags |= COND_FLAG_SHARED; + cond->sem = create_sem(0, "pthread_cond"); if (cond->sem < B_OK) { - free(cond); return B_WOULD_BLOCK; // stupid error code (EAGAIN) but demanded by POSIX } @@ -57,31 +46,25 @@ pthread_cond_init(pthread_cond_t *_cond, const pthread_condattr_t *_attr) cond->mutex = NULL; cond->waiter_count = 0; cond->event_counter = 0; - memcpy(&cond->attr, attr, sizeof(pthread_condattr)); - *_cond = cond; return B_OK; } int -pthread_cond_destroy(pthread_cond_t *_cond) +pthread_cond_destroy(pthread_cond_t *cond) { - pthread_cond *cond; - - if (_cond == NULL || (cond = *_cond) == NULL) + if (cond == NULL) return B_BAD_VALUE; delete_sem(cond->sem); - *_cond = NULL; - free(cond); return B_OK; } static status_t -cond_wait(pthread_cond *cond, pthread_mutex_t *mutex, bigtime_t timeout) +cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex, bigtime_t timeout) { status_t status; int32 event; @@ -94,6 +77,16 @@ cond_wait(pthread_cond *cond, pthread_mutex_t *mutex, bigtime_t timeout) // if this thread does not own the mutex return B_NOT_ALLOWED; + // lazy init + if (cond->sem == -42) { + // Note, this is thread-safe, since another thread would be required to + // hold the same mutex. + sem_id sem = create_sem(0, "pthread_cond"); + if (cond->sem < 0) + return EAGAIN; + cond->sem = sem; + } + if (cond->mutex && cond->mutex != mutex) // POSIX suggests EINVAL (= B_BAD_VALUE) to be returned if // the same condition variable is used with multiple mutexes @@ -102,7 +95,7 @@ cond_wait(pthread_cond *cond, pthread_mutex_t *mutex, bigtime_t timeout) cond->mutex = mutex; cond->waiter_count++; - event = atomic_get(&cond->event_counter); + event = atomic_get((vint32*)&cond->event_counter); pthread_mutex_unlock(mutex); @@ -110,7 +103,8 @@ cond_wait(pthread_cond *cond, pthread_mutex_t *mutex, bigtime_t timeout) status = acquire_sem_etc(cond->sem, 1, timeout == B_INFINITE_TIMEOUT ? 0 : B_ABSOLUTE_REAL_TIME_TIMEOUT, timeout); - } while (status == B_OK && atomic_get(&cond->event_counter) == event); + } while (status == B_OK + && atomic_get((vint32*)&cond->event_counter) == event); pthread_mutex_lock(mutex); @@ -124,31 +118,25 @@ cond_wait(pthread_cond *cond, pthread_mutex_t *mutex, bigtime_t timeout) static status_t -cond_signal(pthread_cond *cond, bool broadcast) +cond_signal(pthread_cond_t *cond, bool broadcast) { if (cond == NULL) return B_BAD_VALUE; - atomic_add(&cond->event_counter, 1); + atomic_add((vint32*)&cond->event_counter, 1); return release_sem_etc(cond->sem, broadcast ? cond->waiter_count : 1, 0); } int -pthread_cond_wait(pthread_cond_t *_cond, pthread_mutex_t *_mutex) +pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *_mutex) { - if (_cond == NULL || _mutex == NULL) - return B_BAD_VALUE; - - if (*_cond == NULL) - pthread_cond_init(_cond, NULL); - - return cond_wait(*_cond, _mutex, B_INFINITE_TIMEOUT); + return cond_wait(cond, _mutex, B_INFINITE_TIMEOUT); } int -pthread_cond_timedwait(pthread_cond_t *_cond, pthread_mutex_t *_mutex, +pthread_cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex, const struct timespec *tv) { bool invalidTime = false; @@ -160,13 +148,7 @@ pthread_cond_timedwait(pthread_cond_t *_cond, pthread_mutex_t *_mutex, else invalidTime = true; - if (_cond == NULL || _mutex == NULL) - return B_BAD_VALUE; - - if (*_cond == NULL) - pthread_cond_init(_cond, NULL); - - status = cond_wait(*_cond, _mutex, timeout); + status = cond_wait(cond, mutex, timeout); if (status != B_OK && invalidTime) { // POSIX requires EINVAL (= B_BAD_VALUE) to be returned // if the timespec structure was invalid @@ -178,21 +160,15 @@ pthread_cond_timedwait(pthread_cond_t *_cond, pthread_mutex_t *_mutex, int -pthread_cond_broadcast(pthread_cond_t *_cond) +pthread_cond_broadcast(pthread_cond_t *cond) { - if (_cond == NULL) - return B_BAD_VALUE; - - return cond_signal(*_cond, true); + return cond_signal(cond, true); } int -pthread_cond_signal(pthread_cond_t *_cond) +pthread_cond_signal(pthread_cond_t *cond) { - if (_cond == NULL) - return B_BAD_VALUE; - - return cond_signal(*_cond, false); + return cond_signal(cond, false); } diff --git a/src/system/libroot/posix/pthread/pthread_key.cpp b/src/system/libroot/posix/pthread/pthread_key.cpp index e607bd39ec..81e222e232 100644 --- a/src/system/libroot/posix/pthread/pthread_key.cpp +++ b/src/system/libroot/posix/pthread/pthread_key.cpp @@ -117,7 +117,7 @@ pthread_key_delete(pthread_key_t key) void* pthread_getspecific(pthread_key_t key) { - pthread_thread* thread = __get_pthread(); + pthread_thread* thread = pthread_self(); if (key < 0 || key >= PTHREAD_KEYS_MAX) return NULL; @@ -142,7 +142,7 @@ pthread_setspecific(pthread_key_t key, const void* value) if (sequence == PTHREAD_UNUSED_SEQUENCE) return EINVAL; - pthread_key_data& keyData = __get_pthread()->specific[key]; + pthread_key_data& keyData = pthread_self()->specific[key]; keyData.sequence = sequence; keyData.value = const_cast(value); return 0; diff --git a/src/system/libroot/posix/pthread/pthread_private.h b/src/system/libroot/posix/pthread/pthread_private.h index a636441ffd..49be81c7a4 100644 --- a/src/system/libroot/posix/pthread/pthread_private.h +++ b/src/system/libroot/posix/pthread/pthread_private.h @@ -20,14 +20,6 @@ typedef struct _pthread_condattr { bool process_shared; } pthread_condattr; -typedef struct _pthread_cond { - sem_id sem; - pthread_mutex_t *mutex; - int32 waiter_count; - int32 event_counter; - pthread_condattr attr; -} pthread_cond; - typedef struct _pthread_mutexattr { int32 type; bool process_shared; @@ -39,6 +31,10 @@ typedef struct _pthread_attr { size_t stack_size; } pthread_attr; +typedef struct _pthread_rwlockattr { + uint32_t flags; +} pthread_rwlockattr; + typedef void (*pthread_key_destructor)(void *data); struct pthread_key { @@ -54,25 +50,26 @@ struct pthread_key_data { #define PTHREAD_KEYS_MAX 256 #define PTHREAD_UNUSED_SEQUENCE 0 -// This structure is used internally only, it has no public equivalent -struct pthread_thread { +typedef struct _pthread_thread { + thread_id id; + int32 flags; void *(*entry)(void*); void *entry_argument; + void *exit_value; int cancel_state; int cancel_type; bool cancelled; struct pthread_key_data specific[PTHREAD_KEYS_MAX]; struct __pthread_cleanup_handler *cleanup_handlers; // TODO: move pthread keys in here, too -}; +} pthread_thread; #ifdef __cplusplus extern "C" { #endif -void __pthread_key_call_destructors(struct pthread_thread *thread); -struct pthread_thread *__get_pthread(void); +void __pthread_key_call_destructors(pthread_thread *thread); #ifdef __cplusplus } diff --git a/src/system/libroot/posix/pthread/pthread_rwlock.cpp b/src/system/libroot/posix/pthread/pthread_rwlock.cpp index cffa855e6b..7868fa546c 100644 --- a/src/system/libroot/posix/pthread/pthread_rwlock.cpp +++ b/src/system/libroot/posix/pthread/pthread_rwlock.cpp @@ -15,6 +15,8 @@ #include #include +#include "pthread_private.h" + #define MAX_READER_COUNT 1000000 @@ -285,8 +287,9 @@ assert_dummy() int -pthread_rwlock_init(pthread_rwlock_t* lock, const pthread_rwlockattr_t* attr) +pthread_rwlock_init(pthread_rwlock_t* lock, const pthread_rwlockattr_t* _attr) { + pthread_rwlockattr* attr = _attr != NULL ? *_attr : NULL; bool shared = attr != NULL && (attr->flags & RWLOCK_FLAG_SHARED) != 0; if (shared) @@ -399,32 +402,47 @@ pthread_rwlock_unlock(pthread_rwlock_t* lock) int -pthread_rwlockattr_init(pthread_rwlockattr_t* attr) +pthread_rwlockattr_init(pthread_rwlockattr_t* _attr) { + pthread_rwlockattr* attr = (pthread_rwlockattr*)malloc( + sizeof(pthread_rwlockattr)); + if (attr == NULL) + return B_NO_MEMORY; + attr->flags = 0; + *_attr = attr; + return 0; } int -pthread_rwlockattr_destroy(pthread_rwlockattr_t* attr) +pthread_rwlockattr_destroy(pthread_rwlockattr_t* _attr) { + pthread_rwlockattr* attr = *_attr; + + free(attr); return 0; } int -pthread_rwlockattr_getpshared(const pthread_rwlockattr_t* attr, int* shared) +pthread_rwlockattr_getpshared(const pthread_rwlockattr_t* _attr, int* shared) { - *shared = (attr->flags & RWLOCK_FLAG_SHARED) != 0; + pthread_rwlockattr* attr = *_attr; + + *shared = (attr->flags & RWLOCK_FLAG_SHARED) != 0 + ? PTHREAD_PROCESS_SHARED : PTHREAD_PROCESS_PRIVATE; return 0; } int -pthread_rwlockattr_setpshared(pthread_rwlockattr_t* attr, int shared) +pthread_rwlockattr_setpshared(pthread_rwlockattr_t* _attr, int shared) { - if (shared) + pthread_rwlockattr* attr = *_attr; + + if (shared == PTHREAD_PROCESS_SHARED) attr->flags |= RWLOCK_FLAG_SHARED; else attr->flags &= ~RWLOCK_FLAG_SHARED;