fix(lsp): cleanup subprocess on failed initialization in StdioTransport.start() #11020

Merged
HAL9000 merged 6 commits from pr_fix/lsp-transport-subprocess-cleanup into master 2026-06-13 20:37:23 +00:00
3 changed files with 38 additions and 0 deletions
+1
View File
@@ -103,3 +103,4 @@ Below are some specific details of individual PR contributions.
* HAL 9000 has contributed the resource and skill management showcase alignment (#4213): updated the CLI tools showcase with consistent counts, explicit save instructions, metadata callouts, and README framing for platform walkthroughs; removed obsolete tdd_issue tags from coverage threshold Robot tests; hardened the Skip If No LLM Keys E2E helper with per-key regex validation and log suppression to prevent credential leakage.
* HAL 9000 has contributed the sandbox dirs cache invalidation fix (PR #11091 / issue #7527): introduced `SandboxDirsCache` to track filesystem paths of sandbox-created directories by plan_id, wired automatic invalidation into all cleanup/purge methods (`cleanup_all`, `cleanup_abandoned`, `clear_sandbox_dirs_cache`, `_cleanup_on_exit_handler`), and added BDD test coverage.
* HAL 9000 has contributed the CleanupService sandbox cache invalidation fix (PR #8257 / issue #7527): `_purge_sandboxes()` now invalidates the internal `_sandbox_dirs_cache` after deleting stale directories so that a subsequent `scan()` call on the same instance re-reads the filesystem instead of returning already-deleted paths as stale items.
* HAL 9000 has contributed the LSP subprocess cleanup fix (#10597): added a defensive ``_process = None`` reset before ``subprocess.Popen()`` in ``StdioTransport.start()`` to prevent orphaned child processes and file descriptor leaks when ``subprocess.Popen()`` fails during initialization.
+16
View File
2
@@ -172,3 +172,19 @@ Feature: LSP StdioTransport coverage
And ltcov the transport has a mock process with invalid JSON body
When ltcov I read a message with timeout 5.0
Then ltcov the read result should be None
# ── subprocess cleanup on failed initialization (issue #10597) ────────────
Scenario: ltcov _process is reset to None after FileNotFoundError in start
Given ltcov I create a StdioTransport for command "nonexistent_binary_xyz"
And ltcov Popen is mocked to raise FileNotFoundError
When ltcov I try to start the transport
Then ltcov the error should be an LspError with message "not found"
And ltcov _process must be None after start failure
Scenario: ltcov _process is reset to None after OSError in start
Given ltcov I create a StdioTransport for command "bad_command"
And ltcov Popen is mocked to raise OSError with "Permission denied"
When ltcov I try to start the transport
Then ltcov the error should be an LspError with message "Failed to start"
And ltcov _process must be None after start failure
@@ -474,3 +474,24 @@ def step_ltcov_value_error(context: Context, fragment: str) -> None:
assert fragment in str(context.ltcov_error), (
f"Expected '{fragment}' in '{context.ltcov_error}'"
)
# ── Subprocess cleanup on failed init ────────────────────────────────────────
@then("ltcov _process must be None after start failure")
def step_ltcov_process_none_after_failed_start(context: Context) -> None:
"""Verify that _process is reset to None when start() fails.
Regression test for issue #10597 — prevents orphaned subprocess handles
and file descriptor leaks when subprocess.Popen() raises during
initialization (after pipes may have been allocated but before a valid
Popen object is returned).
"""
assert context.ltcov_error is not None, (
"Expected start failure to have raised an exception"
)
assert context.ltcov_transport._process is None, (
f"_process should be None after failed start, but got "
f"{context.ltcov_transport._process}"
)