runtime: guard engram activation against the unsynchronized awareness thread
The soul daemon had two engram callers and only one of them locked.
soul.el:729 starts the HTTP server via http_serve_async (spawning
http_worker threads); soul.el:731 then runs awareness_run() on the MAIN
thread. awareness.el's perceive() -> engram_activate_json() ->
engram_activate() -> eg_vindex_sync() -> vindex_insert() mutates the same
g->nodes/g->edges and the process-global _eg_vindex HNSW index that the
workers touch. g_engram_req_lock existed to serialize exactly this, but it
was only ever taken inside http_worker: engram_req_lock/engram_req_unlock
appear in ZERO .el sources, so the awareness loop ran lock-free beside the
workers on every tick (SOUL_TICK_MS=1000).
Result was a crash-loop under launchd KeepAlive: five crashes in ~4 minutes
on 2026-08-16 with varying faulting frames -- search_layer<-vindex_insert
<-eg_vindex_sync, engram_activate, abort, and one inside xzm_realloc's own
freelist. Varying sites plus a fault in allocator metadata means heap
corruption. The SIGSEGV address 0x65646f4e6d617267 is little-endian ASCII
"gramNode": string bytes dereferenced as an Elem vector pointer.
Diagnosed by bisection rather than inspection:
- Replaying all 13,820 real dim-768 vectors harvested from the live store
through the index single-threaded under ASan is 100% clean, which rules
out an HNSW logic/bounds bug.
- Two threads on one index trip ThreadSanitizer immediately at
engram_vindex.c:195 (visited_reset), reached from both vindex_search and
vindex_insert. VIndex keeps a SHARED visited-epoch scratch buffer, so
even two concurrent READS corrupt each other's traversal and walk bogus
element indices.
So this is purely a concurrency defect, not an HNSW logic error. (An
inspection-derived hypothesis about an out-of-bounds reverse-link write at
engram_vindex.c:340 was disproved by the single-threaded run.)
Fix: a thread-local ownership depth (_eg_req_depth) lets engram entry points
self-guard. engram_activate() becomes a wrapper over engram_activate_inner()
that acquires g_engram_req_lock when called with depth 0 (the awareness
thread) and passes through when depth > 0 (nested inside an http_worker that
already holds it), so the non-recursive mutex cannot self-deadlock. The depth
is a plain counter, never a recursive-mutex count, preserving
engram_self_reify_beat_json's contract of genuinely releasing the lock
mid-beat.
This commit is contained in:
@@ -1600,8 +1600,50 @@ typedef struct {
|
||||
* no longer blocks ingest/reads (measured: non-health latency during a beat
|
||||
* 13.9s → sub-second). */
|
||||
static pthread_mutex_t g_engram_req_lock = PTHREAD_MUTEX_INITIALIZER;
|
||||
void engram_req_unlock(void){ pthread_mutex_unlock(&g_engram_req_lock); }
|
||||
void engram_req_lock(void){ pthread_mutex_lock(&g_engram_req_lock); }
|
||||
|
||||
/* ── AWARENESS-THREAD GUARD (2026-08-16 self-review) ─────────────────────────
|
||||
* The request lock above serialized http_worker threads against EACH OTHER, but
|
||||
* the soul daemon has a SECOND, unsynchronized engram caller: soul.el starts the
|
||||
* HTTP server with http_serve_async (spawning worker threads) and then runs
|
||||
* awareness_run() on the MAIN thread, whose perceive() -> engram_activate_json()
|
||||
* -> engram_activate() -> eg_vindex_sync() path mutates the very same RAM graph
|
||||
* and the process-global _eg_vindex HNSW index. Nothing in any .el source ever
|
||||
* called engram_req_lock, so that whole loop ran lock-free beside the workers.
|
||||
*
|
||||
* Measured consequence (2026-08-16): five crashes in ~4 minutes, all one bug —
|
||||
* SIGSEGV in search_layer<-vindex_insert<-eg_vindex_sync at address
|
||||
* 0x65646f4e6d617267 (little-endian ASCII "gramNode": a string being
|
||||
* dereferenced as an Elem vector pointer), plus a SIGABRT and a fault inside
|
||||
* xzm_realloc's freelist, i.e. corrupted allocator metadata. Confirmed by
|
||||
* bisection: replaying ALL 13,820 real dim-768 store vectors through the index
|
||||
* single-threaded under ASan is 100% clean, while two threads on one index trip
|
||||
* ThreadSanitizer instantly at engram_vindex.c:195 (visited_reset) — VIndex keeps
|
||||
* a SHARED visited-epoch scratch buffer, so even two concurrent READS stomp each
|
||||
* other's traversal state and walk bogus element indices. So this is purely a
|
||||
* concurrency defect, not a logic error in the HNSW code.
|
||||
*
|
||||
* Fix: a thread-local ownership depth lets engram entry points self-guard. A call
|
||||
* arriving on the awareness thread (depth 0) acquires the lock; one arriving from
|
||||
* inside an http_worker that already holds it (depth > 0) is a no-op, so there is
|
||||
* no self-deadlock on this NON-recursive mutex. Depth is a plain counter, never a
|
||||
* recursive-mutex count, which preserves engram_self_reify_beat_json's contract of
|
||||
* really releasing the lock mid-beat (see engram_req_unlock at the reify beat). */
|
||||
static __thread int _eg_req_depth = 0;
|
||||
void engram_req_unlock(void){ if(_eg_req_depth > 0) _eg_req_depth--; pthread_mutex_unlock(&g_engram_req_lock); }
|
||||
void engram_req_lock(void){ pthread_mutex_lock(&g_engram_req_lock); _eg_req_depth++; }
|
||||
/* Acquire only if this thread does not already hold the request lock.
|
||||
* Returns 1 if this call took ownership (caller must release), 0 if nested. */
|
||||
static int eg_guard_enter(void){
|
||||
if (_eg_req_depth > 0) return 0;
|
||||
pthread_mutex_lock(&g_engram_req_lock);
|
||||
_eg_req_depth++;
|
||||
return 1;
|
||||
}
|
||||
static void eg_guard_exit(int owned){
|
||||
if (!owned) return;
|
||||
if (_eg_req_depth > 0) _eg_req_depth--;
|
||||
pthread_mutex_unlock(&g_engram_req_lock);
|
||||
}
|
||||
|
||||
static void* http_worker(void* arg) {
|
||||
HttpWorkerArg* a = (HttpWorkerArg*)arg;
|
||||
@@ -1642,7 +1684,7 @@ static void* http_worker(void* arg) {
|
||||
(plen == 1 && path[0] == '/'))
|
||||
health_exempt = 1;
|
||||
}
|
||||
if (!health_exempt) pthread_mutex_lock(&g_engram_req_lock);
|
||||
if (!health_exempt) engram_req_lock(); /* tracks _eg_req_depth for eg_guard_enter */
|
||||
if (h) {
|
||||
el_val_t r = h(EL_STR(dispatch_method), EL_STR(path), EL_STR(body));
|
||||
const char* rs = EL_CSTR(r);
|
||||
@@ -1669,7 +1711,7 @@ static void* http_worker(void* arg) {
|
||||
}
|
||||
/* end of the engram critical section — the response is now a private malloc'd
|
||||
* copy; arena teardown + socket write touch no shared engram state. */
|
||||
if (!health_exempt) pthread_mutex_unlock(&g_engram_req_lock);
|
||||
if (!health_exempt) engram_req_unlock();
|
||||
el_request_end(); /* free all intermediate strings */
|
||||
_tl_http_head_only = head_only;
|
||||
http_send_response(fd, response);
|
||||
@@ -9727,7 +9769,9 @@ static int64_t engram_activate_beam(void) {
|
||||
v = d; return v;
|
||||
}
|
||||
|
||||
el_val_t engram_activate(el_val_t query, el_val_t depth) {
|
||||
/* Core activation. Callers must hold the engram request lock — reached only via
|
||||
* the engram_activate() wrapper below, which self-guards (see eg_guard_enter). */
|
||||
static el_val_t engram_activate_inner(el_val_t query, el_val_t depth) {
|
||||
EngramStore* g = engram_get();
|
||||
const char* q = EL_CSTR(query);
|
||||
int64_t max_depth = (int64_t)depth; if (max_depth <= 0) max_depth = 2;
|
||||
@@ -14186,6 +14230,17 @@ el_val_t engram_neighbors_json(el_val_t node_id, el_val_t max_depth, el_val_t di
|
||||
return el_wrap_str(b.buf);
|
||||
}
|
||||
|
||||
/* Public activation entry point. Serializes against the http_worker threads that
|
||||
* share g->nodes/g->edges and the global _eg_vindex — this is the guard the
|
||||
* awareness main thread (soul.el: awareness_run) was missing entirely. Nested
|
||||
* calls from a worker that already holds the lock pass straight through. */
|
||||
el_val_t engram_activate(el_val_t query, el_val_t depth) {
|
||||
int owned = eg_guard_enter();
|
||||
el_val_t r = engram_activate_inner(query, depth);
|
||||
eg_guard_exit(owned);
|
||||
return r;
|
||||
}
|
||||
|
||||
el_val_t engram_activate_json(el_val_t query, el_val_t depth) {
|
||||
/* Run two-layer engram_activate and serialize the result list to JSON.
|
||||
* Each entry includes both activation_strength (layer 1 background) and
|
||||
|
||||
Reference in New Issue
Block a user