Fixed some caching bugs in Db.

Note that Db has the potential to deadlock when caching is being used,
and when caching isn't being used, an inconsistent state can occur. Future
changes to Db will fix both of these issues.

TODO.txt

@@ -82,6 +82,9 @@ Milestone: v0.3.0
     [ ] Refactor dev pages so function description and
         return value are in the same location.
 
+[ ] Debug memory leaks with caching
+[ ] Debug OpenSSL
+
 [~] Client-Server API
     [x] 4: Token-based user registration
         [x] Implement user-interactive auth flow
@@ -90,9 +93,9 @@ Milestone: v0.3.0
             flow
             - Ensure that registration tokens can be used even if
               registration is disabled.
-        [~] User-Interactive fallback
+        [x] User-Interactive fallback
-            [~] Password
+            [x] Password
-            [~] Registration token
+            [x] Registration token
     [~] 4: Account management
         [~] Deactivate
             [x] Make sure UserLogin() fails if user is deactivated.

src/Db.c

@@ -28,6 +28,7 @@
 #include <Util.h>
 #include <Str.h>
 #include <Stream.h>
+#include <Log.h>
 
 #include <sys/types.h>
 #include <dirent.h>
@@ -287,7 +288,7 @@ DbCacheEvict(Db * db)
 
     while (ref && db->cacheSize > db->maxCache)
     {
-        char *hash = DbHashKey(ref->name);
+        char *hash;
 
         if (pthread_mutex_trylock(&ref->lock) != 0)
         {
@@ -308,15 +309,23 @@ DbCacheEvict(Db * db)
 
         db->cacheSize -= ref->size;
 
-        ref->next->prev = ref->prev;
+        if (ref->next)
-        if (!ref->prev)
         {
-            db->leastRecent = ref->next;
+            ref->next->prev = ref->prev;
         }
         else
+        {
+            db->mostRecent = ref->prev;
+        }
+
+        if (ref->prev)
         {
             ref->prev->next = ref->next;
         }
+        else
+        {
+            db->leastRecent = ref->next;
+        }
 
         tmp = ref->next;
         Free(ref);
@@ -348,6 +357,7 @@ DbOpen(char *dir, size_t cache)
 
     db->mostRecent = NULL;
     db->leastRecent = NULL;
+    db->cacheSize = 0;
 
     if (db->maxCache)
     {
@@ -374,31 +384,29 @@ DbMaxCacheSet(Db * db, size_t cache)
     }
 
     db->maxCache = cache;
+    if (db->maxCache && !db->cache)
+    {
+        db->cache = HashMapCreate();
+        db->cacheSize = 0;
+    }
+
     DbCacheEvict(db);
 }
 
 void
 DbClose(Db * db)
 {
-    char *key;
-    DbRef *val;
-
     if (!db)
     {
         return;
     }
 
-    pthread_mutex_destroy(&db->lock);
+    DbMaxCacheSet(db, 0);
-
+    DbCacheEvict(db);
-    while (HashMapIterate(db->cache, &key, (void **) &val))
-    {
-        JsonFree(val->json);
-        StringArrayFree(val->name);
-        pthread_mutex_destroy(&val->lock);
-        Free(val);
-    }
     HashMapFree(db->cache);
 
+    pthread_mutex_destroy(&db->lock);
+
     Free(db);
 }
 
@@ -459,6 +467,11 @@ DbLockFromArr(Db * db, Array * args)
                 db->leastRecent = ref->next;
             }
 
+            if (!db->leastRecent)
+            {
+                db->leastRecent = db->mostRecent;
+            }
+
             pthread_mutex_unlock(&ref->lock);
             pthread_mutex_destroy(&ref->lock);
             Free(ref);
@@ -488,6 +501,9 @@ DbLockFromArr(Db * db, Array * args)
 
         pthread_mutex_lock(&ref->lock);
 
