Blocking thread.join() from async handlers freezes the MCP event loop (observations.py:140)

[emp3thy]

Severity: high

Location: better_memory/async_bridge.py:59 (SQLite path) and better_memory/mcp/handlers/observations.py:140 (agentcore path)

Merged: same defect — synchronous I/O called directly from async def handlers with no to_thread/run_in_executor — at two families of call sites.

What breaks. run_async_in_worker does thread.start() then thread.join(timeout=timeout) (async_bridge.py:57-59) with _WORKER_TIMEOUT = 15.0 (sync_embed.py:37, :84). join releases the GIL but does not yield to asyncio. Reached synchronously from async handlers:

• ReflectionToolHandlers.retrieve (async, reflections.py:67) → sync self._backend.retrieve (reflections.py:126, sqlite.py:137-156) → _heal_missing_embeddings (reflection.py:1509 → embed_batch :1297) and embed_text (:1511) — two consecutive blocking bridge calls, up to 30 s.
• SemanticToolHandlers.semantic_observe/semantic_update (semantics.py:44, 59) → semantic.py:101, 123.
• synthesize_next_apply (reflections.py:175) → _apply_new/_apply_augment (reflection.py:835, 991), one blocking embed per reflection.
• Agentcore mode: handlers/observations.py:140, handlers/reflections.py:126, handlers/semantics.py:51,94,115,124, handlers/sessions.py:63,107,125,141 all call methods storage/protocol.py declares sync. _fetch_reflection_buckets fans out on a ThreadPoolExecutor then blocks on future.result() (agentcore.py:485-491), and _retry_on_transient_404 does time.sleep(10.0) × max_attempts=3 (agentcore.py:1198-1222), reachable from record_use (:1376) and _credit_counter (:2275).

The invariant asserted at mcp/server.py:192-204 — "the MCP stdio transport serialises requests" — is false: mcp/server/lowlevel/server.py:673-683 does tg.start_soon(self._handle_message, ...) per message, so _call_tool coroutines are genuinely concurrent. A grep for to_thread|run_in_executor across better_memory/ hits only storage/agentcore.py:271, 1122 — nothing in the handler path. AgentCoreBackend.observe already applies the correct pattern and comments "Without this, every await observe(...) freezes the loop for the duration of the AWS call" (:266-287); that mitigation is defeated whenever any sync path above is running.

Refuter correction (agentcore half, 2/3). One refuter argued the non-yielding behavior is a documented design invariant, not an oversight: mcp/server.py:199-204 states the design depends on _call_tool running one invocation at a time because the services share one memory_conn. Treat the agentcore call sites as "the invariant is wrong and load-bearing" rather than "someone forgot an await" — fixing it requires addressing connection sharing, not just wrapping calls in to_thread.

Failure scenario. A parallel tool batch of memory.retrieve + memory.observe. The SDK starts both. Retrieve enters _heal_missing_embeddings → thread.join(15.0) while Ollama trickles bytes (per-read timeout never fires, the 15 s bridge backstop is what trips). For 15 s — 30 s with the second embed — the loop thread is parked: the observe task's await self._embedder.embed(...) (observation.py:204) is never scheduled, no stdio message is dispatched, no ping is answered, notifications/cancelled cannot be delivered. In agentcore mode a memory.credit/memory.record_use on a record AWS 404s (retired record, or the documented ~10 s create→update lag) freezes the entire server for 20 s of time.sleep.

Fix. Wrap every sync backend/embedder call reached from an async def handler in await asyncio.to_thread(...), and replace the shared-connection assumption in mcp/server.py:192-204 with per-call connections or an explicit async lock so the offload is actually safe.

---

_{Filed from an automated adversarial bug hunt (bughunt workflow, run
wf_43511264-124, 87 agents). Each finding was attacked by three independent
refuters from different angles — reproduce, reachability, and library/language
semantics — and survived a majority. Where a refuter corrected or narrowed the
claim, that correction is preserved inline above rather than dropped. Line
numbers reflect the tree at the time of the run; verify before acting.}

[emp3thy]

Not for bugbot / automated fix — this requires a design decision.

A reviewer explicitly argued the non-yielding behaviour is a documented, load-bearing invariant (mcp/server.py:199-204: the services share one memory_conn, so serialised execution is what makes that safe) rather than a forgotten await. Wrapping the calls in to_thread without first addressing connection sharing would introduce concurrent writes on a shared connection — a worse bug than the one being fixed.

Please leave this for a human. An automated patch here is likely to be confidently wrong: the diff will look small and plausible while silently picking one side of an unresolved design question.