Fix first-embed deadlock on Windows (sentence-transformers) - #171
Open
danieltsai0423 wants to merge 1 commit into
Open
Fix first-embed deadlock on Windows (sentence-transformers)#171danieltsai0423 wants to merge 1 commit into
danieltsai0423 wants to merge 1 commit into
Conversation
The first embed() call lazily imports sentence_transformers (which pulls in
scipy's native BLAS extension) via SentenceTransformerEmbedder._ensure_model.
When that first-time native import runs inside the already-running asyncio
event loop, it deadlocks in the Windows DLL loader lock and the MCP server
hangs forever on the first memory_add_fact / memory_search -- regardless of
which thread runs the import. Confirmed with two py-spy dumps wedged at
scipy/linalg/blas.py create_module with zero CPU progress over minutes.
Fix:
- embeddings/sentence_transformers.py: offload _ensure_model() to the executor
in embed()/embed_batch() so the model load never runs on the event-loop
thread, and add a preload() helper that imports the native stack eagerly.
- cli/main.py (mcp serve): call preload() on the main thread before
asyncio.run() when a local sentence-transformers embedder is configured, so
the native import happens in a safe, no-loop context. No-op for cloud
embedders and when sentence-transformers isn't installed.
- tests: regression tests that embed()/embed_batch() load the model off the
event-loop thread, plus preload() unit tests.
Verified end-to-end on Windows 11 / Python 3.10: memory_add_fact returns
{"stored": true} in ~9s instead of hanging indefinitely.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@danieltsai0423 is attempting to deploy a commit to the lyonwj's projects Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Windows, the MCP server hangs forever on the first
memory_add_fact/memory_searchwhen using a localsentence-transformersembedder (neo4j-agent-memory mcp serve --embedding sentence-transformers/...). The tool call never returns and never errors — the process just wedges.Root cause
SentenceTransformerEmbedder.embed()correctly offloadsmodel.encode()to a thread executor, but callsself._ensure_model()synchronously on the asyncio event-loop thread._ensure_model()performs the firstimport sentence_transformers, which pulls in scipy's native BLAS extension.Importing that native extension for the first time while the asyncio event loop is already running deadlocks in the Windows DLL loader lock — and it does so regardless of which thread runs the import.
Confirmed with two
py-spydumps of the hung process taken seconds apart: both wedged at the exact same frame, with zero CPU progress:The model was already fully downloaded (no network activity), Neo4j was healthy, and the process used ~4.5s CPU across several hours — a deadlock, not slow loading or OOM.
Why offloading
_ensure_modelalone is not enoughMy first attempt was to move
_ensure_model()into the samerun_in_executorcall.py-spythen showed the import running on a worker thread instead of the loop thread — but it still deadlocked at the samescipy/linalg/blas.pyframe. The trigger is "first nativesentence_transformersimport while an event loop is running", not which thread performs it. The native import must happen before the loop starts.Fix
embeddings/sentence_transformers.py— offload_ensure_model()to the executor inembed()/embed_batch()(so a model load never blocks the event loop), and add apreload()helper that imports the native stack eagerly.cli/main.py(mcp serve) — callpreload()on the main thread beforeasyncio.run()when the configured embedder resolves toSentenceTransformersProvider, so the native import happens in a safe, no-loop context. No-op for cloud embedders and whensentence-transformersisn't installed.embed()/embed_batch()load the model off the event-loop thread, pluspreload()unit tests.Verification
ruff checkandruff format --checkare clean.--embedding sentence-transformers/paraphrase-multilingual-MiniLM-L12-v2 --embedding-dimensions 384:memory_add_factnow returns{"stored": true}in ~9s instead of hanging indefinitely.Notes for reviewers
make check(mypy--strict+ ty) locally, as it needsuv sync --all-extras(torch etc.). The changes use only stdlib + internal, already-typed APIs and do not alter the existing typed surface, but please confirm in CI._ensure_model()on the event-loop thread is arguably a latent issue on all platforms (the first embed blocks the whole server while the model loads); the Windows DLL loader-lock deadlock is the platform-specific escalation of the same root cause.