Merge pull request #42 from fangfufu/cache_bug_fix

Cache system bug fix
fangfufu · Aug 31, 2019 · 55692cf · 55692cf
2 parents ef7630f + 92a9658
commit 55692cf
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 35 deletions.
diff --git a/src/cache.c b/src/cache.c
@@ -37,11 +37,21 @@ typedef enum {
     EMEM        = -4    /**< Memory allocation failure */
 } MetaError;
 
+/* ---------------- External variables -----------------------*/
 
 int CACHE_SYSTEM_INIT = 0;
 int DATA_BLK_SZ = 0;
 int MAX_SEGBC = DEFAULT_MAX_SEGBC;
 
+/* ----------------- Static variables ----------------------- */
+
+/**
+ * \brief Cache file locking
+ * \details Ensure cache opening and cache closing is an atomic operation
+ */
+static pthread_mutex_t cf_lock;
+
+
 /**
  * \brief The metadata directory
  */
@@ -111,6 +121,12 @@ static char *CacheSystem_calc_dir(const char *url)
 
 void CacheSystem_init(const char *path, int url_supplied)
 {
+    if (pthread_mutex_init(&cf_lock, NULL) != 0) {
+        fprintf(stderr,
+                "CacheSystem_init(): cf_lock initialisation failed!\n");
+        exit(EXIT_FAILURE);
+    }
+
     if (url_supplied) {
         path = CacheSystem_calc_dir(path);
     }
@@ -696,16 +712,43 @@ Cache *Cache_open(const char *fn)
         return NULL;
     }
 
+    /* Obtain the link structure memory pointer */
+    Link *link = path_to_Link(fn);
+    if (!link) {
+        return NULL;
+    }
+
+    /*---------------- Cache_open() critical section -----------------*/
+
+#ifdef CACHE_LOCK_DEBUG
+    fprintf(stderr, "Cache_open(): thread %lu: locking cf_lock;\n",
+            pthread_self());
+#endif
+    pthread_mutex_lock(&cf_lock);
+
+    if (link->cache_opened) {
+        link->cache_opened++;
+#ifdef CACHE_LOCK_DEBUG
+        fprintf(stderr, "Cache_open(): thread %lu: unlocking cf_lock;\n",
+                pthread_self());
+#endif
+        pthread_mutex_unlock(&cf_lock);
+        return link->cache_ptr;
+    }
+
+#ifdef CACHE_LOCK_DEBUG
+    fprintf(stderr, "Cache_open(): thread %lu: unlocking cf_lock;\n",
+            pthread_self());
+#endif
+    pthread_mutex_unlock(&cf_lock);
+    /*----------------------------------------------------------------*/
+
     /* Create the cache in-memory data structure */
     Cache *cf = Cache_alloc();
     cf->path = strndup(fn, MAX_PATH_LEN);
 
     /* Associate the cache structure with a link */
-    cf->link = path_to_Link(fn);
-    if (!cf->link) {
-        Cache_free(cf);
-        return NULL;
-    }
+    cf->link = link;
 
     if (Meta_open(cf)) {
         Cache_free(cf);
@@ -750,22 +793,41 @@ cf->content_length: %ld, Data_size(fn): %ld.\n", fn, cf->content_length,
         return NULL;
     }
 
