Shut down superseded MCP managers on refresh - #29608
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
MCP refresh replaced the published connection manager without shutting down the manager it superseded. If another task retained that old manager, its stdio MCP processes stayed alive and accumulated across refreshes.
Atomically swap in the refreshed manager, then explicitly shut down the exact manager returned by the swap. Add a process-level regression test that retains the old manager during refresh and verifies its stdio process exits while the replacement remains available.
Context
Explicit cleanup was lost when manager publication moved to
ArcSwap. Dropping the old manager is not a reliable shutdown boundary because active callers can retain itsArcand underlying client process handles.