Skip to content

Commit

Permalink
add missing spinlock unlocks on containers (netdata#19011)
Browse files Browse the repository at this point in the history 
* add missing spinlock unlocks on containers

* do not destroy the hashtable when cleaning up old versions

(cherry picked from commit efded89)
  • Loading branch information
@ktsaou @stelfrag
ktsaou authored and stelfrag committed Nov 14, 2024
1 parent 18fffc6 commit a66a0ab
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 58 deletions.
58 changes: 30 additions & 28 deletions src/libnetdata/os/system-maps/cached-gid-groupname.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ static struct {
bool initialized;
SPINLOCK spinlock;
SIMPLE_HASHTABLE_GROUPNAMES_CACHE ht;
} uc = {
} group_cache = {
.initialized = false,
.spinlock = NETDATA_SPINLOCK_INITIALIZER,
.ht = { 0 },
};
Expand All @@ -31,13 +32,13 @@ static bool compar_gid_ptr(gid_t *a, gid_t *b) {
}

void cached_groupname_populate_by_gid(gid_t gid, const char *groupname, uint32_t version) {
internal_fatal(!uc.initialized, "system-users cache needs to be initialized");
internal_fatal(!group_cache.initialized, "system-users cache needs to be initialized");
if(!groupname || !*groupname) return;

spinlock_lock(&uc.spinlock);
spinlock_lock(&group_cache.spinlock);

XXH64_hash_t hash = XXH3_64bits(&gid, sizeof(gid));
SIMPLE_HASHTABLE_SLOT_GROUPNAMES_CACHE *sl = simple_hashtable_get_slot_GROUPNAMES_CACHE(&uc.ht, hash, &gid, true);
SIMPLE_HASHTABLE_SLOT_GROUPNAMES_CACHE *sl = simple_hashtable_get_slot_GROUPNAMES_CACHE(&group_cache.ht, hash, &gid, true);
CACHED_GROUPNAME *cg = SIMPLE_HASHTABLE_SLOT_DATA(sl);
if(!cg || (cg->version && version > cg->version)) {
internal_fatal(cg && cg->gid != gid, "invalid gid matched from cache");
Expand All @@ -50,19 +51,19 @@ void cached_groupname_populate_by_gid(gid_t gid, const char *groupname, uint32_t
cg->version = version;
cg->gid = gid;
cg->groupname = string_strdupz(groupname);
simple_hashtable_set_slot_GROUPNAMES_CACHE(&uc.ht, sl, hash, cg);
simple_hashtable_set_slot_GROUPNAMES_CACHE(&group_cache.ht, sl, hash, cg);
}

spinlock_unlock(&uc.spinlock);
spinlock_unlock(&group_cache.spinlock);
}

CACHED_GROUPNAME cached_groupname_get_by_gid(gid_t gid) {
internal_fatal(!uc.initialized, "system-users cache needs to be initialized");
internal_fatal(!group_cache.initialized, "system-users cache needs to be initialized");

spinlock_lock(&uc.spinlock);
spinlock_lock(&group_cache.spinlock);

XXH64_hash_t hash = XXH3_64bits(&gid, sizeof(gid));
SIMPLE_HASHTABLE_SLOT_GROUPNAMES_CACHE *sl = simple_hashtable_get_slot_GROUPNAMES_CACHE(&uc.ht, hash, &gid, true);
SIMPLE_HASHTABLE_SLOT_GROUPNAMES_CACHE *sl = simple_hashtable_get_slot_GROUPNAMES_CACHE(&group_cache.ht, hash, &gid, true);
CACHED_GROUPNAME *cg = SIMPLE_HASHTABLE_SLOT_DATA(sl);
if(!cg) {
cg = callocz(1, sizeof(*cg));
Expand All @@ -79,7 +80,7 @@ CACHED_GROUPNAME cached_groupname_get_by_gid(gid_t gid) {
cg->groupname = string_strdupz(gr.gr_name);

cg->gid = gid;
simple_hashtable_set_slot_GROUPNAMES_CACHE(&uc.ht, sl, hash, cg);
simple_hashtable_set_slot_GROUPNAMES_CACHE(&group_cache.ht, sl, hash, cg);
}

internal_fatal(cg->gid != gid, "invalid gid matched from cache");
Expand All @@ -90,7 +91,7 @@ CACHED_GROUPNAME cached_groupname_get_by_gid(gid_t gid) {
.groupname = string_dup(cg->groupname),
};

spinlock_unlock(&uc.spinlock);
spinlock_unlock(&group_cache.spinlock);
return rc;
}

Expand All @@ -99,21 +100,21 @@ void cached_groupname_release(CACHED_GROUPNAME cg) {
}

void cached_groupnames_init(void) {
if(uc.initialized) return;
uc.initialized = true;
if(group_cache.initialized) return;
group_cache.initialized = true;

spinlock_init(&uc.spinlock);
simple_hashtable_init_GROUPNAMES_CACHE(&uc.ht, 100);
spinlock_init(&group_cache.spinlock);
simple_hashtable_init_GROUPNAMES_CACHE(&group_cache.ht, 100);
}

void cached_groupnames_destroy(void) {
if(!uc.initialized) return;
if(!group_cache.initialized) return;

spinlock_lock(&uc.spinlock);
spinlock_lock(&group_cache.spinlock);

for(SIMPLE_HASHTABLE_SLOT_GROUPNAMES_CACHE *sl = simple_hashtable_first_read_only_GROUPNAMES_CACHE(&uc.ht);
for(SIMPLE_HASHTABLE_SLOT_GROUPNAMES_CACHE *sl = simple_hashtable_first_read_only_GROUPNAMES_CACHE(&group_cache.ht);
sl;
sl = simple_hashtable_next_read_only_GROUPNAMES_CACHE(&uc.ht, sl)) {
sl = simple_hashtable_next_read_only_GROUPNAMES_CACHE(&group_cache.ht, sl)) {
CACHED_GROUPNAME *u = SIMPLE_HASHTABLE_SLOT_DATA(sl);
if(u) {
string_freez(u->groupname);
Expand All @@ -122,26 +123,27 @@ void cached_groupnames_destroy(void) {
}
}

simple_hashtable_destroy_GROUPNAMES_CACHE(&uc.ht);
uc.initialized = false;
simple_hashtable_destroy_GROUPNAMES_CACHE(&group_cache.ht);
group_cache.initialized = false;

spinlock_unlock(&group_cache.spinlock);
}

void cached_groupnames_delete_old_versions(uint32_t version) {
if(!uc.initialized) return;
if(!group_cache.initialized) return;

spinlock_lock(&uc.spinlock);
spinlock_lock(&group_cache.spinlock);

for(SIMPLE_HASHTABLE_SLOT_GROUPNAMES_CACHE *sl = simple_hashtable_first_read_only_GROUPNAMES_CACHE(&uc.ht);
for(SIMPLE_HASHTABLE_SLOT_GROUPNAMES_CACHE *sl = simple_hashtable_first_read_only_GROUPNAMES_CACHE(&group_cache.ht);
sl;
sl = simple_hashtable_next_read_only_GROUPNAMES_CACHE(&uc.ht, sl)) {
sl = simple_hashtable_next_read_only_GROUPNAMES_CACHE(&group_cache.ht, sl)) {
CACHED_GROUPNAME *cg = SIMPLE_HASHTABLE_SLOT_DATA(sl);
if(cg && cg->version && cg->version < version) {
string_freez(cg->groupname);
freez(cg);
simple_hashtable_del_slot_GROUPNAMES_CACHE(&uc.ht, sl);
simple_hashtable_del_slot_GROUPNAMES_CACHE(&group_cache.ht, sl);
}
}

simple_hashtable_destroy_GROUPNAMES_CACHE(&uc.ht);
uc.initialized = false;
spinlock_unlock(&group_cache.spinlock);
}
3 changes: 2 additions & 1 deletion src/libnetdata/os/system-maps/cached-sid-username.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ static struct {
SPINLOCK spinlock;
struct simple_hashtable_SID hashtable;
} sid_globals = {
.spinlock = NETDATA_SPINLOCK_INITIALIZER,
.spinlock = NETDATA_SPINLOCK_INITIALIZER,
.hashtable = { 0 },
};

static inline SID_KEY *sid_value_to_key(SID_VALUE *s) {
Expand Down
59 changes: 30 additions & 29 deletions src/libnetdata/os/system-maps/cached-uid-username.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
#include "libnetdata/simple_hashtable/simple_hashtable.h"

static struct {
uint32_t version;
bool initialized;
SPINLOCK spinlock;
SIMPLE_HASHTABLE_USERNAMES_CACHE ht;
} uc = {
} user_cache = {
.initialized = false,
.spinlock = NETDATA_SPINLOCK_INITIALIZER,
.ht = { 0 },
};
Expand All @@ -32,13 +32,13 @@ static bool compar_uid_ptr(uid_t *a, uid_t *b) {
}

void cached_username_populate_by_uid(uid_t uid, const char *username, uint32_t version) {
internal_fatal(!uc.initialized, "system-users cache needs to be initialized");
internal_fatal(!user_cache.initialized, "system-users cache needs to be initialized");
if(!username || !*username) return;

spinlock_lock(&uc.spinlock);
spinlock_lock(&user_cache.spinlock);

XXH64_hash_t hash = XXH3_64bits(&uid, sizeof(uid));
SIMPLE_HASHTABLE_SLOT_USERNAMES_CACHE *sl = simple_hashtable_get_slot_USERNAMES_CACHE(&uc.ht, hash, &uid, true);
SIMPLE_HASHTABLE_SLOT_USERNAMES_CACHE *sl = simple_hashtable_get_slot_USERNAMES_CACHE(&user_cache.ht, hash, &uid, true);
CACHED_USERNAME *cu = SIMPLE_HASHTABLE_SLOT_DATA(sl);
if(!cu || (cu->version && version > cu->version)) {
internal_fatal(cu && cu->uid != uid, "invalid uid matched from cache");
Expand All @@ -51,19 +51,19 @@ void cached_username_populate_by_uid(uid_t uid, const char *username, uint32_t v
cu->version = version;
cu->uid = uid;
cu->username = string_strdupz(username);
simple_hashtable_set_slot_USERNAMES_CACHE(&uc.ht, sl, hash, cu);
simple_hashtable_set_slot_USERNAMES_CACHE(&user_cache.ht, sl, hash, cu);
}

spinlock_unlock(&uc.spinlock);
spinlock_unlock(&user_cache.spinlock);
}

CACHED_USERNAME cached_username_get_by_uid(uid_t uid) {
internal_fatal(!uc.initialized, "system-users cache needs to be initialized");
internal_fatal(!user_cache.initialized, "system-users cache needs to be initialized");

spinlock_lock(&uc.spinlock);
spinlock_lock(&user_cache.spinlock);

XXH64_hash_t hash = XXH3_64bits(&uid, sizeof(uid));
SIMPLE_HASHTABLE_SLOT_USERNAMES_CACHE *sl = simple_hashtable_get_slot_USERNAMES_CACHE(&uc.ht, hash, &uid, true);
SIMPLE_HASHTABLE_SLOT_USERNAMES_CACHE *sl = simple_hashtable_get_slot_USERNAMES_CACHE(&user_cache.ht, hash, &uid, true);
CACHED_USERNAME *cu = SIMPLE_HASHTABLE_SLOT_DATA(sl);
if(!cu) {
cu = callocz(1, sizeof(*cu));
Expand All @@ -80,7 +80,7 @@ CACHED_USERNAME cached_username_get_by_uid(uid_t uid) {
cu->username = string_strdupz(pw.pw_name);

cu->uid = uid;
simple_hashtable_set_slot_USERNAMES_CACHE(&uc.ht, sl, hash, cu);
simple_hashtable_set_slot_USERNAMES_CACHE(&user_cache.ht, sl, hash, cu);
}

internal_fatal(cu->uid != uid, "invalid uid matched from cache");
Expand All @@ -91,7 +91,7 @@ CACHED_USERNAME cached_username_get_by_uid(uid_t uid) {
.username = string_dup(cu->username),
};

spinlock_unlock(&uc.spinlock);
spinlock_unlock(&user_cache.spinlock);
return rc;
}

Expand All @@ -100,21 +100,21 @@ void cached_username_release(CACHED_USERNAME cu) {
}

void cached_usernames_init(void) {
if(uc.initialized) return;
uc.initialized = true;
if(user_cache.initialized) return;
user_cache.initialized = true;

spinlock_init(&uc.spinlock);
simple_hashtable_init_USERNAMES_CACHE(&uc.ht, 100);
spinlock_init(&user_cache.spinlock);
simple_hashtable_init_USERNAMES_CACHE(&user_cache.ht, 100);
}

void cached_usernames_destroy(void) {
if(!uc.initialized) return;
if(!user_cache.initialized) return;

spinlock_lock(&uc.spinlock);
spinlock_lock(&user_cache.spinlock);

for(SIMPLE_HASHTABLE_SLOT_USERNAMES_CACHE *sl = simple_hashtable_first_read_only_USERNAMES_CACHE(&uc.ht);
for(SIMPLE_HASHTABLE_SLOT_USERNAMES_CACHE *sl = simple_hashtable_first_read_only_USERNAMES_CACHE(&user_cache.ht);
sl;
sl = simple_hashtable_next_read_only_USERNAMES_CACHE(&uc.ht, sl)) {
sl = simple_hashtable_next_read_only_USERNAMES_CACHE(&user_cache.ht, sl)) {
CACHED_USERNAME *u = SIMPLE_HASHTABLE_SLOT_DATA(sl);
if(u) {
string_freez(u->username);
Expand All @@ -123,26 +123,27 @@ void cached_usernames_destroy(void) {
}
}

simple_hashtable_destroy_USERNAMES_CACHE(&uc.ht);
uc.initialized = false;
simple_hashtable_destroy_USERNAMES_CACHE(&user_cache.ht);
user_cache.initialized = false;

spinlock_unlock(&user_cache.spinlock);
}

void cached_usernames_delete_old_versions(uint32_t version) {
if(!uc.initialized) return;
if(!user_cache.initialized) return;

spinlock_lock(&uc.spinlock);
spinlock_lock(&user_cache.spinlock);

for(SIMPLE_HASHTABLE_SLOT_USERNAMES_CACHE *sl = simple_hashtable_first_read_only_USERNAMES_CACHE(&uc.ht);
for(SIMPLE_HASHTABLE_SLOT_USERNAMES_CACHE *sl = simple_hashtable_first_read_only_USERNAMES_CACHE(&user_cache.ht);
sl;
sl = simple_hashtable_next_read_only_USERNAMES_CACHE(&uc.ht, sl)) {
sl = simple_hashtable_next_read_only_USERNAMES_CACHE(&user_cache.ht, sl)) {
CACHED_USERNAME *cu = SIMPLE_HASHTABLE_SLOT_DATA(sl);
if(cu && cu->version && cu->version < version) {
string_freez(cu->username);
freez(cu);
simple_hashtable_del_slot_USERNAMES_CACHE(&uc.ht, sl);
simple_hashtable_del_slot_USERNAMES_CACHE(&user_cache.ht, sl);
}
}

simple_hashtable_destroy_USERNAMES_CACHE(&uc.ht);
uc.initialized = false;
spinlock_unlock(&user_cache.spinlock);
}

0 comments on commit a66a0ab

Please sign in to comment.