+    cf->link->cache_opened = 1;
+    /* Yup, we just created a circular loop. ;) */
+    cf->link->cache_ptr = cf;
+
     return cf;
 }
 
 void Cache_close(Cache *cf)
 {
+    /*--------------- Cache_close() critical section -----------------*/
+
 #ifdef CACHE_LOCK_DEBUG
-    /* Must wait for the background download thread to stop */
-    fprintf(stderr, "Cache_close(): locking bgt_lock;\n");
-        pthread_mutex_lock(&cf->bgt_lock);
-    fprintf(stderr, "Cache_close(): unlocking bgt_lock;\n");
-        pthread_mutex_unlock(&cf->bgt_lock);
-    fprintf(stderr, "Cache_close(): locking rw_lock;\n");
-        pthread_mutex_lock(&cf->rw_lock);
-    fprintf(stderr, "Cache_close(): unlocking rw_lock;\n");
-        pthread_mutex_unlock(&cf->rw_lock);
+    fprintf(stderr, "Cache_close(): thread %lu: locking cf_lock;\n",
+            pthread_self());
+#endif
+    pthread_mutex_lock(&cf_lock);
+
+    cf->link->cache_opened--;
+
+    if (cf->link->cache_opened > 0) {
+#ifdef CACHE_LOCK_DEBUG
+        fprintf(stderr, "Cache_close(): thread %lu: unlocking cf_lock;\n",
+                pthread_self());
+#endif
+        pthread_mutex_unlock(&cf_lock);
+        return;
+    }
+
+#ifdef CACHE_LOCK_DEBUG
+    fprintf(stderr, "Cache_close(): thread %lu: unlocking cf_lock;\n",
+            pthread_self());
 #endif
+    pthread_mutex_unlock(&cf_lock);
+
+    /*----------------------------------------------------------------*/
 
     if (Meta_write(cf)) {
         fprintf(stderr, "Cache_close(): Meta_write() error.");
@@ -876,15 +938,13 @@ long Cache_read(Cache *cf, char *output_buf, off_t len, off_t offset)
 
 #ifdef CACHE_LOCK_DEBUG
         /* Wait for the background download thread to finish */
-        fprintf(stderr, "Cache_read(): thread %lu: locking bgt_lock;\n",
+        fprintf(stderr,
+                "Cache_read(): thread %lu: locking and unlocking bgt_lock;\n",
                 pthread_self());
 #endif
         pthread_mutex_lock(&cf->bgt_lock);
-#ifdef CACHE_LOCK_DEBUG
-        fprintf(stderr, "Cache_read(): thread %lu: unlocking bgt_lock;\n",
-                pthread_self());
-#endif
         pthread_mutex_unlock(&cf->bgt_lock);
+
 #ifdef CACHE_LOCK_DEBUG
         /* Wait for any other download thread to finish*/
         fprintf(stderr, "Cache_read(): thread %lu: locking rw_lock;\n",

diff --git a/src/cache.h b/src/cache.h
@@ -1,10 +1,15 @@
 #ifndef CACHE_H
 #define CACHE_H
 
-#include "link.h"
-
 #include <pthread.h>
 
+/**
+ * \brief cache data type
+ */
+typedef struct Cache Cache;
+
+#include "link.h"
+
 /**
  * \file cache.h
  * \brief cache related structures and functions
@@ -19,9 +24,9 @@
 typedef uint8_t Seg;
 
 /**
- * \brief cache in-memory data structure
+ * \brief cache data type in-memory data structure
  */
-typedef struct {
+struct Cache {
     char *path; /**< the path to the file on the web server */
     Link *link; /**< the Link associated with this cache data set */
     long time; /**<the modified time of the file */
@@ -41,7 +46,7 @@ typedef struct {
     int blksz; /**<the block size of the data file */
     long segbc; /**<segment array byte count */
     Seg *seg; /**< the detail of each segment */
-} Cache;
+};
 
 /**
  * \brief whether the cache system is enabled

diff --git a/src/link.c b/src/link.c
@@ -15,7 +15,8 @@
 LinkTable *ROOT_LINK_TBL = NULL;
 int ROOT_LINK_OFFSET = 0;
 
-/* ----------------- Static variable ----------------------- */
+/* ----------------- Static variables ----------------------- */
+
 /**
  * \brief LinkTable generation priority lock
  * \details This allows LinkTable generation to be run exclusively. This
@@ -584,16 +585,13 @@ long path_download(const char *path, char *output_buf, size_t size,
 
 #ifdef LINK_LOCK_DEBUG
     fprintf(stderr,
-            "path_download(): thread %lu: locking link_lock;\n",
+            "path_download(): thread %lu: locking and unlocking link_lock;\n",
             pthread_self());
 #endif
+
     pthread_mutex_lock(&link_lock);
-#ifdef LINK_LOCK_DEBUG
-    fprintf(stderr,
-            "path_download(): thread %lu: unlocking link_lock;\n",
-            pthread_self());
-#endif
     pthread_mutex_unlock(&link_lock);
+
     transfer_blocking(curl);
 
     long http_resp;

diff --git a/src/link.h b/src/link.h
@@ -5,6 +5,11 @@
 
 #include <curl/curl.h>
 
+/** \brief Link type */
+typedef struct Link Link;
+
+#include "cache.h"
+
 /** \brief the link type */
 typedef enum {
     LINK_HEAD = 'H',
@@ -19,11 +24,8 @@ typedef enum {
  */
 typedef struct LinkTable LinkTable;
 
-/** \brief link data type */
-typedef struct Link Link;
-
 /**
- * \brief Link data structure
+ * \brief Link type data structure
  */
 struct Link {
     char linkname[MAX_FILENAME_LEN+1]; /**< The link name in the last level of
@@ -33,6 +35,8 @@ struct Link {
     size_t content_length; /**< CURLINFO_CONTENT_LENGTH_DOWNLOAD of the file */
     LinkTable *next_table; /**< The next LinkTable level, if it is a LINK_DIR */
     long time; /**< CURLINFO_FILETIME obtained from the server */
+    int cache_opened; /**< How many times associated cache has been opened */
+    Cache *cache_ptr; /**< The pointer associated with the cache file */
 };
 
 struct LinkTable {