+        ref->fd = fd;
+        ref->stream = stream;
+
         if (diskTs > ref->ts)
         {
             /* File was modified on disk since it was cached */
@@ -523,9 +539,21 @@ DbLockFromArr(Db * db, Array * args)
 
             ref->prev = db->mostRecent;
             ref->next = NULL;
+            if (db->mostRecent)
+            {
+                db->mostRecent->next = ref;
+            }
             db->mostRecent = ref;
         }
 
+        /* If there is no least recent, this is the only
+         * thing in the cache, so it is also least recent.
+         */
+        if (!db->leastRecent)
+        {
+            db->leastRecent = ref;
+        }
+
         /* The file on disk may be larger than what we have in memory,
          * which may require items in cache to be evicted. */
         DbCacheEvict(db);
@@ -567,7 +595,7 @@ DbLockFromArr(Db * db, Array * args)
         }
         ref->name = name;
 
-        if (db->maxCache)
+        if (db->cache)
         {
             ref->ts = UtilServerTs();
             ref->size = DbComputeSize(ref->json);
@@ -576,8 +604,17 @@ DbLockFromArr(Db * db, Array * args)
 
             ref->next = NULL;
             ref->prev = db->mostRecent;
+            if (db->mostRecent)
+            {
+                db->mostRecent->next = ref;
+            }
             db->mostRecent = ref;
 
+            if (!db->leastRecent)
+            {
+                db->leastRecent = ref;
+            }
+
             /* Adding this item to the cache may case it to grow too
              * large, requiring some items to be evicted */
             DbCacheEvict(db);
@@ -708,6 +745,11 @@ DbDelete(Db * db, size_t nArgs,...)
             db->leastRecent = ref->next;
         }
 
+        if (!db->leastRecent)
+        {
+            db->leastRecent = db->mostRecent;
+        }
+
         pthread_mutex_unlock(&ref->lock);
         pthread_mutex_destroy(&ref->lock);
         Free(ref);
@@ -753,6 +795,8 @@ DbLock(Db * db, size_t nArgs,...)
 int
 DbUnlock(Db * db, DbRef * ref)
 {
+    int destroy;
+
     if (!db || !ref)
     {
         return 0;
@@ -764,31 +808,46 @@ DbUnlock(Db * db, DbRef * ref)
     if (ftruncate(ref->fd, 0) < 0)
     {
         pthread_mutex_unlock(&db->lock);
+        Log(LOG_ERR, "Failed to truncate file on disk.");
+        Log(LOG_ERR, "Error: %s", strerror(errno));
         return 0;
     }
 
     JsonEncode(ref->json, ref->stream, JSON_DEFAULT);
-
     StreamClose(ref->stream);
 
-    if (db->maxCache)
+    if (db->cache)
     {
-        db->cacheSize -= ref->size;
+        char *key = DbHashKey(ref->name);
-        ref->size = DbComputeSize(ref->json);
-        db->cacheSize += ref->size;
 
-        /* If this ref has grown significantly since we last computed
+        if (HashMapGet(db->cache, key))
-         * its size, it may have filled the cache and require some
+        {
-         * items to be evicted. */
+            db->cacheSize -= ref->size;
-        DbCacheEvict(db);
+            ref->size = DbComputeSize(ref->json);
-        pthread_mutex_unlock(&ref->lock);
+            db->cacheSize += ref->size;
+
+            /* If this ref has grown significantly since we last computed
+            * its size, it may have filled the cache and require some
+            * items to be evicted. */
+            DbCacheEvict(db);
+
+            destroy = 0;
+        } else {
+            destroy = 1;
+        }
+
+        Free(key);
+    } else {
+        destroy = 1;
     }
-    else
+
+    pthread_mutex_unlock(&ref->lock);
+
+    if (destroy)
     {
         JsonFree(ref->json);
         StringArrayFree(ref->name);
 
-        pthread_mutex_unlock(&ref->lock);
         pthread_mutex_destroy(&ref->lock);
 
         Free(ref);
@@ -913,3 +972,4 @@ DbJsonSet(DbRef * ref, HashMap * json)
     ref->json = JsonDuplicate(json);
     return 1;
 }
+