Skip to content

Commit

Permalink
fraud_detection: Fix hash-level locking issues
Browse files Browse the repository at this point in the history
This patch prevent some rare occassions when, due to too-loose locking,
fraud_detection could run into either:

    * SHM memory leaks
    * incorrectly processed counters (corrupt stats)
  • Loading branch information
liviuchircu committed Jan 23, 2019
1 parent b491999 commit 32ec7a2
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 20 deletions.
17 changes: 4 additions & 13 deletions modules/fraud_detection/frd_hashmap.c
Expand Up @@ -53,22 +53,13 @@ int init_hash_map(hash_map_t *hm)
return 0;
}

void** get_item (hash_map_t *hm, str key)
void** get_item (hash_map_t *hm, str key, rw_lock_t **unlock)
{
unsigned int hash = core_hash(&key, NULL, hm->size);

lock_start_read(hm->buckets[hash].lock);
void **find_res = map_find(hm->buckets[hash].items, key);
lock_stop_read(hm->buckets[hash].lock);
if (find_res) {
return find_res;
}
else {
lock_start_write(hm->buckets[hash].lock);
find_res = map_get(hm->buckets[hash].items, key);
lock_stop_write(hm->buckets[hash].lock);
return find_res;
}
*unlock = hm->buckets[hash].lock;
lock_start_write(hm->buckets[hash].lock);
return map_get(hm->buckets[hash].items, key);
}

void free_hash_map(hash_map_t* hm, void (*value_destroy_func)(void *))
Expand Down
2 changes: 1 addition & 1 deletion modules/fraud_detection/frd_hashmap.h
Expand Up @@ -43,7 +43,7 @@ typedef struct {


int init_hash_map(hash_map_t* hm);
void** get_item (hash_map_t *hm, str key);
void** get_item (hash_map_t *hm, str key, rw_lock_t **unlock);
void free_hash_map(hash_map_t* hm, void (*value_destroy_func)(void *));

#endif
30 changes: 24 additions & 6 deletions modules/fraud_detection/frd_stats.c
Expand Up @@ -58,17 +58,21 @@ int init_stats_table(void)

frd_stats_entry_t* get_stats(str user, str prefix, str *shm_user)
{
rw_lock_t *item_lock;

/* First go one level below using the user key */
frd_users_map_item_t **hm =
(frd_users_map_item_t **)get_item(&stats_table, user);
(frd_users_map_item_t **)get_item(&stats_table, user, &item_lock);

if (*hm == NULL) {
/* First time the user is seen, we must create a hashmap */
*hm = shm_malloc(sizeof(frd_users_map_item_t));
if (*hm == NULL) {
lock_stop_write(item_lock);
LM_ERR("no more shm memory\n");
return NULL;
}
lock_stop_write(item_lock);

(*hm)->numbers_hm.size = FRD_PREFIX_HASH_SIZE;
if (init_hash_map(&(*hm)->numbers_hm) != 0) {
Expand All @@ -83,19 +87,23 @@ frd_stats_entry_t* get_stats(str user, str prefix, str *shm_user)
return NULL;
}
}
lock_stop_write(item_lock);

if (shm_user)
*shm_user = (*hm)->user;

frd_stats_entry_t **stats_entry =
(frd_stats_entry_t**)get_item(&(*hm)->numbers_hm, prefix);
(frd_stats_entry_t**)get_item(&(*hm)->numbers_hm, prefix, &item_lock);

if (*stats_entry == NULL) {
/* First time the prefix is seen for this user */
*stats_entry = shm_malloc(sizeof(frd_stats_entry_t));
if (*stats_entry == NULL) {
lock_stop_write(item_lock);
LM_ERR("no more shm memory\n");
return NULL;
}
lock_stop_write(item_lock);

/* Now init the auxiliary info for a stats structure */
if (!lock_init(&(*stats_entry)->lock)) {
Expand All @@ -105,24 +113,34 @@ frd_stats_entry_t* get_stats(str user, str prefix, str *shm_user)
}
memset(&((*stats_entry)->stats), 0, sizeof(frd_stats_t));
}
lock_stop_write(item_lock);

return *stats_entry;
}


int stats_exist(str user, str prefix)
{
rw_lock_t *item_lock;

/* First go one level below using the user key */
frd_users_map_item_t **hm =
(frd_users_map_item_t **)get_item(&stats_table, user);
(frd_users_map_item_t **)get_item(&stats_table, user, &item_lock);

if (*hm == NULL)
if (*hm == NULL) {
lock_stop_write(item_lock);
return 0;
}
lock_stop_write(item_lock);

frd_stats_entry_t **stats_entry =
(frd_stats_entry_t**)get_item(&(*hm)->numbers_hm, prefix);
if (*stats_entry == NULL)
(frd_stats_entry_t**)get_item(&(*hm)->numbers_hm, prefix, &item_lock);

if (*stats_entry == NULL) {
lock_stop_write(item_lock);
return 0;
}
lock_stop_write(item_lock);

return 1;
}
Expand Down

0 comments on commit 32ec7a2

Please sign in to comment.