Skip to content

Commit

Permalink
SDL_HashTable is now optionally thread-safe
Browse files Browse the repository at this point in the history 
Fixes libsdl-org#11635
  • Loading branch information
@slouken
slouken committed Dec 12, 2024
1 parent e0cee83 commit 68f1f31
Show file tree
Hide file tree
Showing 12 changed files with 175 additions and 181 deletions.
98 changes: 80 additions & 18 deletions src/SDL_hashtable.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ SDL_COMPILE_TIME_ASSERT(sizeof_SDL_HashItem, sizeof(SDL_HashItem) <= MAX_HASHITE

struct SDL_HashTable
{
SDL_RWLock *lock;
SDL_HashItem *table;
SDL_HashTable_HashFn hash;
SDL_HashTable_KeyMatchFn keymatch;
Expand All @@ -60,6 +61,7 @@ SDL_HashTable *SDL_CreateHashTable(void *data,
SDL_HashTable_HashFn hashfn,
SDL_HashTable_KeyMatchFn keymatchfn,
SDL_HashTable_NukeFn nukefn,
bool threadsafe,
bool stackable)
{
SDL_HashTable *table;
Expand All @@ -80,9 +82,14 @@ SDL_HashTable *SDL_CreateHashTable(void *data,
return NULL;
}

if (threadsafe) {
// Don't fail if we can't create a lock (single threaded environment?)
table->lock = SDL_CreateRWLock();
}

table->table = (SDL_HashItem *)SDL_calloc(num_buckets, sizeof(SDL_HashItem));
if (!table->table) {
SDL_free(table);
SDL_DestroyHashTable(table);
return NULL;
}

Expand Down Expand Up @@ -289,17 +296,22 @@ bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const void *
{
SDL_HashItem *item;
Uint32 hash;
bool result = false;

if (!table) {
return false;
}

if (table->lock) {
SDL_LockRWLockForWriting(table->lock);
}

hash = calc_hash(table, key);
item = find_first_item(table, key, hash);

if (item && !table->stackable) {
// TODO: Maybe allow overwrites? We could do it more efficiently here than unset followed by insert.
return false;
// Allow overwrites, this might have been inserted on another thread
delete_item(table, item);
}

SDL_HashItem new_item;
Expand All @@ -313,18 +325,25 @@ bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const void *

if (!maybe_resize(table)) {
table->num_occupied_slots--;
return false;
goto done;
}

// This never returns NULL
insert_item(&new_item, table->table, table->hash_mask, &table->max_probe_len);
return true;
result = true;

done:
if (table->lock) {
SDL_UnlockRWLock(table->lock);
}
return result;
}

bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, const void **value)
{
Uint32 hash;
SDL_HashItem *i;
bool result = false;

if (!table) {
if (value) {
Expand All @@ -333,26 +352,39 @@ bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, const void
return false;
}

if (table->lock) {
SDL_LockRWLockForReading(table->lock);
}

hash = calc_hash(table, key);
i = find_first_item(table, key, hash);
if (i) {
if (value) {
*value = i->value;
}
return true;
result = true;
}
return false;

if (table->lock) {
SDL_UnlockRWLock(table->lock);
}
return result;
}

bool SDL_RemoveFromHashTable(SDL_HashTable *table, const void *key)
{
Uint32 hash;
SDL_HashItem *item;
bool result = false;

if (!table) {
return false;
}

if (table->lock) {
SDL_LockRWLockForWriting(table->lock);
}

// FIXME: what to do for stacking hashtables?
// The original code removes just one item.
// This hashtable happens to preserve the insertion order of multi-value keys,
Expand All @@ -361,13 +393,18 @@ bool SDL_RemoveFromHashTable(SDL_HashTable *table, const void *key)

hash = calc_hash(table, key);
item = find_first_item(table, key, hash);

if (!item) {
return false;
goto done;
}

delete_item(table, item);
return true;
result = true;

done:
if (table->lock) {
SDL_UnlockRWLock(table->lock);
}
return result;
}

bool SDL_IterateHashTableKey(const SDL_HashTable *table, const void *key, const void **_value, void **iter)
Expand Down Expand Up @@ -456,6 +493,28 @@ bool SDL_HashTableEmpty(SDL_HashTable *table)
return !(table && table->num_occupied_slots);
}

void SDL_LockHashTable(SDL_HashTable *table, bool for_writing)
{
if (!table) {
return;
}

if (for_writing) {
SDL_LockRWLockForWriting(table->lock);
} else {
SDL_LockRWLockForReading(table->lock);
}
}

void SDL_UnlockHashTable(SDL_HashTable *table)
{
if (!table) {
return;
}

SDL_UnlockRWLock(table->lock);
}

static void nuke_all(SDL_HashTable *table)
{
void *data = table->data;
Expand All @@ -472,22 +531,25 @@ static void nuke_all(SDL_HashTable *table)
void SDL_EmptyHashTable(SDL_HashTable *table)
{
if (table) {
if (table->nuke) {
nuke_all(table);
}
SDL_LockRWLockForWriting(table->lock);
{
if (table->nuke) {
nuke_all(table);
}

SDL_memset(table->table, 0, sizeof(*table->table) * (table->hash_mask + 1));
table->num_occupied_slots = 0;
SDL_memset(table->table, 0, sizeof(*table->table) * (table->hash_mask + 1));
table->num_occupied_slots = 0;
}
SDL_UnlockRWLock(table->lock);
}
}

void SDL_DestroyHashTable(SDL_HashTable *table)
{
if (table) {
if (table->nuke) {
nuke_all(table);
}
SDL_EmptyHashTable(table);

SDL_DestroyRWLock(table->lock);
SDL_free(table->table);
SDL_free(table);
}
Expand Down
17 changes: 17 additions & 0 deletions src/SDL_hashtable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,36 @@ extern SDL_HashTable *SDL_CreateHashTable(void *data,
SDL_HashTable_HashFn hashfn,
SDL_HashTable_KeyMatchFn keymatchfn,
SDL_HashTable_NukeFn nukefn,
bool threadsafe,
bool stackable);

// This function is thread-safe if the hashtable was created with threadsafe = true
extern void SDL_EmptyHashTable(SDL_HashTable *table);

// This function is not thread-safe.
extern void SDL_DestroyHashTable(SDL_HashTable *table);

// This function is thread-safe if the hashtable was created with threadsafe = true
extern bool SDL_InsertIntoHashTable(SDL_HashTable *table, const void *key, const void *value);

// This function is thread-safe if the hashtable was created with threadsafe = true
extern bool SDL_RemoveFromHashTable(SDL_HashTable *table, const void *key);

// This function is thread-safe if the hashtable was created with threadsafe = true
extern bool SDL_FindInHashTable(const SDL_HashTable *table, const void *key, const void **_value);

// This function is thread-safe if the hashtable was created with threadsafe = true
extern bool SDL_HashTableEmpty(SDL_HashTable *table);

extern void SDL_LockHashTable(SDL_HashTable *table, bool for_writing);
extern void SDL_UnlockHashTable(SDL_HashTable *table);

// iterate all values for a specific key. This only makes sense if the hash is stackable. If not-stackable, just use SDL_FindInHashTable().
// This function is not thread-safe, you should use SDL_LockHashTable() if necessary
extern bool SDL_IterateHashTableKey(const SDL_HashTable *table, const void *key, const void **_value, void **iter);

// iterate all key/value pairs in a hash (stackable hashes can have duplicate keys with multiple values).
// This function is not thread-safe, you should use SDL_LockHashTable() if necessary
extern bool SDL_IterateHashTable(const SDL_HashTable *table, const void **_key, const void **_value, void **iter);

extern Uint32 SDL_HashPointer(const void *key, void *unused);
Expand Down
Loading

0 comments on commit 68f1f31

Please sign in to comment.