From 170d8032b2fd55450e4533357f4de118b619a8e8 Mon Sep 17 00:00:00 2001 From: rpj Date: Fri, 18 Aug 2000 14:17:28 +0000 Subject: 2000-08-18 Ross Johnson * mutex.c (pthread_mutex_destroy): Check that the mutex isn't held; invalidate the mutex as early as possible to avoid contention; not perfect - FIXME! * rwlock.c (pthread_rwlock_init): Remove redundant assignment to "rw". (pthread_rwlock_destroy): Invalidate the rwlock before freeing up any of it's resources - to avoid contention. * private.c (ptw32_tkAssocCreate): Change assoc->lock to use a dynamically initialised mutex - only consumes a W32 mutex or critical section when first used, not before. * mutex.c (pthread_mutex_init): Remove redundant assignment to "mx". (pthread_mutexattr_destroy): Set attribute to NULL before freeing it's memory - to avoid contention. --- ChangeLog | 21 ++++++++++++++++++++- mutex.c | 63 ++++++++++++++++++++++++++++++++++++++++++--------------------- private.c | 14 ++++++++++++-- rwlock.c | 11 +++++++---- 4 files changed, 81 insertions(+), 28 deletions(-) diff --git a/ChangeLog b/ChangeLog index e2b48ee..7c6b731 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,23 @@ -2000-08-18 Ross Johnson +2000-08-18 Ross Johnson + + * mutex.c (pthread_mutex_destroy): Check that the mutex isn't + held; invalidate the mutex as early as possible to avoid + contention; not perfect - FIXME! + + * rwlock.c (pthread_rwlock_init): Remove redundant assignment + to "rw". + (pthread_rwlock_destroy): Invalidate the rwlock before + freeing up any of it's resources - to avoid contention. + + * private.c (ptw32_tkAssocCreate): Change assoc->lock + to use a dynamically initialised mutex - only consumes + a W32 mutex or critical section when first used, + not before. + + * mutex.c (pthread_mutex_init): Remove redundant assignment + to "mx". + (pthread_mutexattr_destroy): Set attribute to NULL + before freeing it's memory - to avoid contention. * implement.h (PTW32_EPS_CANCEL/PTW32_EPS_EXIT): Must be defined for all compilers - used as generic diff --git a/mutex.c b/mutex.c index 3e0e484..42c0844 100644 --- a/mutex.c +++ b/mutex.c @@ -96,8 +96,6 @@ pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t *attr) return EINVAL; } - mx = *mutex; - mx = (pthread_mutex_t) calloc(1, sizeof(*mx)); if (mx == NULL) @@ -213,20 +211,40 @@ pthread_mutex_destroy(pthread_mutex_t *mutex) { mx = *mutex; - if (mx->mutex == 0) - { - DeleteCriticalSection(&mx->cs); - } - else - { - result = (CloseHandle (mx->mutex) ? 0 : EINVAL); - } - - if (result == 0) + if ((result = pthread_mutex_trylock(&mx)) == 0) { - mx->mutex = 0; - free(mx); + /* + * FIXME!!! + * The mutex isn't held by another thread but we could still + * be too late invalidating the mutex below. Yet we can't do it + * earlier in case another thread needs to unlock the mutex + * that it's holding. + */ *mutex = NULL; + + pthread_mutex_unlock(&mx); + + if (mx->mutex == 0) + { + DeleteCriticalSection(&mx->cs); + } + else + { + result = (CloseHandle (mx->mutex) ? 0 : EINVAL); + } + + if (result == 0) + { + mx->mutex = 0; + free(mx); + } + else + { + /* + * Restore the mutex before we return the error. + */ + *mutex = mx; + } } } else @@ -291,16 +309,17 @@ pthread_mutexattr_init (pthread_mutexattr_t * attr) * ------------------------------------------------------ */ { - pthread_mutexattr_t attr_result; + pthread_mutexattr_t ma; int result = 0; - attr_result = (pthread_mutexattr_t) calloc (1, sizeof (*attr_result)); + ma = (pthread_mutexattr_t) calloc (1, sizeof (*ma)); - result = (attr_result == NULL) - ? ENOMEM - : 0; + if (ma == NULL) + { + result = ENOMEM; + } - *attr = attr_result; + *attr = ma; return (result); @@ -343,9 +362,11 @@ pthread_mutexattr_destroy (pthread_mutexattr_t * attr) } else { - free (*attr); + pthread_mutexattr_t ma = *attr; *attr = NULL; + free (ma); + result = 0; } diff --git a/private.c b/private.c index 0f8a175..905a7f6 100644 --- a/private.c +++ b/private.c @@ -391,8 +391,7 @@ ptw32_tkAssocCreate (ThreadKeyAssoc ** assocP, * Have to create an association and add it * to both the key and the thread. */ - assoc = (ThreadKeyAssoc *) - calloc (1, sizeof (*assoc)); + assoc = (ThreadKeyAssoc *) calloc (1, sizeof (*assoc)); if (assoc == NULL) { @@ -400,11 +399,22 @@ ptw32_tkAssocCreate (ThreadKeyAssoc ** assocP, goto FAIL0; } +#if 0 + if ((result = pthread_mutex_init (&(assoc->lock), NULL)) != 0) { goto FAIL1; } +#else + + /* + * Initialise only when used for the first time. + */ + assoc->lock = PTHREAD_MUTEX_INITIALIZER; + +#endif + assoc->thread = thread; assoc->key = key; diff --git a/rwlock.c b/rwlock.c index d02461c..4353847 100644 --- a/rwlock.c +++ b/rwlock.c @@ -94,8 +94,6 @@ pthread_rwlock_init(pthread_rwlock_t *rwlock, const pthread_rwlockattr_t *attr) return EINVAL; } - rw = *rwlock; - rw = (pthread_rwlock_t) calloc(1, sizeof(*rw)); if (rw == NULL) @@ -182,14 +180,19 @@ pthread_rwlock_destroy(pthread_rwlock_t *rwlock) } else { + /* + * Need to NULL this before we start freeing up + * and destroying it's components. + */ + *rwlock = NULL; rw->rw_magic = 0; + (void) pthread_mutex_unlock(&(rw->rw_lock)); + (void) pthread_cond_destroy(&(rw->rw_condreaders)); (void) pthread_cond_destroy(&(rw->rw_condwriters)); (void) pthread_mutex_destroy(&(rw->rw_lock)); - free(rw); - *rwlock = NULL; } } else -- cgit v1.2.3