runtime: state_get leaked its value on every call
El SDK CI - dev / build-and-test (pull_request) Failing after 14m26s
El SDK CI - dev / build-and-test (pull_request) Failing after 14m26s
char* result = el_strdup_persist(e ? e->value : ""); // never freed
pthread_mutex_unlock(&_state_mu);
char* copy = el_strdup(result); // arena-tracked
return el_wrap_str(copy);
Two copies were made. `result` existed only as the source for `copy` — never
returned, never freed — and el_strdup_persist bypasses the arena BY DESIGN
("state_set, engram internals"), so arena-pop could never reclaim it. Every
state_get leaked its full value string, permanently.
MEASURED: 200,000 state_get calls against a 64-byte value.
before 15 MB peak RSS growth (~75 bytes/call — the value plus overhead)
after 0 MB
IMPACT. The soul's awareness loop has 68 state_get call sites and ticks every
200ms. Live measurement before the fix: RSS climbing 112 MB per 20s, about
19 GB/hour, in awareness_run -> one_cycle -> perceive, while node_count stayed
flat at ~13,479 — growth with no data behind it. It drove the host from 20 GB
free to 4.3 GB in roughly an hour.
WHY NOW, since the code is old: the soul used to restart constantly (no
write-through, divergent graph, 2.11 GB). Stabilising it (neuron #162) let it
stay up long enough to accumulate. The fix did not cause this leak; it removed
the crashes that were hiding it. Same pattern as the test framework surfacing
math_log — the defect was always there, something finally made it visible.
Found by Ishikawa rather than by reading the nearest code: method (arena
push/pop IS correctly paired per tick), material (node count flat, so not data
growth), environment (19 GB/hr / 18,000 ticks = ~1.1 MB per tick, so per-tick
not one-shot), machine (an allocator that bypasses the arena) — which is where
the evidence pointed.
el_strdup tracks into the thread-local arena, which touches no shared state, so
taking the single copy under _state_mu is safe and removes the temporary
entirely.
Verified: self-hosting fixpoint byte-identical; state round-trip correct for
hit, miss, and overwrite.
This commit is contained in:
@@ -5168,10 +5168,23 @@ el_val_t state_get(el_val_t key) {
|
||||
if (!k) return el_wrap_str(el_strdup(""));
|
||||
pthread_mutex_lock(&_state_mu);
|
||||
StateEntry* e = state_find(k);
|
||||
char* result = el_strdup_persist(e ? e->value : "");
|
||||
/* ONE arena-tracked copy, taken under the lock.
|
||||
*
|
||||
* This used to make TWO copies: an el_strdup_persist temporary, then an
|
||||
* arena-tracked copy of that temporary. The persistent one was never
|
||||
* returned and never freed — el_strdup_persist bypasses the arena by
|
||||
* design ("state_set, engram internals"), so arena-pop could not reclaim
|
||||
* it. Every state_get therefore leaked its full value string, permanently.
|
||||
*
|
||||
* The soul's awareness loop has 68 state_get call sites and ticks every
|
||||
* 200ms; measured leak was ~1.1 MB per tick, about 19 GB/hour. It went
|
||||
* unnoticed for as long as the soul restarted often enough to mask it.
|
||||
*
|
||||
* el_strdup tracks into the thread-local arena, which touches no shared
|
||||
* state, so doing it under _state_mu is safe and removes the need for the
|
||||
* temporary entirely. */
|
||||
char* copy = el_strdup(e ? e->value : "");
|
||||
pthread_mutex_unlock(&_state_mu);
|
||||
/* wrap in arena-tracked copy for the caller's request lifetime */
|
||||
char* copy = el_strdup(result);
|
||||
return el_wrap_str(copy);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user