From 92a9658c666cdc2892575a3b559716346438dab3 Mon Sep 17 00:00:00 2001 From: Fufu Fang Date: Sun, 1 Sep 2019 00:43:50 +0100 Subject: [PATCH] Cache system bug fix - Now keep track of the number of times a file has been opened. The on-disk cache file no longer gets opened multiple times, if a file is opened multiple times. --- src/cache.c | 98 ++++++++++++++++++++++++++++++++++++++++++----------- src/cache.h | 15 +++++--- src/link.c | 12 +++---- src/link.h | 12 ++++--- 4 files changed, 102 insertions(+), 35 deletions(-) diff --git a/src/cache.c b/src/cache.c index 3cde97c..b87a2f3 100644 --- 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 index 083fa36..38c894b 100644 --- a/src/cache.h +++ b/src/cache.h @@ -1,10 +1,15 @@ #ifndef CACHE_H #define CACHE_H -#include "link.h" - #include +/** + * \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; /** +/** \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 {