runtime: publish the vector index instead of guarding it #143

Merged

will.anderson merged 3 commits from fix/awareness-thread-engram-race into dev

2026-08-16 16:33:32 +00:00

Author	SHA1	Message	Date
Neuron	8e9d88fc01	runtime: publish the vector index instead of guarding it El SDK CI - dev / build-and-test (pull_request) Failing after 10m59s Details The crash (SIGTRAP in engram_activate -> eg_vindex_sync -> vindex_insert -> _realloc) had three read paths mutating five process-global statics. engram_activate, eg_knn_for_node (whose own comment says "No writes.") and engram_geo_reify_run_json all called eg_vindex_sync, which frees the index, reallocs the seen-map and inserts — on a read. Three moves, in decreasing order of how much they dissolve: 1. Misfiled scratch is not shared state. visited/visit_epoch/visited_cap were never owned by the index; they are one traversal's local, hoisted into struct VIndex as an allocation optimisation. They want neither a lock nor a capability nor a pool — just to go back in the call frame. Two concurrent READS stomped each other purely because of this. 2. const IS the capability. Once the scratch leaves the struct, search reads and nothing else, so vindex_search takes a const VIndex. That is exactly what a capability-pointer ABI would have bought — a read path physically cannot call vindex_insert, enforced by the compiler on every future caller — for one qualifier instead of an ABI swept across hundreds of builtins. 3. What survives is publication, not ownership. HNSW insert is NOT an append: it rewires the neighbour links of already-existing elements and reallocs elems[], so the store's append-only property does not transfer to the index derived from it. eg_vindex_sync therefore splits into eg_vindex_maintain (exclusive, sole mutator) and eg_vindex_view (shared, returns const VIndex). A read path may demand that a current snapshot exist — a request to the owner, not a mutation by the reader. Write-side owner: eg_vindex_note_embedded hooks the embedding-ASSIGNMENT sites rather than the append sites, because a node with no embedding cannot be in a vector index — embedding assignment is the event that owns index membership. One O(log n) insert, no O(node_count) presence scan. This also retires the "STALENESS (honest tradeoff)" note where a lazily-embedded older node stayed invisible to route_nearest/autoconnect until a full rebuild (the embed-gap #20 shape). Evidence. The existing harness conflated two hazards, which is why fixing half of it read as failure. Split into four: single (3000 vec, ASan+UBSan) clean -> clean readers (4 readers, no writer, TSan) RACE -> clean unsynchronized (writer+reader, bare) race -> race, expected forever published (owner + 4 readers) n/a -> clean, 3000/3000 landed RESULT: PASS. recall@10 = 0.9365 at ef_search=128 (gate >= 0.90); determinism byte-identical across two independent builds. The unsynchronized half is now permanently expected to race, deliberately: it is the executable proof that the boundary must live above the data structure, not inside it. fb32d15's guard is KEPT, correcting this design's own section 5. Measured, it guards TWO structures and only one was converted here: g->nodes/g->edges are realloc'd in place (el_runtime.c:7618,7629) and engram_activate_inner's embed-backfill writes n->emb through exactly such a borrowed pointer. Deleting the guard reintroduces a measured 11171->9579 edge loss. Its comment is narrowed to the RAM graph and the deletion precondition named. That corrects the ordering claim too: the residual is not one ABI that dissolves everything at once, it is a PROPERTY applied per structure. Residues evaporate in the order the property is applied, and a residue whose structure has not been converted must be left standing.	2026-08-16 11:29:17 -05:00
bigmerge	e99a4640e2	test: regression harness for the vindex concurrency crash Promotes the two throwaway sanitizer harnesses used to diagnose the 2026-08-16 soul crash into engram/test/ so the bug cannot silently regress. The harness has two halves and the PAIR is the point — it is what localises the defect to concurrency rather than to HNSW logic: single 3000 clustered vectors, one thread, ASan+UBSan. The CONTROL. Must always be clean. During diagnosis this cleared all 13,820 real dim-768 vectors from the live store, which DISPROVED an inspection-derived hypothesis about an out-of-bounds reverse-link write at engram_vindex.c:340. concurrent writer + reader on one shared index, TSan. Currently reports a race at engram_vindex.c:195 (visited_reset) reached from both vindex_search and vindex_insert, because VIndex still owns its visited[]/visit_epoch scratch — so even two concurrent READS corrupt each other's traversal. Verified: half 1 passes, half 2 reproduces the race. Gated on EXPECT_RACE, default 1, so the concurrent half documents the known defect without failing the suite today. When the visited set moves to a per-query checkout pool (hnswlib VisitedListPool style — NOT thread_local, since http_worker is a thread per connection and a __thread buffer would leak ~55KB per connection), flip EXPECT_RACE=0 and it becomes a real gate.	2026-08-16 11:29:17 -05:00
bigmerge	bdc1f99fb9	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.	2026-08-16 11:29:17 -05:00

Author

SHA1

Message

Date

Neuron

8e9d88fc01

runtime: publish the vector index instead of guarding it

El SDK CI - dev / build-and-test (pull_request) Failing after 10m59s

Details

The crash (SIGTRAP in engram_activate -> eg_vindex_sync -> vindex_insert ->
_realloc) had three read paths mutating five process-global statics.
engram_activate, eg_knn_for_node (whose own comment says "No writes.") and
engram_geo_reify_run_json all called eg_vindex_sync, which frees the index,
reallocs the seen-map and inserts — on a read.

Three moves, in decreasing order of how much they dissolve:

1. Misfiled scratch is not shared state. visited/visit_epoch/visited_cap
   were never owned by the index; they are one traversal's local, hoisted
   into struct VIndex as an allocation optimisation. They want neither a
   lock nor a capability nor a pool — just to go back in the call frame.
   Two concurrent READS stomped each other purely because of this.

2. const IS the capability. Once the scratch leaves the struct, search
   reads and nothing else, so vindex_search takes a const VIndex*. That is
   exactly what a capability-pointer ABI would have bought — a read path
   physically cannot call vindex_insert, enforced by the compiler on every
   future caller — for one qualifier instead of an ABI swept across
   hundreds of builtins.

3. What survives is publication, not ownership. HNSW insert is NOT an
   append: it rewires the neighbour links of already-existing elements and
   reallocs elems[], so the store's append-only property does not transfer
   to the index derived from it. eg_vindex_sync therefore splits into
   eg_vindex_maintain (exclusive, sole mutator) and eg_vindex_view (shared,
   returns const VIndex*). A read path may demand that a current snapshot
   exist — a request to the owner, not a mutation by the reader.

Write-side owner: eg_vindex_note_embedded hooks the embedding-ASSIGNMENT
sites rather than the append sites, because a node with no embedding cannot
be in a vector index — embedding assignment is the event that owns index
membership. One O(log n) insert, no O(node_count) presence scan. This also
retires the "STALENESS (honest tradeoff)" note where a lazily-embedded
older node stayed invisible to route_nearest/autoconnect until a full
rebuild (the embed-gap #20 shape).

Evidence. The existing harness conflated two hazards, which is why fixing
half of it read as failure. Split into four:

  single (3000 vec, ASan+UBSan)          clean  ->  clean
  readers (4 readers, no writer, TSan)   RACE   ->  clean
  unsynchronized (writer+reader, bare)   race   ->  race, expected forever
  published (owner + 4 readers)          n/a    ->  clean, 3000/3000 landed

RESULT: PASS. recall@10 = 0.9365 at ef_search=128 (gate >= 0.90);
determinism byte-identical across two independent builds.

The unsynchronized half is now permanently expected to race, deliberately:
it is the executable proof that the boundary must live above the data
structure, not inside it.

fb32d15's guard is KEPT, correcting this design's own section 5. Measured,
it guards TWO structures and only one was converted here: g->nodes/g->edges
are realloc'd in place (el_runtime.c:7618,7629) and engram_activate_inner's
embed-backfill writes n->emb through exactly such a borrowed pointer.
Deleting the guard reintroduces a measured 11171->9579 edge loss. Its
comment is narrowed to the RAM graph and the deletion precondition named.

That corrects the ordering claim too: the residual is not one ABI that
dissolves everything at once, it is a PROPERTY applied per structure.
Residues evaporate in the order the property is applied, and a residue
whose structure has not been converted must be left standing.

2026-08-16 11:29:17 -05:00

bigmerge

e99a4640e2

test: regression harness for the vindex concurrency crash

Promotes the two throwaway sanitizer harnesses used to diagnose the
2026-08-16 soul crash into engram/test/ so the bug cannot silently regress.

The harness has two halves and the PAIR is the point — it is what localises
the defect to concurrency rather than to HNSW logic:

  single      3000 clustered vectors, one thread, ASan+UBSan. The CONTROL.
              Must always be clean. During diagnosis this cleared all 13,820
              real dim-768 vectors from the live store, which DISPROVED an
              inspection-derived hypothesis about an out-of-bounds
              reverse-link write at engram_vindex.c:340.

  concurrent  writer + reader on one shared index, TSan. Currently reports a
              race at engram_vindex.c:195 (visited_reset) reached from both
              vindex_search and vindex_insert, because VIndex still owns its
              visited[]/visit_epoch scratch — so even two concurrent READS
              corrupt each other's traversal.

Verified: half 1 passes, half 2 reproduces the race.

Gated on EXPECT_RACE, default 1, so the concurrent half documents the known
defect without failing the suite today. When the visited set moves to a
per-query checkout pool (hnswlib VisitedListPool style — NOT thread_local,
since http_worker is a thread per connection and a __thread buffer would leak
~55KB per connection), flip EXPECT_RACE=0 and it becomes a real gate.

2026-08-16 11:29:17 -05:00

bigmerge

bdc1f99fb9

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.

2026-08-16 11:29:17 -05:00

runtime: publish the vector index instead of guarding it #143

3 Commits