forked from enlightenment/efl
* eet: Fix clearcache race condition.
Patch by Adam Simpkins. SVN revision: 46891
This commit is contained in:
parent
bf056dd2df
commit
c285985eeb
|
@ -9,3 +9,4 @@ Gustavo Sverzut Barbieri <barbieri@gmail.com>
|
||||||
Raphael Kubo da Costa <kubo@profusion.mobi>
|
Raphael Kubo da Costa <kubo@profusion.mobi>
|
||||||
Mathieu Taillefumier <mathieu.taillefumier@free.fr>
|
Mathieu Taillefumier <mathieu.taillefumier@free.fr>
|
||||||
Albin "Lutin" Tonnerre <albin.tonnerre@gmail.com>
|
Albin "Lutin" Tonnerre <albin.tonnerre@gmail.com>
|
||||||
|
Adam Simpkins <adam@adamsimpkins.net>
|
||||||
|
|
|
@ -337,3 +337,7 @@
|
||||||
2010-03-01 Albin Tonnerre
|
2010-03-01 Albin Tonnerre
|
||||||
|
|
||||||
* Fix override of global symbols.
|
* Fix override of global symbols.
|
||||||
|
|
||||||
|
2010-03-05 Adam Simpkins
|
||||||
|
|
||||||
|
* Fix clearcache race condition.
|
||||||
|
|
|
@ -280,7 +280,7 @@ eet_test_close(int test, Eet_File *ef)
|
||||||
if (test)
|
if (test)
|
||||||
{
|
{
|
||||||
ef->delete_me_now = 1;
|
ef->delete_me_now = 1;
|
||||||
eet_internal_close(ef, 0);
|
eet_internal_close(ef, 1);
|
||||||
}
|
}
|
||||||
return test;
|
return test;
|
||||||
}
|
}
|
||||||
|
@ -307,6 +307,7 @@ eet_cache_find(const char *path, Eet_File **cache, int cache_num)
|
||||||
}
|
}
|
||||||
|
|
||||||
/* add to end of cache */
|
/* add to end of cache */
|
||||||
|
/* this should only be called when the cache lock is already held */
|
||||||
static void
|
static void
|
||||||
eet_cache_add(Eet_File *ef, Eet_File ***cache, int *cache_num, int *cache_alloc)
|
eet_cache_add(Eet_File *ef, Eet_File ***cache, int *cache_num, int *cache_alloc)
|
||||||
{
|
{
|
||||||
|
@ -358,6 +359,7 @@ eet_cache_add(Eet_File *ef, Eet_File ***cache, int *cache_num, int *cache_alloc)
|
||||||
}
|
}
|
||||||
|
|
||||||
/* delete from cache */
|
/* delete from cache */
|
||||||
|
/* this should only be called when the cache lock is already held */
|
||||||
static void
|
static void
|
||||||
eet_cache_del(Eet_File *ef, Eet_File ***cache, int *cache_num, int *cache_alloc)
|
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,
|
* 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.
|
* due to eet_close removing them from the cache after each call.
|
||||||
*/
|
*/
|
||||||
|
LOCK_CACHE;
|
||||||
for (i = 0; i < eet_writers_num; i++)
|
for (i = 0; i < eet_writers_num; i++)
|
||||||
{
|
{
|
||||||
if (eet_writers[i]->references <= 0) num++;
|
if (eet_writers[i]->references <= 0) num++;
|
||||||
|
@ -887,9 +890,10 @@ eet_clearcache(void)
|
||||||
|
|
||||||
for (i = 0; i < num; i++)
|
for (i = 0; i < num; i++)
|
||||||
{
|
{
|
||||||
eet_close(closelist[i]);
|
eet_internal_close(closelist[i], 1);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
UNLOCK_CACHE;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* FIXME: MMAP race condition in READ_WRITE_MODE */
|
/* FIXME: MMAP race condition in READ_WRITE_MODE */
|
||||||
|
@ -1281,6 +1285,12 @@ eet_internal_read1(Eet_File *ef)
|
||||||
}
|
}
|
||||||
#endif
|
#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 *
|
static Eet_File *
|
||||||
eet_internal_read(Eet_File *ef)
|
eet_internal_read(Eet_File *ef)
|
||||||
{
|
{
|
||||||
|
@ -1317,6 +1327,9 @@ eet_internal_close(Eet_File *ef, Eina_Bool locked)
|
||||||
/* check to see its' an eet file pointer */
|
/* check to see its' an eet file pointer */
|
||||||
if (eet_check_pointer(ef))
|
if (eet_check_pointer(ef))
|
||||||
return EET_ERROR_BAD_OBJECT;
|
return EET_ERROR_BAD_OBJECT;
|
||||||
|
|
||||||
|
if (!locked) LOCK_CACHE;
|
||||||
|
|
||||||
/* deref */
|
/* deref */
|
||||||
ef->references--;
|
ef->references--;
|
||||||
/* if its still referenced - dont go any further */
|
/* if its still referenced - dont go any further */
|
||||||
|
@ -1329,17 +1342,18 @@ eet_internal_close(Eet_File *ef, Eina_Bool locked)
|
||||||
|
|
||||||
/* if not urgent to delete it - dont free it - leave it in cache */
|
/* 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 ((!ef->delete_me_now) && (ef->mode == EET_FILE_MODE_READ))
|
||||||
return EET_ERROR_NONE;
|
{
|
||||||
|
if (!locked) UNLOCK_CACHE;
|
||||||
|
return EET_ERROR_NONE;
|
||||||
|
}
|
||||||
|
|
||||||
/* remove from cache */
|
/* remove from cache */
|
||||||
if (!locked)
|
|
||||||
{
|
|
||||||
LOCK_CACHE;
|
|
||||||
}
|
|
||||||
if (ef->mode == EET_FILE_MODE_READ)
|
if (ef->mode == EET_FILE_MODE_READ)
|
||||||
eet_cache_del(ef, &eet_readers, &eet_readers_num, &eet_readers_alloc);
|
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))
|
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);
|
eet_cache_del(ef, &eet_writers, &eet_writers_num, &eet_writers_alloc);
|
||||||
|
|
||||||
|
/* we can unlock the cache now */
|
||||||
if (!locked)
|
if (!locked)
|
||||||
{
|
{
|
||||||
UNLOCK_CACHE;
|
UNLOCK_CACHE;
|
||||||
|
@ -1425,7 +1439,11 @@ eet_memopen_read(const void *data, size_t size)
|
||||||
ef->sha1 = NULL;
|
ef->sha1 = NULL;
|
||||||
ef->sha1_length = 0;
|
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 *
|
EAPI Eet_File *
|
||||||
|
@ -1466,7 +1484,6 @@ eet_open(const char *file, Eet_File_Mode mode)
|
||||||
}
|
}
|
||||||
ef = eet_cache_find((char *)file, eet_writers, eet_writers_num);
|
ef = eet_cache_find((char *)file, eet_writers, eet_writers_num);
|
||||||
}
|
}
|
||||||
UNLOCK_CACHE;
|
|
||||||
|
|
||||||
/* try open the file based on mode */
|
/* try open the file based on mode */
|
||||||
if ((mode == EET_FILE_MODE_READ) || (mode == EET_FILE_MODE_READ_WRITE))
|
if ((mode == EET_FILE_MODE_READ) || (mode == EET_FILE_MODE_READ_WRITE))
|
||||||
|
@ -1475,23 +1492,23 @@ eet_open(const char *file, Eet_File_Mode mode)
|
||||||
file_stat.st_mtime = 0;
|
file_stat.st_mtime = 0;
|
||||||
|
|
||||||
fp = fopen(file, "rb");
|
fp = fopen(file, "rb");
|
||||||
if (!fp) goto on_error;
|
if (!fp) goto open_error;
|
||||||
if (fstat(fileno(fp), &file_stat))
|
if (fstat(fileno(fp), &file_stat))
|
||||||
{
|
{
|
||||||
fclose(fp);
|
fclose(fp);
|
||||||
fp = NULL;
|
fp = NULL;
|
||||||
goto on_error;
|
goto open_error;
|
||||||
}
|
}
|
||||||
if ((mode == EET_FILE_MODE_READ) &&
|
if ((mode == EET_FILE_MODE_READ) &&
|
||||||
(file_stat.st_size < ((int) sizeof(int) * 3)))
|
(file_stat.st_size < ((int) sizeof(int) * 3)))
|
||||||
{
|
{
|
||||||
fclose(fp);
|
fclose(fp);
|
||||||
fp = NULL;
|
fp = NULL;
|
||||||
goto on_error;
|
goto open_error;
|
||||||
}
|
}
|
||||||
|
|
||||||
on_error:
|
open_error:
|
||||||
if (fp == NULL && mode == EET_FILE_MODE_READ) return NULL;
|
if (fp == NULL && mode == EET_FILE_MODE_READ) goto on_error;
|
||||||
}
|
}
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
|
@ -1503,7 +1520,7 @@ eet_open(const char *file, Eet_File_Mode mode)
|
||||||
unlink(file);
|
unlink(file);
|
||||||
fd = open(file, O_CREAT | O_TRUNC | O_RDWR, S_IRUSR | S_IWUSR);
|
fd = open(file, O_CREAT | O_TRUNC | O_RDWR, S_IRUSR | S_IWUSR);
|
||||||
fp = fdopen(fd, "wb");
|
fp = fdopen(fd, "wb");
|
||||||
if (!fp) return NULL;
|
if (!fp) goto on_error;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* We found one */
|
/* We found one */
|
||||||
|
@ -1520,6 +1537,7 @@ eet_open(const char *file, Eet_File_Mode mode)
|
||||||
/* reference it up and return it */
|
/* reference it up and return it */
|
||||||
if (fp != NULL) fclose(fp);
|
if (fp != NULL) fclose(fp);
|
||||||
ef->references++;
|
ef->references++;
|
||||||
|
UNLOCK_CACHE;
|
||||||
return ef;
|
return ef;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1528,7 +1546,7 @@ eet_open(const char *file, Eet_File_Mode mode)
|
||||||
/* Allocate struct for eet file and have it zero'd out */
|
/* Allocate struct for eet file and have it zero'd out */
|
||||||
ef = malloc(sizeof(Eet_File) + file_len);
|
ef = malloc(sizeof(Eet_File) + file_len);
|
||||||
if (!ef)
|
if (!ef)
|
||||||
return NULL;
|
goto on_error;
|
||||||
|
|
||||||
/* fill some of the members */
|
/* fill some of the members */
|
||||||
INIT_FILE(ef);
|
INIT_FILE(ef);
|
||||||
|
@ -1557,7 +1575,7 @@ eet_open(const char *file, Eet_File_Mode mode)
|
||||||
|
|
||||||
/* if we can't open - bail out */
|
/* if we can't open - bail out */
|
||||||
if (eet_test_close(!ef->fp, ef))
|
if (eet_test_close(!ef->fp, ef))
|
||||||
return NULL;
|
goto on_error;
|
||||||
|
|
||||||
fcntl(fileno(ef->fp), F_SETFD, FD_CLOEXEC);
|
fcntl(fileno(ef->fp), F_SETFD, FD_CLOEXEC);
|
||||||
/* if we opened for read or read-write */
|
/* if we opened for read or read-write */
|
||||||
|
@ -1567,10 +1585,10 @@ eet_open(const char *file, Eet_File_Mode mode)
|
||||||
ef->data = mmap(NULL, ef->data_size, PROT_READ,
|
ef->data = mmap(NULL, ef->data_size, PROT_READ,
|
||||||
MAP_SHARED, fileno(ef->fp), 0);
|
MAP_SHARED, fileno(ef->fp), 0);
|
||||||
if (eet_test_close((ef->data == MAP_FAILED), ef))
|
if (eet_test_close((ef->data == MAP_FAILED), ef))
|
||||||
return NULL;
|
goto on_error;
|
||||||
ef = eet_internal_read(ef);
|
ef = eet_internal_read(ef);
|
||||||
if (!ef)
|
if (!ef)
|
||||||
return NULL;
|
goto on_error;
|
||||||
}
|
}
|
||||||
|
|
||||||
empty_file:
|
empty_file:
|
||||||
|
@ -1584,16 +1602,19 @@ eet_open(const char *file, Eet_File_Mode mode)
|
||||||
/* add to cache */
|
/* add to cache */
|
||||||
if (ef->references == 1)
|
if (ef->references == 1)
|
||||||
{
|
{
|
||||||
LOCK_CACHE;
|
|
||||||
if (ef->mode == EET_FILE_MODE_READ)
|
if (ef->mode == EET_FILE_MODE_READ)
|
||||||
eet_cache_add(ef, &eet_readers, &eet_readers_num, &eet_readers_alloc);
|
eet_cache_add(ef, &eet_readers, &eet_readers_num, &eet_readers_alloc);
|
||||||
else
|
else
|
||||||
if ((ef->mode == EET_FILE_MODE_WRITE) || (ef->mode == EET_FILE_MODE_READ_WRITE))
|
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);
|
eet_cache_add(ef, &eet_writers, &eet_writers_num, &eet_writers_alloc);
|
||||||
UNLOCK_CACHE;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
UNLOCK_CACHE;
|
||||||
return ef;
|
return ef;
|
||||||
|
|
||||||
|
on_error:
|
||||||
|
UNLOCK_CACHE;
|
||||||
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
EAPI Eet_File_Mode
|
EAPI Eet_File_Mode
|
||||||
|
|
|
@ -6,6 +6,7 @@
|
||||||
#include <stdio.h>
|
#include <stdio.h>
|
||||||
#include <fcntl.h>
|
#include <fcntl.h>
|
||||||
#include <unistd.h>
|
#include <unistd.h>
|
||||||
|
#include <pthread.h>
|
||||||
|
|
||||||
#include <check.h>
|
#include <check.h>
|
||||||
|
|
||||||
|
@ -1410,6 +1411,65 @@ START_TEST(eet_cipher_decipher_simple)
|
||||||
}
|
}
|
||||||
END_TEST
|
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 *
|
Suite *
|
||||||
eet_suite(void)
|
eet_suite(void)
|
||||||
|
@ -1455,6 +1515,10 @@ eet_suite(void)
|
||||||
suite_add_tcase(s, tc);
|
suite_add_tcase(s, tc);
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
tc = tcase_create("Eet Cache");
|
||||||
|
tcase_add_test(tc, eet_cache_concurrency);
|
||||||
|
suite_add_tcase(s, tc);
|
||||||
|
|
||||||
return s;
|
return s;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue