* eet: Reintroduce eet cache lock fix.

	This version also fix a bug when file wasn't an eet file.

git-svn-id: http://svn.enlightenment.org/svn/e/trunk/eet@47237 7cbeb6ba-43b4-40fd-8cce-4c39aea84d33

diff --git a/src/lib/eet_lib.c b/src/lib/eet_lib.c
index 8fed189..b6c792b 100644 (file)
--- a/src/lib/eet_lib.c
+++ b/src/lib/eet_lib.c
@@ -280,7 +280,7 @@ eet_test_close(int test, Eet_File *ef)
    if (test)
      {
        ef->delete_me_now = 1;
-        eet_internal_close(ef, 0);
+        eet_internal_close(ef, EINA_TRUE);
      }
    return test;
 }
@@ -307,6 +307,7 @@ eet_cache_find(const char *path, Eet_File **cache, int cache_num)
 }
 
 /* add to end of cache */
+/* this should only be called when the cache lock is already held */
 static void
 eet_cache_add(Eet_File *ef, Eet_File ***cache, int *cache_num, int *cache_alloc)
 {
@@ -333,7 +334,7 @@ eet_cache_add(Eet_File *ef, Eet_File ***cache, int *cache_num, int *cache_alloc)
        if (del_ef)
          {
             del_ef->delete_me_now = 1;
-             eet_internal_close(ef, 1);
+             eet_internal_close(ef, EINA_TRUE);
          }
      }
 
@@ -358,6 +359,7 @@ eet_cache_add(Eet_File *ef, Eet_File ***cache, int *cache_num, int *cache_alloc)
 }
 
 /* delete from cache */
+/* this should only be called when the cache lock is already held */
 static void
 eet_cache_del(Eet_File *ef, Eet_File ***cache, int *cache_num, int *cache_alloc)
 {
@@ -849,6 +851,7 @@ eet_clearcache(void)
     * We need to compute the list of eet file to close separately from the cache,
     * due to eet_close removing them from the cache after each call.
     */
+   LOCK_CACHE;
    for (i = 0; i < eet_writers_num; i++)
      {
        if (eet_writers[i]->references <= 0) num++;
@@ -887,9 +890,10 @@ eet_clearcache(void)
 
        for (i = 0; i < num; i++)
          {
-            eet_close(closelist[i]);
+            eet_internal_close(closelist[i], EINA_TRUE);
          }
      }
+   UNLOCK_CACHE;
 }
 
 /* FIXME: MMAP race condition in READ_WRITE_MODE */
@@ -1281,6 +1285,12 @@ eet_internal_read1(Eet_File *ef)
 }
 #endif
 
+/*
+ * this should only be called when the cache lock is already held
+ * (We could drop this restriction if we add a parameter to eet_test_close
+ * that indicates if the lock is held or not.  For now it is easiest
+ * to just require that it is always held.)
+ */
 static Eet_File *
 eet_internal_read(Eet_File *ef)
 {
@@ -1302,7 +1312,7 @@ eet_internal_read(Eet_File *ef)
        return eet_internal_read2(ef);
       default:
        ef->delete_me_now = 1;
-       eet_close(ef);
+       eet_internal_close(ef, EINA_TRUE);
        break;
      }
 
@@ -1317,10 +1327,13 @@ eet_internal_close(Eet_File *ef, Eina_Bool locked)
    /* check to see its' an eet file pointer */
    if (eet_check_pointer(ef))
      return EET_ERROR_BAD_OBJECT;
+
+   if (!locked) LOCK_CACHE;
+
    /* deref */
    ef->references--;
    /* if its still referenced - dont go any further */
-   if (ef->references > 0) return EET_ERROR_NONE;
+   if (ef->references > 0) goto on_error;
    /* flush any writes */
    err = eet_flush2(ef);
 
@@ -1329,21 +1342,16 @@ eet_internal_close(Eet_File *ef, Eina_Bool locked)
 
    /* if not urgent to delete it - dont free it - leave it in cache */
    if ((!ef->delete_me_now) && (ef->mode == EET_FILE_MODE_READ))
-     return EET_ERROR_NONE;
+     goto on_error;
 
    /* remove from cache */
-   if (!locked)
-     {
-        LOCK_CACHE;
-     }
    if (ef->mode == EET_FILE_MODE_READ)
      eet_cache_del(ef, &eet_readers, &eet_readers_num, &eet_readers_alloc);
    else if ((ef->mode == EET_FILE_MODE_WRITE) || (ef->mode == EET_FILE_MODE_READ_WRITE))
      eet_cache_del(ef, &eet_writers, &eet_writers_num, &eet_writers_alloc);
