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

@@ -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);
     }

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", &registry, 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;
+  LumieraFiledescriptor dest = NULL;
-  lumiera_mutexacquirer_init_mutex (&registry_lock, &registry_mutex, LUMIERA_LOCKED);
 
-  lumiera_filedescriptor fdesc;
+  LUMIERA_MUTEX_SECTION (filedescriptor, &registry_mutex)
-  fdesc.flags = flags;
-
-  if (stat (name, &fdesc.stat) != 0)
     {
-      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);
+          if (errno == ENOENT && flags&O_CREAT)
-          char* slash = dir;
-          while ((slash = strchr (slash+1, '/')))
             {
-              *slash = '\0';
+              char* dir = lumiera_tmpbuf_strndup (name, PATH_MAX);
-              INFO (filedescriptor, "try creating dir: %s", dir);
+              char* slash = dir;
-              if (mkdir (dir, 0777) == -1 && errno != EEXIST)
+              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 (&registry_lock);
   return dest;
-
- efile:
- ecreate:
-  lumiera_mutexacquirer_unlock (&registry_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 (&registry_lock, &registry_mutex, LUMIERA_LOCKED);
 
-  REQUIRE (self->refcount == 0);
+  LUMIERA_MUTEX_SECTION (filedescriptor, &registry_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_mutex_destroy (&self->lock);
-  lumiera_free (self);
+      lumiera_free (self);
-
+    }
-  lumiera_mutexacquirer_unlock (&registry_lock);
 }

src/backend/filedescriptor.h

@@ -55,8 +55,6 @@ struct lumiera_filedescriptor_struct
   LumieraFilehandle handle;
   //LumieraFileMap mappings;
   //LumieraWriteBuffer writebuffer;
-
-  RESOURCE_HANDLE (rh);
 };
 
 /**

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);

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;

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;
 }

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__##_,                     \
+ * Mutual exclusive section.
-                NOBUG_RESOURCE_EXCLUSIVE, rh_##__LINE__##_);                            \
+ */
-for (lumiera_mutexacquirer_init_mutex (&lock_##__LINE__##_, mutex, LUMIERA_LOCKED);     \
+#define LUMIERA_MUTEX_SECTION(nobugflag, mtx)                                                                                   \
-     lock_##__LINE__##_.state == LUMIERA_LOCKED;                                        \
+  for (lumiera_mutexacquirer NOBUG_CLEANUP(lumiera_mutexacquirer_ensureunlocked) lumiera_mutex_section_ = {(LumieraMutex)1};    \
-     lumiera_mutexacquirer_unlock (&lock_##__LINE__##_),                                \
+       lumiera_mutex_section_.mutex;)                                                                                           \
-       ({RESOURCE_LEAVE(flag, rh_##__LINE__##_);}))
+    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;
+  volatile LumieraMutex mutex;
-  enum lumiera_lockstate  state;
+  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");
+  ENSURE (!self->mutex, "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;
 }
 
 #endif

tests/15locking.tests

@@ -9,8 +9,18 @@ END
 
 
 
+TEST "mutex section" mutexsection <<END
+out: mutex locked section 1
+out: mutex locked section 2
+END
+
+
 TEST "mutex not unlocked asserts" mutexforgotunlock <<END
 return: 134
 END
 
 
+TEST "nested mutex section" nestedmutexsection <<END
+out: outer mutex locked section
+out: inner mutex locked section
+END

tests/locking/test-locking.c

@@ -22,9 +22,9 @@
 #include <stdio.h>
 #include <string.h>
 #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