Skip to content

Commit

Permalink
Fix over-freeing in internal object cache
Browse files Browse the repository at this point in the history
When returning item to the cache and the items allocated exceeded cache->limit, we should free the item and not put it to the cache.

The current code will free any item that tried returned to the cache when cache->total < cache->limit which is true in most cases if the total objects has never exceeded the limit.
  • Loading branch information
bitground authored and dormando committed Oct 26, 2020
1 parent 6b319c8 commit b031143
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
2 changes: 1 addition & 1 deletion cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ void do_cache_free(cache_t *cache, void *ptr) {
}
ptr = pre;
#endif
if (cache->limit > cache->total) {
if (cache->limit != 0 && cache->limit < cache->total) {
/* Allow freeing in case the limit was revised downward */
if (cache->destructor) {
cache->destructor(ptr, NULL);
Expand Down
29 changes: 29 additions & 0 deletions testapp.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,34 @@ static enum test_return cache_redzone_test(void)
#endif
}

static enum test_return cache_limit_revised_downward_test(void)
{
int limit = 10, allocated_num = limit + 1, i;
char ** alloc_objs = calloc(allocated_num, sizeof(char *));

cache_t *cache = cache_create("test", sizeof(uint32_t), sizeof(char*),
NULL, NULL);
assert(cache != NULL);

/* cache->limit is 0 and we can allocate limit+1 items */
for (i = 0; i < allocated_num; i++) {
alloc_objs[i] = cache_alloc(cache);
assert(alloc_objs[i] != NULL);
}
assert(cache->total == allocated_num);

/* revised downward cache->limit */
cache_set_limit(cache, limit);

/* If we free one item, the cache->total should decreased by one*/
cache_free(cache, alloc_objs[0]);

assert(cache->total == allocated_num-1);
cache_destroy(cache);

return TEST_PASS;
}

static enum test_return test_stats_prefix_find(void) {
PREFIX_STATS *pfs1, *pfs2;

Expand Down Expand Up @@ -2237,6 +2265,7 @@ struct testcase testcases[] = {
{ "cache_destructor", cache_destructor_test },
{ "cache_reuse", cache_reuse_test },
{ "cache_redzone", cache_redzone_test },
{ "cache_limit_revised_downward", cache_limit_revised_downward_test },
{ "stats_prefix_find", test_stats_prefix_find },
{ "stats_prefix_record_get", test_stats_prefix_record_get },
{ "stats_prefix_record_delete", test_stats_prefix_record_delete },
Expand Down

0 comments on commit b031143

Please sign in to comment.