-   if (!locked)
-     {
-        UNLOCK_CACHE;
-     }
+
+   /* we can unlock the cache now */
+   if (!locked) UNLOCK_CACHE;
 
    DESTROY_FILE(ef);
 
@@ -1394,6 +1402,10 @@ eet_internal_close(Eet_File *ef, Eina_Bool locked)
    /* free it */
    free(ef);
    return err;
+
+ on_error:
+   if (!locked) UNLOCK_CACHE;
+   return EET_ERROR_NONE;
 }
 
 EAPI Eet_File *
@@ -1425,7 +1437,11 @@ eet_memopen_read(const void *data, size_t size)
    ef->sha1 = NULL;
    ef->sha1_length = 0;
 
-   return eet_internal_read(ef);
+   /* eet_internal_read expects the cache lock to be held when it is called */
+   LOCK_CACHE;
+   ef = eet_internal_read(ef);
+   UNLOCK_CACHE;
+   return ef;
 }
 
 EAPI Eet_File *
@@ -1450,7 +1466,7 @@ eet_open(const char *file, Eet_File_Mode mode)
             eet_flush2(ef);
             ef->references++;
             ef->delete_me_now = 1;
-             eet_internal_close(ef, 1);
+             eet_internal_close(ef, EINA_TRUE);
          }
        ef = eet_cache_find((char *)file, eet_readers, eet_readers_num);
      }
@@ -1462,11 +1478,10 @@ eet_open(const char *file, Eet_File_Mode mode)
          {
             ef->delete_me_now = 1;
             ef->references++;
-             eet_internal_close(ef, 1);
+             eet_internal_close(ef, EINA_TRUE);
          }
        ef = eet_cache_find((char *)file, eet_writers, eet_writers_num);
      }
-   UNLOCK_CACHE;
    
     /* try open the file based on mode */
    if ((mode == EET_FILE_MODE_READ) || (mode == EET_FILE_MODE_READ_WRITE))
@@ -1475,23 +1490,23 @@ eet_open(const char *file, Eet_File_Mode mode)
        file_stat.st_mtime = 0;
 
        fp = fopen(file, "rb");
-       if (!fp) goto on_error;
+       if (!fp) goto open_error;
        if (fstat(fileno(fp), &file_stat))
          {
             fclose(fp);
             fp = NULL;
-            goto on_error;
+            goto open_error;
          }
        if ((mode == EET_FILE_MODE_READ) &&
            (file_stat.st_size < ((int) sizeof(int) * 3)))
          {
             fclose(fp);
             fp = NULL;
-            goto on_error;
+            goto open_error;
          }
 
-     on_error:
-       if (fp == NULL && mode == EET_FILE_MODE_READ) return NULL;
+     open_error:
+       if (fp == NULL && mode == EET_FILE_MODE_READ) goto on_error;
      }
    else
      {
@@ -1503,7 +1518,7 @@ eet_open(const char *file, Eet_File_Mode mode)
        unlink(file);
        fd = open(file, O_CREAT | O_TRUNC | O_RDWR, S_IRUSR | S_IWUSR);
        fp = fdopen(fd, "wb");
-       if (!fp) return NULL;
+       if (!fp) goto on_error;
      }
 
    /* We found one */
@@ -1511,7 +1526,7 @@ eet_open(const char *file, Eet_File_Mode mode)
      {
        ef->delete_me_now = 1;
        ef->references++;
-        eet_internal_close(ef, 0);
+        eet_internal_close(ef, EINA_FALSE);
        ef = NULL;
      }
 
@@ -1520,6 +1535,7 @@ eet_open(const char *file, Eet_File_Mode mode)
        /* reference it up and return it */
        if (fp != NULL) fclose(fp);
        ef->references++;
+       UNLOCK_CACHE;
        return ef;
      }
 
@@ -1528,7 +1544,7 @@ eet_open(const char *file, Eet_File_Mode mode)
    /* Allocate struct for eet file and have it zero'd out */
    ef = malloc(sizeof(Eet_File) + file_len);
    if (!ef)
-     return NULL;
+     goto on_error;
 
    /* fill some of the members */
    INIT_FILE(ef);
@@ -1557,7 +1573,7 @@ eet_open(const char *file, Eet_File_Mode mode)
 
    /* if we can't open - bail out */
    if (eet_test_close(!ef->fp, ef))
