Skip to content

Commit a61ffb2

Browse files
ericdalloeca-agent
andcommitted
Dispatch handlers off protocol thread to prevent ECA unresponsiveness
The jsonrpc4clj protocol thread processes all client messages sequentially on a single thread. Any blocking handler (MCP init/stop, inline completion, provider login, etc.) would stall all subsequent messages. Adopt clojure-lsp's `eventually` pattern (promesa p/thread) to dispatch request and notification handlers to background threads, releasing the protocol thread immediately. - Add `eventually` macro for request handlers (returns promise, jsonrpc4clj sends response when it resolves) - Add `async-notify` macro for notification handlers (fire-and-forget with error logging) - Wrap all request handlers except initialize/shutdown (lifecycle ordering) - Wrap all notification handlers except exit/toolCallApprove/toolCallReject/ promptStop (lifecycle and critical-path control flow) - Fix MCP stop-server! to cancel in-progress init and use fire-and-forget for force-stop path - Simplify MCP start-server!/restart-server! by removing redundant daemon threads (async-notify already provides background thread) - Extract with-init-thread macro for init thread lifecycle tracking 🤖 Generated with [eca](https://eca.dev) Co-Authored-By: eca <git@eca.dev>
1 parent 2887379 commit a61ffb2

File tree

3 files changed

+77
-47
lines changed

3 files changed

+77
-47
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
## Unreleased
44

55
- Bump plumcp to 0.2.0-beta6.
6+
- Fix MCP server start/stop blocking the protocol thread, causing ECA to become unresponsive.
7+
- Dispatch request and notification handlers off the protocol thread to prevent blocking.
68

79
## 0.122.1
810

src/eca/features/tools/mcp.clj

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,18 @@
5050
(.interrupt thread))
5151
(reset! init-threads* {}))
5252

