* Reimplemented pthread_once. The old one was neither thread-safe nor

particularly efficient.
* pthread_mutex implementation:
  - Removed the pthread_mutex_t indirection (the type was a pointer to
    the actual structure which was allocated on the heap), as it made
    sharing the mutex between processes impossible.
  - Removed the distinction between process shared and non-shared
    mutexes. Benaphores work just as well in shared memory, so we always
    use them.
* Fixed some static initializer macros. PTHREAD_COND_INITIALIZER is
  still broken, since it doesn't work in C code.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@25481 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/posix/pthread.h

@@ -12,7 +12,7 @@
 
 typedef int							pthread_t;
 typedef struct  _pthread_attr		*pthread_attr_t;
-typedef struct  _pthread_mutex		*pthread_mutex_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_condattr	*pthread_condattr_t;
@@ -24,9 +24,16 @@ 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;
+	int32_t		count;
+	int32_t		sem;
+	int32_t		owner;
+	int32_t		owner_count;
+};
+
 struct _pthread_once {
-	int	state;
+	int32_t		state;
-	pthread_mutex_t mutex;
 };
 
 struct _pthread_rwlock {
@@ -84,9 +91,7 @@ enum pthread_process_shared {
 #define PTHREAD_CANCEL_ASYNCHRONOUS	2
 #define PTHREAD_CANCELED			((void *) 1)
 
-#define PTHREAD_NEEDS_INIT			0
+#define PTHREAD_ONCE_INIT 			{ -1 }
-#define PTHREAD_DONE_INIT			1
-#define PTHREAD_ONCE_INIT 			{ PTHREAD_NEEDS_INIT, NULL }
 
 #define PTHREAD_BARRIER_SERIAL_THREAD -1
 #define PTHREAD_PRIO_NONE			0
@@ -120,14 +125,13 @@ struct __pthread_cleanup_handler {
 extern "C" {
 #endif
 
-extern pthread_mutex_t _pthread_mutex_static_initializer(void);
+#define PTHREAD_MUTEX_INITIALIZER \
-extern pthread_mutex_t _pthread_recursive_mutex_static_initializer(void);
+	{ PTHREAD_MUTEX_DEFAULT, 0, -42, -1, 0 }
-#define PTHREAD_MUTEX_INITIALIZER NULL
 #define PTHREAD_RECURSIVE_MUTEX_INITIALIZER \
-	pthread_recursive_mutex_static_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 _pthread_cond_static_initializer()
 
 /* mutex functions */
 extern int pthread_mutex_destroy(pthread_mutex_t *mutex);
@@ -205,7 +209,8 @@ extern int pthread_rwlockattr_setpshared(pthread_rwlockattr_t *attr,
 /* misc. functions */
 extern int pthread_atfork(void (*prepare)(void), void (*parent)(void),
 	void (*child)(void));
-extern int pthread_once(pthread_once_t *once_control, void (*init_routine)());
+extern int pthread_once(pthread_once_t *once_control,
+	void (*init_routine)(void));
 
 /* thread attributes functions */
 extern int pthread_attr_destroy(pthread_attr_t *attr);

src/system/libroot/posix/pthread/Jamfile

@@ -14,7 +14,7 @@ MergeObject posix_pthread.o :
 	pthread_key.cpp
 	pthread_mutex.c
 	pthread_mutexattr.c
-	pthread_once.c
+	pthread_once.cpp
 	pthread_rwlock.cpp
 ;
 

src/system/libroot/posix/pthread/pthread_cond.c

@@ -81,30 +81,30 @@ pthread_cond_destroy(pthread_cond_t *_cond)
 
 
 static status_t
-cond_wait(pthread_cond *cond, pthread_mutex_t *_mutex, bigtime_t timeout)
+cond_wait(pthread_cond *cond, pthread_mutex_t *mutex, bigtime_t timeout)
 {
 	status_t status;
 	int32 event;
 
-	if (cond == NULL || *_mutex == NULL)
+	if (cond == NULL || mutex == NULL)
 		return B_BAD_VALUE;
 
-	if ((*_mutex)->owner != find_thread(NULL))
+	if (mutex->owner != find_thread(NULL))
 		// POSIX suggests EPERM (= B_NOT_ALLOWED) to be returned
 		// if this thread does not own the mutex
 		return B_NOT_ALLOWED;
 
-	if (cond->mutex && cond->mutex != _mutex)
+	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
 		return B_BAD_VALUE;
 
-	cond->mutex = _mutex;
+	cond->mutex = mutex;
 	cond->waiter_count++;
 
 	event = atomic_get(&cond->event_counter);
 
-	pthread_mutex_unlock(_mutex);
+	pthread_mutex_unlock(mutex);
 
 	do {
 		status = acquire_sem_etc(cond->sem, 1,
@@ -112,7 +112,7 @@ cond_wait(pthread_cond *cond, pthread_mutex_t *_mutex, bigtime_t timeout)
 			timeout);
 	} while (status == B_OK && atomic_get(&cond->event_counter) == event);
 
-	pthread_mutex_lock(_mutex);
+	pthread_mutex_lock(mutex);
 
 	cond->waiter_count--;
 	// If there are no more waiters, we can change mutexes

src/system/libroot/posix/pthread/pthread_mutex.c

@@ -12,61 +12,32 @@
 #include <string.h>
 
 
+#define MUTEX_FLAG_SHARED	0x80000000
+#define MUTEX_TYPE_BITS		0x0000000f
+#define MUTEX_TYPE(mutex)	((mutex)->flags & MUTEX_TYPE_BITS)
+
+
 static const pthread_mutexattr pthread_mutexattr_default = {
 	PTHREAD_MUTEX_DEFAULT,
 	false
 };
 
 
-pthread_mutex_t
-_pthread_recursive_mutex_static_initializer(void)
-{
-	pthread_mutex_t mutex;
-	pthread_mutexattr attr;
-	pthread_mutexattr_t attrPointer = &attr;
-
-	attr.type = PTHREAD_MUTEX_RECURSIVE;
-	attr.process_shared = false;
-
-	if (pthread_mutex_init(&mutex, &attrPointer) == B_OK)
-		return mutex;
-
-	return NULL;
-}
-
-
-pthread_mutex_t 
-_pthread_mutex_static_initializer(void)
-{
-	pthread_mutex_t mutex;
-	if (pthread_mutex_init(&mutex, NULL) == B_OK)
-		return mutex;
-
-	return NULL;
-}
-
-
 int
-pthread_mutex_init(pthread_mutex_t *_mutex, const pthread_mutexattr_t *_attr)
+pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *_attr)
 {
-	pthread_mutex *mutex;
 	const pthread_mutexattr *attr = NULL;
 
-	if (_mutex == NULL)
-		return B_BAD_VALUE;
-
-	mutex = (pthread_mutex *)malloc(sizeof(pthread_mutex));
 	if (mutex == NULL)
-		return B_NO_MEMORY;
+		return B_BAD_VALUE;
 
 	if (_attr != NULL)
 		attr = *_attr;
 	else
 		attr = &pthread_mutexattr_default;
 
-	mutex->sem = create_sem(attr && attr->process_shared ? 0 : 1, "pthread_mutex");
+	mutex->sem = create_sem(0, "pthread_mutex");
 	if (mutex->sem < B_OK) {
-		free(mutex);
 		return B_WOULD_BLOCK;
 			// stupid error code (EAGAIN) but demanded by POSIX
 	}
@@ -74,58 +45,26 @@ pthread_mutex_init(pthread_mutex_t *_mutex, const pthread_mutexattr_t *_attr)
 	mutex->count = 0;
 	mutex->owner = -1;
 	mutex->owner_count = 0;
-	memcpy(&mutex->attr, attr, sizeof(pthread_mutexattr));	
+	mutex->flags = attr->type | (attr->process_shared ? MUTEX_FLAG_SHARED : 0);
 
-	*_mutex = mutex;
 	return B_OK;
 }
 
 
 int
-pthread_mutex_destroy(pthread_mutex_t *_mutex)
+pthread_mutex_destroy(pthread_mutex_t *mutex)
-{
-	pthread_mutex *mutex;
-
-	if (_mutex == NULL || (mutex = *_mutex) == NULL)
-		return B_BAD_VALUE;
-
-	delete_sem(mutex->sem);
-	*_mutex = NULL;
-	free(mutex);
-
-	return B_OK;
-}
-
-
-static status_t
-mutex_unlock(pthread_mutex *mutex)
 {
 	if (mutex == NULL)
 		return B_BAD_VALUE;
 
-	if (mutex->owner != find_thread(NULL)) {
+	delete_sem(mutex->sem);
-		// this is a bug in the calling application!
-		// ToDo: should we handle it in another way?
-		fprintf(stderr, "mutex unlocked from foreign thread!\n");
-	}
-
-	if (mutex->attr.type == PTHREAD_MUTEX_RECURSIVE) {
-
-		if (mutex->owner_count-- > 1)
-			return B_OK;
-
-		mutex->owner = -1;
-	}
-
-	if (!mutex->attr.process_shared || atomic_add(&mutex->count, -1) > 1)
-		return release_sem(mutex->sem);
 
 	return B_OK;
 }
 
 
 static status_t
-mutex_lock(pthread_mutex *mutex, bigtime_t timeout)
+mutex_lock(pthread_mutex_t *mutex, bigtime_t timeout)
 {
 	thread_id thisThread = find_thread(NULL);
 	status_t status = B_OK;
@@ -133,18 +72,30 @@ mutex_lock(pthread_mutex *mutex, bigtime_t timeout)
 	if (mutex == NULL)
 		return B_BAD_VALUE;
 
-	if (mutex->attr.type == PTHREAD_MUTEX_ERRORCHECK && mutex->owner == thisThread) {
+	// If statically initialized, we need to create the semaphore, now.
+	if (mutex->sem == -42) {
+		sem_id sem = create_sem(0, "pthread_mutex");
+		if (sem < 0)
+			return EAGAIN;
+
+		if (atomic_test_and_set((vint32*)&mutex->sem, sem, -42) != -42)
+			delete_sem(sem);
+	}
+
+	if (MUTEX_TYPE(mutex) == PTHREAD_MUTEX_ERRORCHECK
+		&& mutex->owner == thisThread) {
 		// we detect this kind of deadlock and return an error
 		return B_BUSY;
 	}
 
-	if (mutex->attr.type == PTHREAD_MUTEX_RECURSIVE && mutex->owner == thisThread) {
+	if (MUTEX_TYPE(mutex) == PTHREAD_MUTEX_RECURSIVE
+		&& mutex->owner == thisThread) {
 		// if we already hold the mutex, we don't need to grab it again
 		mutex->owner_count++;
 		return B_OK;
 	}
 
-	if (!mutex->attr.process_shared || atomic_add(&mutex->count, 1) > 0) {
+	if (atomic_add((vint32*)&mutex->count, 1) > 0) {
 		// this mutex is already locked by someone else, so we need
 		// to wait
 		status = acquire_sem_etc(mutex->sem, 1,
@@ -163,30 +114,21 @@ mutex_lock(pthread_mutex *mutex, bigtime_t timeout)
 
 
 int
-pthread_mutex_lock(pthread_mutex_t *_mutex)
+pthread_mutex_lock(pthread_mutex_t *mutex)
 {
-	if (_mutex == NULL)
+	return mutex_lock(mutex, B_INFINITE_TIMEOUT);
-		return B_BAD_VALUE;
-
-	if (*_mutex == NULL)
-		pthread_mutex_init(_mutex, NULL);
-
-	return mutex_lock(*_mutex, B_INFINITE_TIMEOUT);
 }
 
 
 int
-pthread_mutex_trylock(pthread_mutex_t *_mutex)
+pthread_mutex_trylock(pthread_mutex_t *mutex)
 {
-	if (_mutex == NULL)
+	return mutex_lock(mutex, 0);
-		return B_BAD_VALUE;
-
-	return mutex_lock(*_mutex, 0);
 }
 
 
 int 
-pthread_mutex_timedlock(pthread_mutex_t *_mutex, const struct timespec *tv)
+pthread_mutex_timedlock(pthread_mutex_t *mutex, const struct timespec *tv)
 {
 	bool invalidTime = false;
 	status_t status;
@@ -197,10 +139,7 @@ pthread_mutex_timedlock(pthread_mutex_t *_mutex, const struct timespec *tv)
 	else
 		invalidTime = true;
 
-	if (_mutex == NULL)
+	status = mutex_lock(mutex, timeout);
-		return B_BAD_VALUE;
-
-	status = mutex_lock(*_mutex, timeout);
 	if (status != B_OK && invalidTime) {
 		// POSIX requires EINVAL (= B_BAD_VALUE) to be returned
 		// if the timespec structure was invalid
@@ -212,21 +151,35 @@ pthread_mutex_timedlock(pthread_mutex_t *_mutex, const struct timespec *tv)
 
 
 int
-pthread_mutex_unlock(pthread_mutex_t *_mutex)
+pthread_mutex_unlock(pthread_mutex_t *mutex)
 {
-	if (_mutex == NULL)
+	if (mutex == NULL)
 		return B_BAD_VALUE;
 
-	return mutex_unlock(*_mutex);
+	if (mutex->owner != find_thread(NULL)) {
+		// this is a bug in the calling application!
+		// ToDo: should we handle it in another way?
+		fprintf(stderr, "mutex unlocked from foreign thread!\n");
+	}
+
+	if (MUTEX_TYPE(mutex) == PTHREAD_MUTEX_RECURSIVE) {
+		if (mutex->owner_count-- > 1)
+			return B_OK;
+
+		mutex->owner = -1;
+	}
+
+	if (atomic_add((vint32*)&mutex->count, -1) > 1)
+		return release_sem(mutex->sem);
+
+	return B_OK;
 }
 
 
 int 
-pthread_mutex_getprioceiling(pthread_mutex_t *_mutex, int *_prioCeiling)
+pthread_mutex_getprioceiling(pthread_mutex_t *mutex, int *_prioCeiling)
 {
-	pthread_mutex *mutex;
+	if (mutex == NULL || _prioCeiling == NULL)
-
-	if (_mutex == NULL || (mutex = *_mutex) == NULL || _prioCeiling == NULL)
 		return B_BAD_VALUE;
 
 	*_prioCeiling = 0;
@@ -237,15 +190,12 @@ pthread_mutex_getprioceiling(pthread_mutex_t *_mutex, int *_prioCeiling)
 
 
 int 
-pthread_mutex_setprioceiling(pthread_mutex_t *_mutex, int prioCeiling, int *_oldCeiling)
+pthread_mutex_setprioceiling(pthread_mutex_t *mutex, int prioCeiling,
+	int *_oldCeiling)
 {
-	pthread_mutex *mutex;
+	if (mutex == NULL)
-
-	if (_mutex == NULL || (mutex = *_mutex) == NULL)
 		return B_BAD_VALUE;
 
 	// not implemented
 	return B_NOT_ALLOWED;
 }
-
-

src/system/libroot/posix/pthread/pthread_once.c

@@ -1,31 +0,0 @@
-/* 
-** Copyright 2007, Jérôme Duval. All rights reserved.
-** Distributed under the terms of the MIT License.
-*/
-
-
-#include <pthread.h>
-#include "pthread_private.h"
-
-
-int 
-pthread_once(pthread_once_t *once_control, void (*init_routine)(void))
-{
-	if (once_control->state == PTHREAD_NEEDS_INIT) {
-		// TODO race condition ?
-		if (once_control->mutex == NULL) {
-			if (pthread_mutex_init(&once_control->mutex, NULL) != 0)
-				return -1;
-		}
-		pthread_mutex_lock(&once_control->mutex);
-
-		if (once_control->state == PTHREAD_NEEDS_INIT) {
-			init_routine();
-			once_control->state = PTHREAD_DONE_INIT;
-		}
-
-		pthread_mutex_unlock(&once_control->mutex);
-	}
-	return 0;
-}
-

src/system/libroot/posix/pthread/pthread_once.cpp

@@ -0,0 +1,88 @@
+/*
+ * Copyright 2008, Ingo Weinhold, [email protected].
+ * Distributed under the terms of the MIT License.
+ */
+
+#include <pthread.h>
+
+#include <OS.h>
+
+
+enum {
+	STATE_UNINITIALIZED	= -1,	// keep in sync with PTHREAD_ONCE_INIT
+	STATE_INITIALIZING	= -2,
+	STATE_SPINNING		= -3,
+	STATE_INITIALIZED	= -4
+};
+
+
+int
+pthread_once(pthread_once_t* onceControl, void (*initRoutine)(void))
+{
+	// Algorithm:
+	// The state goes through at most four states:
+	// STATE_UNINITIALIZED: The initial uninitialized state.
+	// STATE_INITIALIZING: Set by the first thread entering the function. It
+	// will call initRoutine.
+	// semaphore/STATE_SPINNING: Set by the second thread entering the function,
+	// when the first thread is still executing initRoutine. The normal case is
+	// that the thread manages to create a semaphore. This thread (and all
+	// following threads) will block on the semaphore until the first thread is
+	// done.
+	// STATE_INITIALIZED: Set by the first thread when it returns from
+	// initRoutine. All following threads will return right away.
+
+	int32 value = atomic_test_and_set((vint32*)&onceControl->state,
+		STATE_INITIALIZING, STATE_UNINITIALIZED);
+
+	if (value == STATE_INITIALIZED)
+		return 0;
+
+	if (value == STATE_UNINITIALIZED) {
+		// we're the first -- perform the initialization
+		initRoutine();
+
+		value = atomic_set((vint32*)&onceControl->state, STATE_INITIALIZED);
+
+		// If someone else is waiting, we need to delete the semaphore.
+		if (value >= 0)
+			delete_sem(value);
+
+		return 0;
+	}
+
+	if (value == STATE_INITIALIZING) {
+		// someone is initializing -- we need to create a semaphore we can wait
+		// on
+		sem_id semaphore = create_sem(0, "pthread once");
+		if (semaphore >= 0) {
+			// successfully created -- set it
+			value = atomic_test_and_set((vint32*)&onceControl->state,
+				semaphore, STATE_INITIALIZING);
+			if (value == STATE_INITIALIZING)
+				value = semaphore;
+			else
+				delete_sem(semaphore);
+		} else {
+			// Failed to create the semaphore. Can only happen when the system
+			// runs out of semaphores, but we can still handle the situation
+			// gracefully by spinning.
+			value = atomic_test_and_set((vint32*)&onceControl->state,
+				STATE_SPINNING, STATE_INITIALIZING);
+			if (value == STATE_INITIALIZING)
+				value = STATE_SPINNING;
+		}
+	}
+
+	if (value >= 0) {
+		// wait on the semaphore
+		while (acquire_sem(value) == B_INTERRUPTED);
+
+		return 0;
+	} else if (value == STATE_SPINNING) {
+		// out of semaphores -- spin
+		while (atomic_get((vint32*)&onceControl->state) == STATE_SPINNING);
+	}
+
+	return 0;
+}

src/system/libroot/posix/pthread/pthread_private.h

@@ -33,14 +33,6 @@ typedef struct _pthread_mutexattr {
 	bool		process_shared;
 } pthread_mutexattr;
 
-typedef struct _pthread_mutex {
-	vint32		count;
-	sem_id		sem;
-	thread_id	owner;
-	int32		owner_count;
-	pthread_mutexattr attr;
-} pthread_mutex;
-
 typedef struct _pthread_attr {
 	int32		detach_state;
 	int32		sched_priority;