diff --git a/src/perf/perf.c b/src/perf/perf.c index 068a247..a2875dc 100644 --- a/src/perf/perf.c +++ b/src/perf/perf.c @@ -147,6 +147,8 @@ static void cache_push_front(eb_lru_cache_t *c, eb_lru_node_t *n) { bool eb_cache_put(eb_lru_cache_t *c, const char *key, const void *val, size_t sz, uint32_t ttl) { if (!c || !key || !val) return false; + /* Reject entries that can never fit before evicting usable data. */ + if (c->max_entries <= 0 || (c->max_bytes && sz > c->max_bytes)) return false; /* Check if already exists */ for (int i = 0; i < c->count; i++) { if (strcmp(c->entries[i].key, key) == 0) { @@ -163,10 +165,20 @@ bool eb_cache_put(eb_lru_cache_t *c, const char *key, const void *val, size_t sz return true; } } - /* Evict if needed */ - while (c->count >= c->max_entries || (c->max_bytes && c->current_bytes + sz > c->max_bytes)) + /* Evict until the new entry fits, or until eviction stops making room. + * Both conditions can stay true against a cache with nothing left to give: + * a value larger than the whole byte budget, or a cache configured with + * max_entries == 0. eb_cache_evict_lru() is a no-op once the list is + * empty, so the unguarded loop spun here forever. */ + while (c->count >= c->max_entries || + (c->max_bytes && c->current_bytes > c->max_bytes - sz)) { + int before = c->count; eb_cache_evict_lru(c); - if (c->count >= EB_CACHE_MAX_ENTRIES) return false; + if (c->count >= before) break; + } + /* Still does not fit after evicting everything it could — refuse it. */ + if (c->count >= c->max_entries || c->count >= EB_CACHE_MAX_ENTRIES) return false; + if (c->max_bytes && c->current_bytes > c->max_bytes - sz) return false; eb_lru_node_t *n = &c->entries[c->count]; memset(n, 0, sizeof(*n)); @@ -182,7 +194,8 @@ bool eb_cache_put(eb_lru_cache_t *c, const char *key, const void *val, size_t sz } void *eb_cache_get(eb_lru_cache_t *c, const char *key, size_t *sz) { - if (!c || !key) { c->misses++; return NULL; } + if (!c) return NULL; /* c->misses++ here dereferenced NULL */ + if (!key) { c->misses++; return NULL; } uint64_t now = perf_now_us(); for (int i = 0; i < c->count; i++) { eb_lru_node_t *n = &c->entries[i]; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 86d1ebe..99300db 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -82,6 +82,13 @@ add_executable(test_bookmark test_bookmark.c) target_link_libraries(test_bookmark PRIVATE eb_ui) add_test(NAME test_bookmark COMMAND test_bookmark) +add_executable(test_perf_cache test_perf_cache.c) +target_link_libraries(test_perf_cache PRIVATE eb_perf) +add_test(NAME test_perf_cache COMMAND test_perf_cache) +# Both regressions this covers were a hang, not a wrong answer, so an +# unbounded test run would stall the suite rather than fail it. +set_tests_properties(test_perf_cache PROPERTIES TIMEOUT 30) + # --- Benchmark --- add_executable(benchmark benchmark.c) target_link_libraries(benchmark PRIVATE diff --git a/tests/test_perf_cache.c b/tests/test_perf_cache.c new file mode 100644 index 0000000..22ef896 --- /dev/null +++ b/tests/test_perf_cache.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: MIT +/* Unit tests for the LRU cache in src/perf/perf.c. + * + * eb_perf had no ctest target at all, so none of this code was covered: the + * three executables that link it (benchmark, http2_server, load_test_combined) + * are build targets, not registered tests. The first two cases below are + * regressions — both crashed or hung before the guards in eb_cache_put() and + * eb_cache_get() were repaired. */ +#include "eBrowser/perf.h" +#include +#include + +static int s_pass = 0, s_fail = 0; +#define TEST(name) static void name(void) +#define RUN(name) do { \ + int failures_before = s_fail; \ + printf(" %s... ", #name); \ + name(); \ + if (s_fail == failures_before) { printf("PASS\n"); s_pass++; } \ +} while(0) +#define ASSERT(cond) do { if(!(cond)) { printf("FAIL: %s:%d: %s\n", __FILE__, __LINE__, #cond); s_fail++; return; } } while(0) + +/* Regression: the guard that rejects a NULL cache dereferenced it first. */ +TEST(test_get_null_cache_does_not_crash) { + ASSERT(eb_cache_get(NULL, "k", NULL) == NULL); +} + +TEST(test_get_null_key_counts_a_miss) { + eb_lru_cache_t c; + eb_cache_init(&c, 8, 4096); + ASSERT(eb_cache_get(&c, NULL, NULL) == NULL); + ASSERT(c.misses == 1); + eb_cache_destroy(&c); +} + +/* Regression: a value that cannot ever fit the byte budget made the eviction + * loop spin against an already-empty cache. */ +TEST(test_put_larger_than_byte_budget_is_refused) { + eb_lru_cache_t c; + eb_cache_init(&c, 8, 64); + char big[256]; + memset(big, 'x', sizeof(big)); + ASSERT(eb_cache_put(&c, "keep", "existing", 9, 0) == true); + ASSERT(eb_cache_put(&c, "big", big, sizeof(big), 0) == false); + ASSERT(c.count == 1); + ASSERT(c.current_bytes == 9); + ASSERT(eb_cache_get(&c, "keep", NULL) != NULL); + eb_cache_destroy(&c); +} + +/* Regression: max_entries == 0 makes `count >= max_entries` true forever. */ +TEST(test_put_into_zero_entry_cache_is_refused) { + eb_lru_cache_t c; + eb_cache_init(&c, 1, 0); + ASSERT(eb_cache_put(&c, "keep", "v", 1, 0) == true); + c.max_entries = 0; + ASSERT(eb_cache_put(&c, "k", "v", 1, 0) == false); + ASSERT(c.count == 1); + ASSERT(eb_cache_get(&c, "keep", NULL) != NULL); + eb_cache_destroy(&c); +} + +TEST(test_put_get_remove_roundtrip) { + eb_lru_cache_t c; + eb_cache_init(&c, 4, 4096); + ASSERT(eb_cache_put(&c, "a", "alpha", 6, 0) == true); + size_t sz = 0; + void *v = eb_cache_get(&c, "a", &sz); + ASSERT(v != NULL); + ASSERT(sz == 6); + ASSERT(strcmp((const char *)v, "alpha") == 0); + ASSERT(c.hits == 1); + ASSERT(eb_cache_get(&c, "missing", NULL) == NULL); + ASSERT(c.misses == 1); + ASSERT(eb_cache_remove(&c, "a") == true); + ASSERT(eb_cache_get(&c, "a", NULL) == NULL); + eb_cache_destroy(&c); +} + +/* A put that fits the budget still evicts to make room for itself. */ +TEST(test_put_evicts_to_make_room) { + eb_lru_cache_t c; + eb_cache_init(&c, 8, 16); + ASSERT(eb_cache_put(&c, "a", "0123456789", 10, 0) == true); + ASSERT(c.count == 1); + ASSERT(eb_cache_put(&c, "b", "0123456789", 10, 0) == true); + ASSERT(c.count == 1); + ASSERT(eb_cache_get(&c, "b", NULL) != NULL); + eb_cache_destroy(&c); +} + +int main(void) { + printf("=== Performance LRU Cache Tests ===\n"); + RUN(test_get_null_cache_does_not_crash); + RUN(test_get_null_key_counts_a_miss); + RUN(test_put_larger_than_byte_budget_is_refused); + RUN(test_put_into_zero_entry_cache_is_refused); + RUN(test_put_get_remove_roundtrip); + RUN(test_put_evicts_to_make_room); + printf("\nResults: %d passed, %d failed\n", s_pass, s_fail); + return s_fail > 0 ? 1 : 0; +}