-     return NULL;
+     goto on_error;
 
    fcntl(fileno(ef->fp), F_SETFD, FD_CLOEXEC);
    /* if we opened for read or read-write */
@@ -1567,10 +1583,10 @@ eet_open(const char *file, Eet_File_Mode mode)
        ef->data = mmap(NULL, ef->data_size, PROT_READ,
                        MAP_SHARED, fileno(ef->fp), 0);
        if (eet_test_close((ef->data == MAP_FAILED), ef))
-         return NULL;
+         goto on_error;
        ef = eet_internal_read(ef);
        if (!ef)
-         return NULL;
+         goto on_error;
      }
 
  empty_file:
@@ -1584,16 +1600,19 @@ eet_open(const char *file, Eet_File_Mode mode)
    /* add to cache */
    if (ef->references == 1)
      {
-       LOCK_CACHE;
        if (ef->mode == EET_FILE_MODE_READ)
          eet_cache_add(ef, &eet_readers, &eet_readers_num, &eet_readers_alloc);
        else
          if ((ef->mode == EET_FILE_MODE_WRITE) || (ef->mode == EET_FILE_MODE_READ_WRITE))
            eet_cache_add(ef, &eet_writers, &eet_writers_num, &eet_writers_alloc);
-       UNLOCK_CACHE;
      }
 
+   UNLOCK_CACHE;
    return ef;
+
+on_error:
+   UNLOCK_CACHE;
+   return NULL;
 }
 
 EAPI Eet_File_Mode
@@ -1654,7 +1673,7 @@ eet_identity_set(Eet_File *ef, Eet_Key *key)
 EAPI Eet_Error
 eet_close(Eet_File *ef)
 {
-   return eet_internal_close(ef, 0);
+   return eet_internal_close(ef, EINA_FALSE);
 }
 
 EAPI void *

diff --git a/src/tests/eet_suite.c b/src/tests/eet_suite.c
index a41e8f7..0faa01b 100644 (file)
--- a/src/tests/eet_suite.c
+++ b/src/tests/eet_suite.c
@@ -6,6 +6,7 @@
 #include <stdio.h>
 #include <fcntl.h>
 #include <unistd.h>
+#include <pthread.h>
 
 #include <check.h>
 
@@ -1410,6 +1411,65 @@ START_TEST(eet_cipher_decipher_simple)
 }
 END_TEST
 
+static Eina_Bool open_worker_stop;
+static void*
+open_close_worker(void* path)
+{
+   while (!open_worker_stop)
+     {
+       Eet_File* ef = eet_open((char const*)path, EET_FILE_MODE_READ);
+       if (ef == NULL)
+         {
+            pthread_exit("eet_open() failed");
+         }
+       else
+         {
+            Eet_Error err_code = eet_close(ef);
+            if (err_code != EET_ERROR_NONE)
+              pthread_exit("eet_close() failed");
+         }
+     }
+
+   pthread_exit(NULL);
+}
+
+START_TEST(eet_cache_concurrency)
+{
+   char *file = strdup("/tmp/eet_suite_testXXXXXX");
+   const char *buffer = "test data";
+   Eet_File *ef;
+   void *thread_ret;
+   unsigned int n;
+
+   eet_init();
+
+   /* create a file to test with */
+   fail_if(!mktemp(file));
+   ef = eet_open(file, EET_FILE_MODE_WRITE);
+   fail_if(!ef);
+   fail_if(!eet_write(ef, "keys/tests", buffer, strlen(buffer) + 1, 0));
+
+   /* start a thread that repeatedly opens and closes a file */
+   open_worker_stop = 0;
+   pthread_t thread;
+   pthread_create(&thread, NULL, open_close_worker, file);
+
+   /* clear the cache repeatedly in this thread */
+   for (n = 0; n < 100000; ++n)
+     {
+       eet_clearcache();
+     }
+
+   /* join the other thread, and fail if it returned an error message */
+   open_worker_stop = 1;
+   fail_if(pthread_join(thread, &thread_ret) != 0);
+   fail_unless(thread_ret == NULL, (char const*)thread_ret);
+
+   fail_if(unlink(file) != 0);
+   eet_shutdown();
+}
+END_TEST
+
 
 Suite *
 eet_suite(void)
@@ -1455,6 +1515,10 @@ eet_suite(void)
    suite_add_tcase(s, tc);
 #endif
 
+   tc = tcase_create("Eet Cache");
+   tcase_add_test(tc, eet_cache_concurrency);
+   suite_add_tcase(s, tc);
+
    return s;
 }