forked from lda/telodendria
Fix race conditions that are possible in Db.
This is accomplished by locking the entire database, and keeping it locked until the last reference is unlocked. We get rid of per-reference locks, because those are what cause race conditions.
This commit is contained in:
parent
313249ca88
commit
279f261aed
1 changed files with 28 additions and 37 deletions
63
src/Db.c
63
src/Db.c
|
@ -75,7 +75,6 @@ struct Db
|
||||||
struct DbRef
|
struct DbRef
|
||||||
{
|
{
|
||||||
HashMap *json;
|
HashMap *json;
|
||||||
pthread_mutex_t lock;
|
|
||||||
|
|
||||||
unsigned long ts;
|
unsigned long ts;
|
||||||
size_t size;
|
size_t size;
|
||||||
|
@ -290,16 +289,7 @@ DbCacheEvict(Db * db)
|
||||||
{
|
{
|
||||||
char *hash;
|
char *hash;
|
||||||
|
|
||||||
if (pthread_mutex_trylock(&ref->lock) != 0)
|
|
||||||
{
|
|
||||||
/* This ref is locked by another thread, don't evict it. */
|
|
||||||
ref = ref->next;
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
|
|
||||||
JsonFree(ref->json);
|
JsonFree(ref->json);
|
||||||
pthread_mutex_unlock(&ref->lock);
|
|
||||||
pthread_mutex_destroy(&ref->lock);
|
|
||||||
|
|
||||||
hash = DbHashKey(ref->name);
|
hash = DbHashKey(ref->name);
|
||||||
HashMapDelete(db->cache, hash);
|
HashMapDelete(db->cache, hash);
|
||||||
|
@ -338,6 +328,7 @@ Db *
|
||||||
DbOpen(char *dir, size_t cache)
|
DbOpen(char *dir, size_t cache)
|
||||||
{
|
{
|
||||||
Db *db;
|
Db *db;
|
||||||
|
pthread_mutexattr_t attr;
|
||||||
|
|
||||||
if (!dir)
|
if (!dir)
|
||||||
{
|
{
|
||||||
|
@ -353,7 +344,10 @@ DbOpen(char *dir, size_t cache)
|
||||||
db->dir = dir;
|
db->dir = dir;
|
||||||
db->maxCache = cache;
|
db->maxCache = cache;
|
||||||
|
|
||||||
pthread_mutex_init(&db->lock, NULL);
|
pthread_mutexattr_init(&attr);
|
||||||
|
pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
|
||||||
|
pthread_mutex_init(&db->lock, &attr);
|
||||||
|
pthread_mutexattr_destroy(&attr);
|
||||||
|
|
||||||
db->mostRecent = NULL;
|
db->mostRecent = NULL;
|
||||||
db->leastRecent = NULL;
|
db->leastRecent = NULL;
|
||||||
|
@ -383,6 +377,8 @@ DbMaxCacheSet(Db * db, size_t cache)
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pthread_mutex_lock(&db->lock);
|
||||||
|
|
||||||
db->maxCache = cache;
|
db->maxCache = cache;
|
||||||
if (db->maxCache && !db->cache)
|
if (db->maxCache && !db->cache)
|
||||||
{
|
{
|
||||||
|
@ -391,6 +387,8 @@ DbMaxCacheSet(Db * db, size_t cache)
|
||||||
}
|
}
|
||||||
|
|
||||||
DbCacheEvict(db);
|
DbCacheEvict(db);
|
||||||
|
|
||||||
|
pthread_mutex_unlock(&db->lock);
|
||||||
}
|
}
|
||||||
|
|
||||||
void
|
void
|
||||||
|
@ -401,10 +399,13 @@ DbClose(Db * db)
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pthread_mutex_lock(&db->lock);
|
||||||
|
|
||||||
DbMaxCacheSet(db, 0);
|
DbMaxCacheSet(db, 0);
|
||||||
DbCacheEvict(db);
|
DbCacheEvict(db);
|
||||||
HashMapFree(db->cache);
|
HashMapFree(db->cache);
|
||||||
|
|
||||||
|
pthread_mutex_unlock(&db->lock);
|
||||||
pthread_mutex_destroy(&db->lock);
|
pthread_mutex_destroy(&db->lock);
|
||||||
|
|
||||||
Free(db);
|
Free(db);
|
||||||
|
@ -441,8 +442,6 @@ DbLockFromArr(Db * db, Array * args)
|
||||||
{
|
{
|
||||||
if (ref)
|
if (ref)
|
||||||
{
|
{
|
||||||
pthread_mutex_lock(&ref->lock);
|
|
||||||
|
|
||||||
HashMapDelete(db->cache, hash);
|
HashMapDelete(db->cache, hash);
|
||||||
JsonFree(ref->json);
|
JsonFree(ref->json);
|
||||||
StringArrayFree(ref->name);
|
StringArrayFree(ref->name);
|
||||||
|
@ -472,8 +471,6 @@ DbLockFromArr(Db * db, Array * args)
|
||||||
db->leastRecent = db->mostRecent;
|
db->leastRecent = db->mostRecent;
|
||||||
}
|
}
|
||||||
|
|
||||||
pthread_mutex_unlock(&ref->lock);
|
|
||||||
pthread_mutex_destroy(&ref->lock);
|
|
||||||
Free(ref);
|
Free(ref);
|
||||||
}
|
}
|
||||||
ref = NULL;
|
ref = NULL;
|
||||||
|
@ -499,8 +496,6 @@ DbLockFromArr(Db * db, Array * args)
|
||||||
{
|
{
|
||||||
unsigned long diskTs = UtilLastModified(file);
|
unsigned long diskTs = UtilLastModified(file);
|
||||||
|
|
||||||
pthread_mutex_lock(&ref->lock);
|
|
||||||
|
|
||||||
ref->fd = fd;
|
ref->fd = fd;
|
||||||
ref->stream = stream;
|
ref->stream = stream;
|
||||||
|
|
||||||
|
@ -511,7 +506,6 @@ DbLockFromArr(Db * db, Array * args)
|
||||||
|
|
||||||
if (!json)
|
if (!json)
|
||||||
{
|
{
|
||||||
pthread_mutex_unlock(&ref->lock);
|
|
||||||
StreamClose(ref->stream);
|
StreamClose(ref->stream);
|
||||||
ref = NULL;
|
ref = NULL;
|
||||||
goto finish;
|
goto finish;
|
||||||
|
@ -585,9 +579,6 @@ DbLockFromArr(Db * db, Array * args)
|
||||||
ref->fd = fd;
|
ref->fd = fd;
|
||||||
ref->stream = stream;
|
ref->stream = stream;
|
||||||
|
|
||||||
pthread_mutex_init(&ref->lock, NULL);
|
|
||||||
pthread_mutex_lock(&ref->lock);
|
|
||||||
|
|
||||||
name = ArrayCreate();
|
name = ArrayCreate();
|
||||||
for (i = 0; i < ArraySize(args); i++)
|
for (i = 0; i < ArraySize(args); i++)
|
||||||
{
|
{
|
||||||
|
@ -622,7 +613,10 @@ DbLockFromArr(Db * db, Array * args)
|
||||||
}
|
}
|
||||||
|
|
||||||
finish:
|
finish:
|
||||||
|
if (!ref)
|
||||||
|
{
|
||||||
pthread_mutex_unlock(&db->lock);
|
pthread_mutex_unlock(&db->lock);
|
||||||
|
}
|
||||||
|
|
||||||
Free(file);
|
Free(file);
|
||||||
Free(hash);
|
Free(hash);
|
||||||
|
@ -654,12 +648,15 @@ DbCreate(Db * db, size_t nArgs,...)
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pthread_mutex_lock(&db->lock);
|
||||||
|
|
||||||
file = DbFileName(db, args);
|
file = DbFileName(db, args);
|
||||||
|
|
||||||
if (UtilLastModified(file))
|
if (UtilLastModified(file))
|
||||||
{
|
{
|
||||||
Free(file);
|
Free(file);
|
||||||
ArrayFree(args);
|
ArrayFree(args);
|
||||||
|
pthread_mutex_unlock(&db->lock);
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -669,6 +666,7 @@ DbCreate(Db * db, size_t nArgs,...)
|
||||||
Free(file);
|
Free(file);
|
||||||
ArrayFree(args);
|
ArrayFree(args);
|
||||||
Free(dir);
|
Free(dir);
|
||||||
|
pthread_mutex_unlock(&db->lock);
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -679,12 +677,16 @@ DbCreate(Db * db, size_t nArgs,...)
|
||||||
if (!fp)
|
if (!fp)
|
||||||
{
|
{
|
||||||
ArrayFree(args);
|
ArrayFree(args);
|
||||||
|
pthread_mutex_unlock(&db->lock);
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
|
|
||||||
StreamPuts(fp, "{}");
|
StreamPuts(fp, "{}");
|
||||||
StreamClose(fp);
|
StreamClose(fp);
|
||||||
|
|
||||||
|
/* DbLockFromArr() will lock again for us */
|
||||||
|
pthread_mutex_unlock(&db->lock);
|
||||||
|
|
||||||
ret = DbLockFromArr(db, args);
|
ret = DbLockFromArr(db, args);
|
||||||
|
|
||||||
ArrayFree(args);
|
ArrayFree(args);
|
||||||
|
@ -719,8 +721,6 @@ DbDelete(Db * db, size_t nArgs,...)
|
||||||
ref = HashMapGet(db->cache, hash);
|
ref = HashMapGet(db->cache, hash);
|
||||||
if (ref)
|
if (ref)
|
||||||
{
|
{
|
||||||
pthread_mutex_lock(&ref->lock);
|
|
||||||
|
|
||||||
HashMapDelete(db->cache, hash);
|
HashMapDelete(db->cache, hash);
|
||||||
JsonFree(ref->json);
|
JsonFree(ref->json);
|
||||||
StringArrayFree(ref->name);
|
StringArrayFree(ref->name);
|
||||||
|
@ -750,8 +750,6 @@ DbDelete(Db * db, size_t nArgs,...)
|
||||||
db->leastRecent = db->mostRecent;
|
db->leastRecent = db->mostRecent;
|
||||||
}
|
}
|
||||||
|
|
||||||
pthread_mutex_unlock(&ref->lock);
|
|
||||||
pthread_mutex_destroy(&ref->lock);
|
|
||||||
Free(ref);
|
Free(ref);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -802,8 +800,6 @@ DbUnlock(Db * db, DbRef * ref)
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
pthread_mutex_lock(&db->lock);
|
|
||||||
|
|
||||||
lseek(ref->fd, 0L, SEEK_SET);
|
lseek(ref->fd, 0L, SEEK_SET);
|
||||||
if (ftruncate(ref->fd, 0) < 0)
|
if (ftruncate(ref->fd, 0) < 0)
|
||||||
{
|
{
|
||||||
|
@ -841,15 +837,11 @@ DbUnlock(Db * db, DbRef * ref)
|
||||||
destroy = 1;
|
destroy = 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
pthread_mutex_unlock(&ref->lock);
|
|
||||||
|
|
||||||
if (destroy)
|
if (destroy)
|
||||||
{
|
{
|
||||||
JsonFree(ref->json);
|
JsonFree(ref->json);
|
||||||
StringArrayFree(ref->name);
|
StringArrayFree(ref->name);
|
||||||
|
|
||||||
pthread_mutex_destroy(&ref->lock);
|
|
||||||
|
|
||||||
Free(ref);
|
Free(ref);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -874,11 +866,6 @@ DbExists(Db * db, size_t nArgs,...)
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
|
||||||
* Though it's not explicitly required, we lock the
|
|
||||||
* database before checking that an object exists to
|
|
||||||
* prevent any potential race conditions.
|
|
||||||
*/
|
|
||||||
pthread_mutex_lock(&db->lock);
|
pthread_mutex_lock(&db->lock);
|
||||||
|
|
||||||
file = DbFileName(db, args);
|
file = DbFileName(db, args);
|
||||||
|
@ -917,12 +904,15 @@ DbList(Db * db, size_t nArgs,...)
|
||||||
path = ArrayFromVarArgs(nArgs, ap);
|
path = ArrayFromVarArgs(nArgs, ap);
|
||||||
dir = DbDirName(db, path, 0);
|
dir = DbDirName(db, path, 0);
|
||||||
|
|
||||||
|
pthread_mutex_lock(&db->lock);
|
||||||
|
|
||||||
files = opendir(dir);
|
files = opendir(dir);
|
||||||
if (!files)
|
if (!files)
|
||||||
{
|
{
|
||||||
ArrayFree(path);
|
ArrayFree(path);
|
||||||
ArrayFree(result);
|
ArrayFree(result);
|
||||||
Free(dir);
|
Free(dir);
|
||||||
|
pthread_mutex_unlock(&db->lock);
|
||||||
return NULL;
|
return NULL;
|
||||||
}
|
}
|
||||||
while ((file = readdir(files)))
|
while ((file = readdir(files)))
|
||||||
|
@ -944,6 +934,7 @@ DbList(Db * db, size_t nArgs,...)
|
||||||
|
|
||||||
ArrayFree(path);
|
ArrayFree(path);
|
||||||
Free(dir);
|
Free(dir);
|
||||||
|
pthread_mutex_unlock(&db->lock);
|
||||||
|
|
||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue