From 07f06d0d88a65caae3b2d348f9e2fb1df4bead86 Mon Sep 17 00:00:00 2001 From: Christian Thaeter Date: Sat, 9 Aug 2008 09:33:14 +0200 Subject: [PATCH] big mutex update, dropped old acquirer Acquiring mutexes is now wraped in a easy to use MUTEX_SECTION macro. This scheme will be extended for chained lock propagation soon. Notes: * NoBug resourcemanagement is now part of the lower layer, RESOURCE_ENTER/RESOUCE_LEAVE are maintained automatically * one must still call RESOURCE_ANNOUNCE/RESOURCE_FORGET, because we want to maintain high level information about resources. * MUTEX_SECTIONS must not be left with any kind of jump --- src/backend/file.c | 4 +- src/backend/filedescriptor.c | 140 ++++++++++++++--------------- src/backend/filedescriptor.h | 2 - src/backend/filehandlecache.c | 11 +-- src/backend/filehandlecache.h | 1 - src/lib/mutex.c | 1 + src/lib/mutex.h | 160 +++++++--------------------------- tests/15locking.tests | 10 +++ tests/locking/test-locking.c | 68 ++++++++++++++- 9 files changed, 185 insertions(+), 212 deletions(-) diff --git a/src/backend/file.c b/src/backend/file.c index 2b1929d6c..0cf5b6a3c 100644 --- a/src/backend/file.c +++ b/src/backend/file.c @@ -79,7 +79,7 @@ lumiera_file_handle_acquire (LumieraFile self) REQUIRE (self->descriptor); REQUIRE (lumiera_fhcache); - LUMIERA_MUTEX_SECTION (file, self->descriptor->rh, &self->descriptor->lock) + LUMIERA_MUTEX_SECTION (file, &self->descriptor->lock) { if (!self->descriptor->handle) /* no handle yet, get a new one */ @@ -123,7 +123,7 @@ lumiera_file_handle_release (LumieraFile self) { TRACE (file); - LUMIERA_MUTEX_SECTION (file, self->descriptor->rh, &self->descriptor->lock) + LUMIERA_MUTEX_SECTION (file, &self->descriptor->lock) { lumiera_filehandlecache_checkin (lumiera_fhcache, self->descriptor->handle); } diff --git a/src/backend/filedescriptor.c b/src/backend/filedescriptor.c index 005bc425b..2c37a6e5e 100644 --- a/src/backend/filedescriptor.c +++ b/src/backend/filedescriptor.c @@ -93,6 +93,9 @@ lumiera_filedescriptor_registry_init (void) 3); if (!registry) LUMIERA_DIE (NO_MEMORY); + + RESOURCE_HANDLE_INIT (registry_mutex.rh); + RESOURCE_ANNOUNCE (filedescriptor, "mutex", "filedescriptor registry", ®istry, registry_mutex.rh); } void @@ -100,6 +103,9 @@ lumiera_filedescriptor_registry_destroy (void) { TRACE (filedescriptor); REQUIRE (!cuckoo_nelements (registry)); + + RESOURCE_FORGET (filedescriptor, registry_mutex.rh); + if (registry) cuckoo_free (registry); registry = NULL; @@ -112,75 +118,72 @@ lumiera_filedescriptor_acquire (const char* name, int flags) TRACE (filedescriptor, "%s", name); REQUIRE (registry, "not initialized"); - lumiera_mutexacquirer registry_lock; - lumiera_mutexacquirer_init_mutex (®istry_lock, ®istry_mutex, LUMIERA_LOCKED); + LumieraFiledescriptor dest = NULL; - lumiera_filedescriptor fdesc; - fdesc.flags = flags; - - if (stat (name, &fdesc.stat) != 0) + LUMIERA_MUTEX_SECTION (filedescriptor, ®istry_mutex) { - if (errno == ENOENT && flags&O_CREAT) + lumiera_filedescriptor fdesc; + fdesc.flags = flags; + + if (stat (name, &fdesc.stat) != 0) { - char* dir = lumiera_tmpbuf_strndup (name, PATH_MAX); - char* slash = dir; - while ((slash = strchr (slash+1, '/'))) + if (errno == ENOENT && flags&O_CREAT) { - *slash = '\0'; - INFO (filedescriptor, "try creating dir: %s", dir); - if (mkdir (dir, 0777) == -1 && errno != EEXIST) + char* dir = lumiera_tmpbuf_strndup (name, PATH_MAX); + char* slash = dir; + while ((slash = strchr (slash+1, '/'))) + { + *slash = '\0'; + INFO (filedescriptor, "try creating dir: %s", dir); + if (mkdir (dir, 0777) == -1 && errno != EEXIST) + { + LUMIERA_ERROR_SET (filedescriptor, ERRNO); + goto error; + } + *slash = '/'; + } + int fd; + INFO (filedescriptor, "try creating file: %s", name); + fd = creat (name, 0777); + if (fd == -1) { LUMIERA_ERROR_SET (filedescriptor, ERRNO); - goto efile; + goto error; + } + close (fd); + if (stat (name, &fdesc.stat) != 0) + { + /* finally, no luck */ + LUMIERA_ERROR_SET (filedescriptor, ERRNO); + goto error; } - *slash = '/'; - } - int fd; - INFO (filedescriptor, "try creating file: %s", name); - fd = creat (name, 0777); - if (fd == -1) - { - LUMIERA_ERROR_SET (filedescriptor, ERRNO); - goto efile; - } - close (fd); - if (stat (name, &fdesc.stat) != 0) - { - /* finally, no luck */ - LUMIERA_ERROR_SET (filedescriptor, ERRNO); - goto efile; } } + + /* lookup/create descriptor */ + dest = &fdesc; + LumieraFiledescriptor* found = cuckoo_find (registry, &dest); + + if (!found) + { + TRACE (filedescriptor, "Descriptor not found"); + + dest = lumiera_filedescriptor_new (&fdesc); + if (!dest) + goto error; + + cuckoo_insert (registry, &dest); + } + else + { + TRACE (filedescriptor, "Descriptor already existing"); + dest = *found; + ++dest->refcount; + } + error: ; } - /* lookup/create descriptor */ - LumieraFiledescriptor dest = &fdesc; - LumieraFiledescriptor* found = cuckoo_find (registry, &dest); - - if (!found) - { - TRACE (filedescriptor, "Descriptor not found"); - - dest = lumiera_filedescriptor_new (&fdesc); - if (!dest) - goto ecreate; - - cuckoo_insert (registry, &dest); - } - else - { - TRACE (filedescriptor, "Descriptor already existing"); - dest = *found; - ++dest->refcount; - } - - lumiera_mutexacquirer_unlock (®istry_lock); return dest; - - efile: - ecreate: - lumiera_mutexacquirer_unlock (®istry_lock); - return NULL; } @@ -209,7 +212,7 @@ lumiera_filedescriptor_new (LumieraFiledescriptor template) const char* type = "mutex"; const char* name = "filedescriptor"; - RESOURCE_ANNOUNCE (filedescriptor, type, name, self, self->rh); + RESOURCE_ANNOUNCE (filedescriptor, type, name, self, self->lock.rh); return self; } @@ -219,22 +222,21 @@ void lumiera_filedescriptor_delete (LumieraFiledescriptor self) { TRACE (filedescriptor, "%p", self); - lumiera_mutexacquirer registry_lock; - lumiera_mutexacquirer_init_mutex (®istry_lock, ®istry_mutex, LUMIERA_LOCKED); - REQUIRE (self->refcount == 0); + LUMIERA_MUTEX_SECTION (filedescriptor, ®istry_mutex) + { + REQUIRE (self->refcount == 0); - RESOURCE_FORGET (filedescriptor, self->rh); + RESOURCE_FORGET (filedescriptor, self->lock.rh); - cuckoo_remove (registry, cuckoo_find (registry, &self)); + cuckoo_remove (registry, cuckoo_find (registry, &self)); - TODO ("destruct other members (WIP)"); + TODO ("destruct other members (WIP)"); - TODO ("release filehandle"); + TODO ("release filehandle"); - lumiera_mutex_destroy (&self->lock); - lumiera_free (self); - - lumiera_mutexacquirer_unlock (®istry_lock); + lumiera_mutex_destroy (&self->lock); + lumiera_free (self); + } } diff --git a/src/backend/filedescriptor.h b/src/backend/filedescriptor.h index 528be41e8..668419b10 100644 --- a/src/backend/filedescriptor.h +++ b/src/backend/filedescriptor.h @@ -55,8 +55,6 @@ struct lumiera_filedescriptor_struct LumieraFilehandle handle; //LumieraFileMap mappings; //LumieraWriteBuffer writebuffer; - - RESOURCE_HANDLE (rh); }; /** diff --git a/src/backend/filehandlecache.c b/src/backend/filehandlecache.c index 0a4852879..f3eb48d7e 100644 --- a/src/backend/filehandlecache.c +++ b/src/backend/filehandlecache.c @@ -45,7 +45,7 @@ lumiera_filehandlecache_new (int max_entries) lumiera_fhcache->available = max_entries; lumiera_fhcache->checked_out = 0; lumiera_mutex_init (&lumiera_fhcache->lock); - RESOURCE_ANNOUNCE (filehandlecache, "mutex", "filehandlecache", lumiera_fhcache, lumiera_fhcache->rh); + RESOURCE_ANNOUNCE (filehandlecache, "mutex", "filehandlecache", lumiera_fhcache, lumiera_fhcache->lock.rh); } @@ -55,7 +55,7 @@ lumiera_filehandlecache_delete (void) if (lumiera_fhcache) { REQUIRE (!lumiera_fhcache->checked_out, "Filehandles in use at shutdown"); - RESOURCE_FORGET (filehandlecache, lumiera_fhcache->rh); + RESOURCE_FORGET (filehandlecache, lumiera_fhcache->lock.rh); lumiera_mrucache_destroy (&lumiera_fhcache->cache); lumiera_mutex_destroy (&lumiera_fhcache->lock); lumiera_free (lumiera_fhcache); @@ -69,7 +69,8 @@ lumiera_filehandlecache_handle_acquire (LumieraFilehandlecache self, LumieraFile { TRACE (filehandlecache); LumieraFilehandle ret = NULL; - LUMIERA_MUTEX_SECTION (filehandlecache, self->rh, &self->lock) + + LUMIERA_MUTEX_SECTION (filehandlecache, &self->lock) { if (self->available <= 0 && self->cache.cached) { @@ -106,7 +107,7 @@ lumiera_filehandlecache_checkout (LumieraFilehandlecache self, LumieraFilehandle if (!handle->use_cnt) { /* lock cache and checkout */ - LUMIERA_MUTEX_SECTION (filehandlecache, self->rh, &self->lock) + LUMIERA_MUTEX_SECTION (filehandlecache, &self->lock) { lumiera_mrucache_checkout (&self->cache, &handle->cachenode); } @@ -128,7 +129,7 @@ lumiera_filehandlecache_checkin (LumieraFilehandlecache self, LumieraFilehandle if (!--handle->use_cnt) { /* lock cache and checin */ - LUMIERA_MUTEX_SECTION (filehandlecache, self->rh, &self->lock) + LUMIERA_MUTEX_SECTION (filehandlecache, &self->lock) { --self->checked_out; lumiera_mrucache_checkin (&self->cache, &handle->cachenode); diff --git a/src/backend/filehandlecache.h b/src/backend/filehandlecache.h index 37c2a416b..f624caefe 100644 --- a/src/backend/filehandlecache.h +++ b/src/backend/filehandlecache.h @@ -49,7 +49,6 @@ struct lumiera_filehandlecache_struct int available; int checked_out; lumiera_mutex lock; - RESOURCE_HANDLE (rh); }; extern LumieraFilehandlecache lumiera_fhcache; diff --git a/src/lib/mutex.c b/src/lib/mutex.c index 20b0cce70..791ca8287 100644 --- a/src/lib/mutex.c +++ b/src/lib/mutex.c @@ -37,6 +37,7 @@ lumiera_mutex_init (LumieraMutex self) if (self) { pthread_mutex_init (&self->mutex, NULL); + NOBUG_RESOURCE_HANDLE_INIT (self->rh); } return self; } diff --git a/src/lib/mutex.h b/src/lib/mutex.h index 7e67b550a..79424111d 100644 --- a/src/lib/mutex.h +++ b/src/lib/mutex.h @@ -28,15 +28,31 @@ * @file * Mutual exclusion locking, header. */ -#define LUMIERA_MUTEX_SECTION(flag, handle, mutex) \ -RESOURCE_HANDLE (rh_##__LINE__##_); \ -lumiera_mutexacquirer lock_##__LINE__##_; \ -RESOURCE_ENTER (flag, handle, "acquire mutex", &lock_##__LINE__##_, \ - NOBUG_RESOURCE_EXCLUSIVE, rh_##__LINE__##_); \ -for (lumiera_mutexacquirer_init_mutex (&lock_##__LINE__##_, mutex, LUMIERA_LOCKED); \ - lock_##__LINE__##_.state == LUMIERA_LOCKED; \ - lumiera_mutexacquirer_unlock (&lock_##__LINE__##_), \ - ({RESOURCE_LEAVE(flag, rh_##__LINE__##_);})) + + +/** + * Mutual exclusive section. + */ +#define LUMIERA_MUTEX_SECTION(nobugflag, mtx) \ + for (lumiera_mutexacquirer NOBUG_CLEANUP(lumiera_mutexacquirer_ensureunlocked) lumiera_mutex_section_ = {(LumieraMutex)1}; \ + lumiera_mutex_section_.mutex;) \ + for ( \ + ({ \ + lumiera_mutex_section_.mutex = (mtx); \ + NOBUG_RESOURCE_HANDLE_INIT (lumiera_mutex_section_.rh); \ + RESOURCE_ENTER (nobugflag, (mtx)->rh, "acquire mutex", &lumiera_mutex_section_, \ + NOBUG_RESOURCE_EXCLUSIVE, lumiera_mutex_section_.rh); \ + if (pthread_mutex_lock (&(mtx)->mutex)) LUMIERA_DIE (MUTEX_LOCK); \ + }); \ + lumiera_mutex_section_.mutex; \ + ({ \ + if (lumiera_mutex_section_.mutex) \ + { \ + pthread_mutex_unlock (&lumiera_mutex_section_.mutex->mutex); \ + lumiera_mutex_section_.mutex = NULL; \ + RESOURCE_LEAVE(nobugflag, lumiera_mutex_section_.rh); \ + } \ + })) /** @@ -46,6 +62,7 @@ for (lumiera_mutexacquirer_init_mutex (&lock_##__LINE__##_, mutex, LUMIERA_LOCKE struct lumiera_mutex_struct { pthread_mutex_t mutex; + RESOURCE_HANDLE (rh); }; typedef struct lumiera_mutex_struct lumiera_mutex; typedef lumiera_mutex* LumieraMutex; @@ -69,14 +86,13 @@ LumieraMutex lumiera_mutex_destroy (LumieraMutex self); - /** - * mutexacquirer used to manage the state of a mutex variable. + * mutexacquirer used to manage the state of a mutex. */ struct lumiera_mutexacquirer_struct { - LumieraMutex mutex; - enum lumiera_lockstate state; + volatile LumieraMutex mutex; + RESOURCE_HANDLE (rh); }; typedef struct lumiera_mutexacquirer_struct lumiera_mutexacquirer; typedef struct lumiera_mutexacquirer_struct* LumieraMutexacquirer; @@ -85,123 +101,7 @@ typedef struct lumiera_mutexacquirer_struct* LumieraMutexacquirer; static inline void lumiera_mutexacquirer_ensureunlocked (LumieraMutexacquirer self) { - ENSURE (self->state == LUMIERA_UNLOCKED, "forgot to unlock mutex"); -} - -/* override with a macro to use the cleanup checker */ -#define lumiera_mutexacquirer \ -lumiera_mutexacquirer NOBUG_CLEANUP(lumiera_mutexacquirer_ensureunlocked) - - -/** - * initialize a mutexacquirer state without mutex. - * @param self mutexacquirer to be initialized, must be an automatic variable - * @return self as given - * This initialization is used when lumiera_mutexacquirer_try_mutex shall be used later - */ -static inline LumieraMutexacquirer -lumiera_mutexacquirer_init (LumieraMutexacquirer self) -{ - REQUIRE (self); - self->mutex = NULL; - self->state = LUMIERA_UNLOCKED; - - return self; -} - -/** - * initialize a mutexacquirer state - * @param self mutexacquirer to be initialized, must be an automatic variable - * @param mutex associated mutex - * @param state initial state of the mutex, either LUMIERA_LOCKED or LUMIERA_UNLOCKED - * @return self as given - * errors are fatal - */ -static inline LumieraMutexacquirer -lumiera_mutexacquirer_init_mutex (LumieraMutexacquirer self, LumieraMutex mutex, enum lumiera_lockstate state) -{ - REQUIRE (self); - REQUIRE (mutex); - self->mutex = mutex; - self->state = state; - if (state == LUMIERA_LOCKED) - if (pthread_mutex_lock (&mutex->mutex)) - LUMIERA_DIE (MUTEX_LOCK); - - return self; -} - - -/** - * lock the mutex. - * must not already be locked - * @param self mutexacquirer associated with a mutex variable - */ -static inline void -lumiera_mutexacquirer_lock (LumieraMutexacquirer self) -{ - REQUIRE (self); - REQUIRE (self->state == LUMIERA_UNLOCKED, "mutex already locked"); - - if (pthread_mutex_lock (&self->mutex->mutex)) - LUMIERA_DIE (MUTEX_LOCK); - - self->state = LUMIERA_LOCKED; -} - - -/** - * get the state of a lock. - * @param self mutexacquirer associated with a mutex variable - * @return LUMIERA_LOCKED when the mutex is locked by this thead - */ -static inline enum lumiera_lockstate -lumiera_mutexacquirer_state (LumieraMutexacquirer self) -{ - REQUIRE (self); - return self->state; -} - - -/** - * try to lock a mutex. - * must not already be locked - * @param self mutexacquirer associated with a mutex variable - * @param mutex pointer to a mutex which should be tried - * @return LUMIERA_LOCKED when the mutex got locked - */ -static inline enum lumiera_lockstate -lumiera_mutexacquirer_try_mutex (LumieraMutexacquirer self, LumieraMutex mutex) -{ - REQUIRE (self); - REQUIRE (self->state == LUMIERA_UNLOCKED, "mutex already locked"); - - self->mutex=mutex; - switch (pthread_mutex_trylock (&self->mutex->mutex)) - { - case 0: - return self->state = LUMIERA_LOCKED; - case EBUSY: - return LUMIERA_UNLOCKED; - default: - LUMIERA_DIE (MUTEX_LOCK); - } -} - - -/** - * release mutex. - * a mutexacquirer must be unlocked before leaving scope - * @param self mutexacquirer associated with a mutex variable - */ -static inline void -lumiera_mutexacquirer_unlock (LumieraMutexacquirer self) -{ - REQUIRE (self); - REQUIRE (self->state == LUMIERA_LOCKED, "mutex was not locked"); - if (pthread_mutex_unlock (&self->mutex->mutex)) - LUMIERA_DIE (MUTEX_UNLOCK); - self->state = LUMIERA_UNLOCKED; + ENSURE (!self->mutex, "forgot to unlock mutex"); } #endif diff --git a/tests/15locking.tests b/tests/15locking.tests index 7bea487d5..80dda1a23 100644 --- a/tests/15locking.tests +++ b/tests/15locking.tests @@ -9,8 +9,18 @@ END +TEST "mutex section" mutexsection < #include #include "tests/test.h" +#include "lib/mutex.h" int conditionforgotunlock (); -int mutexforgotunlock (); TESTS_BEGIN @@ -33,9 +33,71 @@ TEST ("conditionforgotunlock") return conditionforgotunlock (); } -TEST ("mutexforgotunlock") +TEST ("mutexsection") { - return mutexforgotunlock (); + lumiera_mutex m; + lumiera_mutex_init (&m); + RESOURCE_ANNOUNCE (NOBUG_ON, "mutex", "mutexsection", &m, m.rh); + + LUMIERA_MUTEX_SECTION (NOBUG_ON, &m) + { + printf ("mutex locked section 1\n"); + } + + LUMIERA_MUTEX_SECTION (NOBUG_ON, &m) + { + printf ("mutex locked section 2\n"); + } + + RESOURCE_FORGET (NOBUG_ON, m.rh); + lumiera_mutex_destroy (&m); } + +TEST ("mutexforgotunlock") +{ + lumiera_mutex m; + lumiera_mutex_init (&m); + RESOURCE_ANNOUNCE (NOBUG_ON, "mutex", "mutexforgotunlock", &m, m.rh); + + LUMIERA_MUTEX_SECTION (NOBUG_ON, &m) + { + break; // MUTEX_SECTIONS must not be left by a jump + } + + RESOURCE_FORGET (NOBUG_ON, m.rh); + lumiera_mutex_destroy (&m); +} + + +TEST ("nestedmutexsection") +{ + lumiera_mutex m; + lumiera_mutex_init (&m); + RESOURCE_ANNOUNCE (NOBUG_ON, "mutex", "m_mutexsection", &m, m.rh); + + lumiera_mutex n; + lumiera_mutex_init (&n); + RESOURCE_ANNOUNCE (NOBUG_ON, "mutex", "n_mutexsection", &n, n.rh); + + LUMIERA_MUTEX_SECTION (NOBUG_ON, &m) + { + printf ("outer mutex locked section\n"); + + LUMIERA_MUTEX_SECTION (NOBUG_ON, &n) + { + printf ("inner mutex locked section\n"); + } + } + + RESOURCE_FORGET (NOBUG_ON, n.rh); + lumiera_mutex_destroy (&n); + + RESOURCE_FORGET (NOBUG_ON, m.rh); + lumiera_mutex_destroy (&m); +} + + + + TESTS_END