53+
(defmacro ^:private with-init-thread
54+
"Registers the current thread in `init-threads*` for the given server name,
55+
executes body, and deregisters in a finally block. Allows `stop-server!` and
56+
`shutdown!` to interrupt in-progress initializations."
57+
[server-name & body]
58+
`(let [name# ~server-name]
59+
(register-init-thread! name# (Thread/currentThread))
60+
(try
61+
~@body
62+
(finally
63+
(deregister-init-thread! name#)))))
64+
5365
(def ^:private env-var-regex
5466
#"\$(\w+)|\$\{([^}]+)\}")
5567

@@ -421,28 +433,32 @@
421433
(on-server-updated (->server server-name server-config :disabled db))
422434
(let [t (Thread.
423435
(fn []
424-
(try
425-
(initialize-server! server-name db* config metrics on-server-updated)
426-
(finally
427-
(deregister-init-thread! server-name)))))]
436+
(with-init-thread server-name
437+
(initialize-server! server-name db* config metrics on-server-updated))))]
428438
(.setName t (str "mcp-init-" server-name))
429439
(.setDaemon t true)
430-
(register-init-thread! server-name t)
431440
(.start t))))))))
432441

433442
(def ^:private disconnect-timeout-ms 3000)
434443

435444
(defn stop-server! [name db* config {:keys [on-server-updated]}]
445+
(when-let [^Thread init-thread (get @init-threads* name)]
446+
(logger/info logger-tag (format "Interrupting in-progress init thread for server '%s'" name))
447+
(.interrupt init-thread)
448+
(deregister-init-thread! name))
436449
(when-let [{:keys [client http-client]} (get-in @db* [:mcp-clients name])]
437450
(let [server-config (get-in config [:mcpServers name])]
438451
(swap! db* assoc-in [:mcp-clients name :status] :stopping)
439452
(on-server-updated (->server name server-config :stopping @db*))
440453
(let [f (future (try (pmc/disconnect! client) (catch Exception _ nil)))]
441454
(when-not (deref f disconnect-timeout-ms nil)
442455
(logger/warn logger-tag (format "Timeout disconnecting MCP server %s, forcing transport stop" name))
443-
(if http-client
444-
(try (pp/stop! http-client) (catch Exception _))
445-
(try (pp/stop-client-transport! (pcs/?transport client) false) (catch Exception _)))))
456+
;; Fire-and-forget: pp/stop! can block indefinitely on HttpClient.close()
457+
;; awaiting internal thread termination, so we must not block the caller.
458+
(future
459+
(if http-client
460+
(try (pp/stop! http-client) (catch Exception _))
461+
(try (pp/stop-client-transport! (pcs/?transport client) false) (catch Exception _))))))
446462
(swap! db* assoc-in [:mcp-clients name :status] :stopped)
447463
(on-server-updated (->server name server-config :stopped @db*))
448464
(swap! db* update :mcp-clients dissoc name)
@@ -452,7 +468,8 @@
452468
(when-let [server-config (get-in config [:mcpServers name])]
453469
(when (get server-config :disabled false)
454470
(logger/info logger-tag (format "Starting MCP server %s from manual request despite :disabled=true" name)))
455-
(initialize-server! name db* config metrics on-server-updated)))
471+
(with-init-thread name
472+
(initialize-server! name db* config metrics on-server-updated))))
456473

457474
(defn ^:private open-browser! [^String url]
458475
(try
@@ -495,20 +512,12 @@
495512
(open-browser! authorization-endpoint)))))
496513

497514
(defn ^:private restart-server!
498-
"Stop the server if running, then spawn a daemon thread to re-initialize it."
515+
"Stop the server if running, then re-initialize it on the current thread."
499516
[name db* config metrics on-server-updated]
500517
(when (get-in @db* [:mcp-clients name :client])
501518
(stop-server! name db* config {:on-server-updated on-server-updated}))
502-
(let [t (Thread.
503-
(fn []
504-
(try
505-
(initialize-server! name db* config metrics on-server-updated)
506-
(finally
507-
(deregister-init-thread! name)))))]
508-
(.setName t (str "mcp-init-" name))
509-
(.setDaemon t true)
510-
(register-init-thread! name t)
511-
(.start t)))
519+
(with-init-thread name
520+
(initialize-server! name db* config metrics on-server-updated)))
512521

513522
(defn logout-server!
514523
"Logout from an MCP server by clearing stored OAuth credentials and restarting it."

src/eca/server.clj

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
[eca.shared :as shared :refer [assoc-some]]
1515
[jsonrpc4clj.io-server :as io-server]
1616
[jsonrpc4clj.liveness-probe :as liveness-probe]
17-
[jsonrpc4clj.server :as jsonrpc.server]))
17+
[jsonrpc4clj.server :as jsonrpc.server]
18+
[promesa.core :as p]))
1819

1920
(set! *warn-on-reflection* true)
2021

@@ -37,13 +38,31 @@
3738
(defn ^:private with-config [components]
3839
(assoc components :config (config/all @(:db* components))))
3940

41+
(defmacro ^:private eventually
42+
"Dispatches handler body to a background thread, returning a promise.
43+
Releases the protocol thread immediately. jsonrpc4clj resolves the promise
44+
and sends the JSON-RPC response when the work completes."
45+
[& body]
46+
`(p/thread ~@body))
47+
48+
(defmacro ^:private async-notify
49+
"Dispatches notification handler body to a background thread.
50+
Releases the protocol thread immediately. Exceptions are logged since
51+
notification handlers have no response channel."
52+
[& body]
53+
`(p/thread
54+
(try
55+
~@body
56+
(catch Throwable e#
57+
(logger/error e# "[server] Error in async notification handler")))))
58+
4059
(defmethod jsonrpc.server/receive-request "initialize" [_ {:keys [server] :as components} params]
4160
(when-let [parent-process-id (:process-id params)]
4261
(liveness-probe/start! parent-process-id log-wrapper-fn #(exit server)))
4362
(handlers/initialize components params))
4463

4564
(defmethod jsonrpc.server/receive-notification "initialized" [_ components _params]
46-
(handlers/initialized (with-config components)))
65+
(async-notify (handlers/initialized (with-config components))))
4766

4867
(defmethod jsonrpc.server/receive-request "shutdown" [_ components _params]
4968
(handlers/shutdown (with-config components)))
@@ -52,19 +71,19 @@
5271
(exit server))
5372

5473
(defmethod jsonrpc.server/receive-notification "workspace/didChangeWorkspaceFolders" [_ components params]
55-
(handlers/workspace-did-change-folders (with-config components) params))
74+
(async-notify (handlers/workspace-did-change-folders (with-config components) params)))
5675

5776
(defmethod jsonrpc.server/receive-request "chat/prompt" [_ components params]
58-
(handlers/chat-prompt (with-config components) params))
77+
(eventually (handlers/chat-prompt (with-config components) params)))
5978

6079
(defmethod jsonrpc.server/receive-request "chat/queryContext" [_ components params]
61-
(handlers/chat-query-context (with-config components) params))
80+
(eventually (handlers/chat-query-context (with-config components) params)))
6281

6382
(defmethod jsonrpc.server/receive-request "chat/queryFiles" [_ components params]
64-
(handlers/chat-query-files (with-config components) params))
83+
(eventually (handlers/chat-query-files (with-config components) params)))
6584

6685
(defmethod jsonrpc.server/receive-request "chat/queryCommands" [_ components params]
67-
(handlers/chat-query-commands (with-config components) params))
86+
(eventually (handlers/chat-query-commands (with-config components) params)))
6887

6988
(defmethod jsonrpc.server/receive-notification "chat/toolCallApprove" [_ components params]
7089
(handlers/chat-tool-call-approve (with-config components) params))
@@ -76,62 +95,62 @@
7695
(handlers/chat-prompt-stop (with-config components) params))
7796

7897
(defmethod jsonrpc.server/receive-request "chat/delete" [_ components params]
79-
(handlers/chat-delete (with-config components) params))
98+
(eventually (handlers/chat-delete (with-config components) params)))
8099

81100
(defmethod jsonrpc.server/receive-request "chat/clear" [_ components params]
82-
(handlers/chat-clear (with-config components) params))
101+
(eventually (handlers/chat-clear (with-config components) params)))
83102

84103
(defmethod jsonrpc.server/receive-request "chat/rollback" [_ components params]
85-
(handlers/chat-rollback (with-config components) params))
104+
(eventually (handlers/chat-rollback (with-config components) params)))
86105

87106
(defmethod jsonrpc.server/receive-notification "mcp/stopServer" [_ components params]
88-
(handlers/mcp-stop-server (with-config components) params))
107+
(async-notify (handlers/mcp-stop-server (with-config components) params)))
89108

90109
(defmethod jsonrpc.server/receive-notification "mcp/startServer" [_ components params]
91-
(handlers/mcp-start-server (with-config components) params))
110+
(async-notify (handlers/mcp-start-server (with-config components) params)))
92111

93112
(defmethod jsonrpc.server/receive-notification "mcp/connectServer" [_ components params]
94-
(handlers/mcp-connect-server (with-config components) params))
113+
(async-notify (handlers/mcp-connect-server (with-config components) params)))
95114

96115
(defmethod jsonrpc.server/receive-notification "mcp/logoutServer" [_ components params]
97-
(handlers/mcp-logout-server (with-config components) params))
116+
(async-notify (handlers/mcp-logout-server (with-config components) params)))
98117

99118
(defmethod jsonrpc.server/receive-notification "mcp/disableServer" [_ components params]
100-
(handlers/mcp-disable-server (with-config components) params))
119+
(async-notify (handlers/mcp-disable-server (with-config components) params)))
101120

102121
(defmethod jsonrpc.server/receive-notification "mcp/enableServer" [_ components params]
103-
(handlers/mcp-enable-server (with-config components) params))
122+
(async-notify (handlers/mcp-enable-server (with-config components) params)))
104123

105124
(defmethod jsonrpc.server/receive-request "mcp/updateServer" [_ components params]
106-
(handlers/mcp-update-server (with-config components) params))
125+
(eventually (handlers/mcp-update-server (with-config components) params)))
107126

108127
(defmethod jsonrpc.server/receive-notification "chat/selectedAgentChanged" [_ components params]
109-
(handlers/chat-selected-agent-changed (with-config components) params))
128+
(async-notify (handlers/chat-selected-agent-changed (with-config components) params)))
110129

111130
;; Legacy: backward compat for clients using old method name
112131
(defmethod jsonrpc.server/receive-notification "chat/selectedBehaviorChanged" [_ components params]
113-
(handlers/chat-selected-agent-changed (with-config components) params))
132+
(async-notify (handlers/chat-selected-agent-changed (with-config components) params)))
114133

115134
(defmethod jsonrpc.server/receive-notification "chat/selectedModelChanged" [_ components params]
116-
(handlers/chat-selected-model-changed (with-config components) params))
135+
(async-notify (handlers/chat-selected-model-changed (with-config components) params)))
117136

118137
(defmethod jsonrpc.server/receive-request "providers/list" [_ components params]
119-
(handlers/providers-list (with-config components) params))
138+
(eventually (handlers/providers-list (with-config components) params)))
120139

121140
(defmethod jsonrpc.server/receive-request "providers/login" [_ components params]
122-
(handlers/providers-login (with-config components) params))
141+
(eventually (handlers/providers-login (with-config components) params)))
123142

124143
(defmethod jsonrpc.server/receive-request "providers/loginInput" [_ components params]
125-
(handlers/providers-login-input (with-config components) params))
144+
(eventually (handlers/providers-login-input (with-config components) params)))
126145

127146
(defmethod jsonrpc.server/receive-request "providers/logout" [_ components params]
128-
(handlers/providers-logout (with-config components) params))
147+
(eventually (handlers/providers-logout (with-config components) params)))
129148

130149
(defmethod jsonrpc.server/receive-request "completion/inline" [_ components params]
131-
(handlers/completion-inline (with-config components) params))
150+
(eventually (handlers/completion-inline (with-config components) params)))
132151

133152
(defmethod jsonrpc.server/receive-request "rewrite/prompt" [_ components params]
134-
(handlers/rewrite-prompt (with-config components) params))
153+
(eventually (handlers/rewrite-prompt (with-config components) params)))
135154

136155
(defn ^:private monitor-server-logs [log-ch]
137156
;; NOTE: if this were moved to `initialize`, after timbre has been configured,
@@ -236,4 +255,4 @@
236255
server (io-server/stdio-server {:log-ch log-ch
237256
:trace-ch log-ch
238257
:trace-level (if verbose? "verbose" "off")})]
239-
(start-server! server))))
258+
(start-server! server))))

0 commit comments

Comments
 (0)