fix(mcp): heartbeat deadlock, chart inline default, lock-free status fast path (#118)#126
Merged
StefanSteiner merged 4 commits intoJun 9, 2026
Conversation
Duration is already imported at file scope (line 39); the local re-import inside the #[cfg(windows)] function body triggered -W redundant-imports.
maybe_send_heartbeat() was re-locking self.engine inside with_engine(), which already holds that mutex. Pass the daemon health port from the caller's engine reference instead of re-acquiring the lock. Co-authored-by: Cursor <cursoragent@cursor.com>
MCP clients (Cursor, Claude Desktop) could not render charts without a separate file-read round-trip because the chart tool defaulted to inline=false (disk-only). Flip the default so charts return base64 image bytes inline, eliminating the extra step. Co-authored-by: Cursor <cursoragent@cursor.com>
StefanSteiner
force-pushed
the
ssteiner/fix-heartbeat-deadlock
branch
from
34233eb to
8792ef3
Compare
…au#118) The `status` tool previously went through `with_engine`, which takes the global engine mutex and blocks until any in-progress tool call releases it. If a data-plane operation was stalled (e.g. a half-open connection blocking a read, or a legitimately long analytics query), `status` would hang behind it indefinitely with no timeout. Fix: `status` uses `try_lock()` instead of blocking `lock()`. If the engine mutex is available, it runs the full `Engine::status()` (table counts, disk usage, endpoints). If the mutex is held OR the engine is not yet initialized, it returns a **degraded-but-instant** response built from metadata available without the engine: daemon health (via `discover()` — a fast file-read + single PING to the known port, max ~300ms; deliberately NOT `find_running_daemon()` which adds a 16-port scan that could block 5s), the persistent path, watchers, attachments, and read-only state. The response includes `engine_busy: true`. Scope: this decouples `status` from the engine mutex so diagnostics never hang. Other data-plane tool calls (`query`, `execute`, etc.) still serialize behind the engine lock — the general serialization issue (connection pool or per-op timeout) is tracked separately. Partially addresses tableau#118.
StefanSteiner
changed the title
Merged
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
1 participant
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
Four fixes on this branch:
Fix heartbeat deadlock.
maybe_send_heartbeat()re-lockedself.engineinsidewith_engine(), which already holds that mutex — under contention this would deadlock and freeze every tool call. The fix passes the daemon health port directly from the caller's already-held engine reference. Fixes MCP: Windows MCP v0.5.2 is broken. Trying to run queries will hang the MCP. This will be fixed soon. #125.Default chart
inlineto true. MCP clients (Cursor, Claude Desktop) couldn't render charts without a separate file-read round-trip because the chart tool defaulted toinline=false. Flipping the default means charts return base64 image bytes inline for immediate display. Callers can still setinline=falsefor disk-only output.Remove unused
Durationimport inconnect_named_pipe(Windows-only warning from the keepalive change).Lock-free
statusfast path (partially addresses MCP: single engine mutex + no op timeout lets one stalled hyperd call hang all tool calls #118). Thestatustool previously went throughwith_engine, blocking behind the global engine mutex. If another tool call was stalled (half-open connection, long query),statuswould hang indefinitely. Nowstatususestry_lock():discover()— max ~300ms, no port scan; persistent path; watchers; attachments; version) withengine_busy: true.This decouples diagnostics from the data-plane lock so
statusnever hangs. Other tool calls (query,execute, etc.) still serialize behind the engine mutex — the general serialization issue (connection pool / per-op timeout) remains tracked in MCP: single engine mutex + no op timeout lets one stalled hyperd call hang all tool calls #118.Test plan
cargo test -p hyperdb-mcp— all tests passcargo clippy --all-targets -- -D warningscleanstatuswhile a long query is running — confirm instant response withengine_busy: truestatusnormally — confirm full response withengine_busy: falsechart(...)withoutinline— confirm base64 image returned inline