Skip to content

fix(render-mcp): close LOVR launch context on respawn (fixes #196) - #218

Merged
wenxind-nvidia merged 1 commit into
mainfrom
fix_render_mcp_managedprocess_leak_issue196
Jun 10, 2026
Merged

fix(render-mcp): close LOVR launch context on respawn (fixes #196)#218
wenxind-nvidia merged 1 commit into
mainfrom
fix_render_mcp_managedprocess_leak_issue196

Conversation

@wenxind-nvidia

Copy link
Copy Markdown
Collaborator

Problem

Fixes #196. render-mcp leaked a ManagedProcess context on every LOVR respawn.

In agent-mcp-servers/render-mcp/render_mcp/__main__.py, each successful start_lovr_once did await self._stack.enter_async_context(ManagedProcess(...)), where self._stack is the process-lifetime AsyncExitStack from _serve. When LOVR exits, _watch reset _lovr_started = False and a later start_xr entered a new ManagedProcess into the same stack — so the previous context's teardown (its two _forward pipe tasks + open log-file handle) never ran until whole-process shutdown. Across N restarts, N-1 dead contexts accumulated.

Fix

  • Each LOVR launch gets its own AsyncExitStack (self._launch_stack); the ManagedProcess is entered into that, not the app-lifetime stack.
  • _watch closes the per-launch stack as soon as the child exits, before resetting _lovr_started (i.e. before a respawn is allowed) — running the ManagedProcess teardown (cancel pipe tasks, close log sink). This is safe post-exit: ManagedProcess skips terminate() when returncode is already set.
  • A single _aclose_live_launch callback is registered once on the app-lifetime stack to cover the shutdown-while-LOVR-still-running case, so nothing accumulates per launch.

Net: the app-lifetime stack now holds one callback instead of one dead ManagedProcess per restart.

Test

Added test_lovr_respawn_closes_previous_launch_context (in the existing local/gpu-marked tests/test_local_render_mcp.py, which already stubs LOVR). It asserts:

  • after a simulated LOVR exit + _watch, the previous launch context is closed (__aexit__ ran) and _launch_stack is cleared;
  • a second start_lovr_once opens a fresh context with no accumulation (exactly one closed context);
  • the still-live context is closed when the app-lifetime stack unwinds at shutdown.

No pyproject.toml change (stdlib only), so no DEPENDENCIES.md update.

Note: the regression test is in the gpu-marked local suite (not in the CI matrix) and I couldn't run it in this environment (no local pytest/zmq). The change + test were verified by py_compile and inspection; please run uv run pytest tests/test_local_render_mcp.py -k respawn on a dev box to confirm.

🤖 Generated with Claude Code

…s/handles)

render-mcp parked every LOVR launch in the process-lifetime AsyncExitStack, so
on each respawn the previous ManagedProcess context (its two _forward pipe
tasks + open log-file handle) was never torn down until whole-process
shutdown — N restarts leaked N-1 dead contexts.

Each launch now gets its own AsyncExitStack, closed inside _watch as soon as
the child exits and before a respawn is allowed. A single _aclose_live_launch
callback registered once on the app-lifetime stack covers the
shutdown-while-LOVR-running case, so nothing accumulates per launch. The
teardown is safe post-exit: ManagedProcess skips terminate when returncode is
already set and just cancels the pipe tasks and closes the sink.

Adds a regression test (local/gpu-marked) asserting the previous launch
context is closed on respawn and the live one is closed at shutdown.

Fixes #196.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@nvddr

nvddr commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Review

Fixes a per-respawn context leak in render-mcp's LOVR child lifecycle (SceneDispatcher). Previously every successful start_lovr_once registered its ManagedProcess into the process-lifetime AsyncExitStack, so on each respawn the prior context's teardown (its two _forward pipe tasks + the open log-file sink) never ran until whole-process shutdown — leaking N-1 dead contexts across N restarts. The fix gives each launch its own AsyncExitStack (self._launch_stack), closes it inside _watch the moment the child exits (before _lovr_started is reset and a respawn is allowed), and registers a single app-lifetime _aclose_live_launch callback for the shutdown-while-LOVR-running case. Fits the ManagedProcess/stack model cleanly: __aexit__ skips terminate() once returncode is set and always cancels pipe tasks + closes the sink, so post-exit aclose() is safe; the stack swaps are identity-guarded and disp.close() cancels _watch before the app stack unwinds, so no concurrent/double aclose. Verified against the real xr_ai_launcher._processes.ManagedProcess at head SHA; the regression test asserts the no-accumulation property (sum(c.exited) == 1) and the shutdown-time close.

Severity Location Finding
(none) Clean; addresses the root cause of #196 and is covered by a test.

@wenxind-nvidia
wenxind-nvidia merged commit c38ce38 into main Jun 10, 2026
9 checks passed
@wenxind-nvidia
wenxind-nvidia deleted the fix_render_mcp_managedprocess_leak_issue196 branch June 10, 2026 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

render-mcp leaks ManagedProcess contexts on LOVR respawn (parked in app-lifetime AsyncExitStack)

2 participants