From 73a9612f2c320cc6779da7ab3063f89cdf920169 Mon Sep 17 00:00:00 2001 From: Carsten Haitzler Date: Sun, 7 Mar 2010 03:01:55 +0000 Subject: [PATCH] also revert - on3e of these 2 - or both, causes a deadlock in e. see my previous commit log. SVN revision: 46918 --- legacy/eet/AUTHORS | 1 - legacy/eet/ChangeLog | 4 -- legacy/eet/src/lib/eet_lib.c | 63 +++++++++++-------------------- legacy/eet/src/tests/eet_suite.c | 64 -------------------------------- 4 files changed, 21 insertions(+), 111 deletions(-) diff --git a/legacy/eet/AUTHORS b/legacy/eet/AUTHORS index d1172a0e26..cd5fc41184 100644 --- a/legacy/eet/AUTHORS +++ b/legacy/eet/AUTHORS @@ -9,4 +9,3 @@ Gustavo Sverzut Barbieri Raphael Kubo da Costa Mathieu Taillefumier Albin "Lutin" Tonnerre -Adam Simpkins diff --git a/legacy/eet/ChangeLog b/legacy/eet/ChangeLog index 093d030808..f71ff865c8 100644 --- a/legacy/eet/ChangeLog +++ b/legacy/eet/ChangeLog @@ -337,7 +337,3 @@ 2010-03-01 Albin Tonnerre * Fix override of global symbols. - -2010-03-05 Adam Simpkins - - * Fix clearcache race condition. diff --git a/legacy/eet/src/lib/eet_lib.c b/legacy/eet/src/lib/eet_lib.c index 4ed8432f60..8fed189666 100644 --- a/legacy/eet/src/lib/eet_lib.c +++ b/legacy/eet/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, 1); + eet_internal_close(ef, 0); } return test; } @@ -307,7 +307,6 @@ 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) { @@ -359,7 +358,6 @@ 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) { @@ -851,7 +849,6 @@ 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++; @@ -890,10 +887,9 @@ eet_clearcache(void) for (i = 0; i < num; i++) { - eet_internal_close(closelist[i], 1); + eet_close(closelist[i]); } } - UNLOCK_CACHE; } /* FIXME: MMAP race condition in READ_WRITE_MODE */ @@ -1285,12 +1281,6 @@ 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) { @@ -1327,9 +1317,6 @@ 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 */ @@ -1342,18 +1329,17 @@ 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)) - { - if (!locked) UNLOCK_CACHE; - return EET_ERROR_NONE; - } + return EET_ERROR_NONE; /* 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); - - /* we can unlock the cache now */ if (!locked) { UNLOCK_CACHE; @@ -1439,11 +1425,7 @@ eet_memopen_read(const void *data, size_t size) ef->sha1 = NULL; ef->sha1_length = 0; - /* 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; + return eet_internal_read(ef); } EAPI Eet_File * @@ -1484,6 +1466,7 @@ eet_open(const char *file, Eet_File_Mode mode) } 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)) @@ -1492,23 +1475,23 @@ eet_open(const char *file, Eet_File_Mode mode) file_stat.st_mtime = 0; fp = fopen(file, "rb"); - if (!fp) goto open_error; + if (!fp) goto on_error; if (fstat(fileno(fp), &file_stat)) { fclose(fp); fp = NULL; - goto open_error; + goto on_error; } if ((mode == EET_FILE_MODE_READ) && (file_stat.st_size < ((int) sizeof(int) * 3))) { fclose(fp); fp = NULL; - goto open_error; + goto on_error; } - open_error: - if (fp == NULL && mode == EET_FILE_MODE_READ) goto on_error; + on_error: + if (fp == NULL && mode == EET_FILE_MODE_READ) return NULL; } else { @@ -1520,7 +1503,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) goto on_error; + if (!fp) return NULL; } /* We found one */ @@ -1537,7 +1520,6 @@ 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; } @@ -1546,7 +1528,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) - goto on_error; + return NULL; /* fill some of the members */ INIT_FILE(ef); @@ -1575,7 +1557,7 @@ eet_open(const char *file, Eet_File_Mode mode) /* if we can't open - bail out */ if (eet_test_close(!ef->fp, ef)) - goto on_error; + return NULL; fcntl(fileno(ef->fp), F_SETFD, FD_CLOEXEC); /* if we opened for read or read-write */ @@ -1585,10 +1567,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)) - goto on_error; + return NULL; ef = eet_internal_read(ef); if (!ef) - goto on_error; + return NULL; } empty_file: @@ -1602,19 +1584,16 @@ 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 diff --git a/legacy/eet/src/tests/eet_suite.c b/legacy/eet/src/tests/eet_suite.c index 0faa01b883..a41e8f7d23 100644 --- a/legacy/eet/src/tests/eet_suite.c +++ b/legacy/eet/src/tests/eet_suite.c @@ -6,7 +6,6 @@ #include #include #include -#include #include @@ -1411,65 +1410,6 @@ 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) @@ -1515,10 +1455,6 @@ 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; }