Skip to content

fix: prevent Windows stdio socket inheritance - #1282

Open
kpkhxlgy0 wants to merge 1 commit into
CoplayDev:betafrom
kpkhxlgy0:codex/windows-stdio-socket-inheritance
Open

fix: prevent Windows stdio socket inheritance#1282
kpkhxlgy0 wants to merge 1 commit into
CoplayDev:betafrom
kpkhxlgy0:codex/windows-stdio-socket-inheritance

Conversation

@kpkhxlgy0

@kpkhxlgy0 kpkhxlgy0 commented Jul 25, 2026

Copy link
Copy Markdown

Description

Prevent the Windows stdio bridge listener socket from being inherited by child processes launched from the Unity Editor. This avoids stale listeners remaining alive in processes such as VS Code after a domain reload or Editor shutdown, which previously forced the bridge to fall back from port 6400 to 6401.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • Test update

Changes Made

  • Clear HANDLE_FLAG_INHERIT on the Windows TcpListener socket before it starts listening.
  • Surface SetHandleInformation failures as SocketException and dispose unpublished listeners safely.
  • Add a Windows EditMode regression test that checks the production listener factory with GetHandleInformation.
  • Preserve the existing macOS exclusive-address and cross-platform listener behavior.

Compatibility / Package Source

  • Unity version(s) tested: Tuanjie 2022.3.62t11 on Windows 11 (Unity 2022.3-compatible)
  • Package source used (beta, main, tag, branch, or file): local file checkout from this branch in MyGame
  • Resolved commit hash from Packages/packages-lock.json (if using a Git package URL): not applicable (file source)

Testing/Screenshots/Recordings

  • Python tests (cd Server && uv run pytest tests/ -v): 1330 passed, 3 skipped
  • Unity EditMode tests: regression test added, but the upstream UnityMCPTests project was not opened during validation
  • Unity PlayMode tests
  • Package import/compile check: MyGame compiled and reconnected with 0 MCP errors/warnings
  • Not applicable (explain why in Additional Notes)

Runtime regression in MyGame:

  • Started VS Code from the Editor after port 6400 was listening.
  • Kept VS Code running across two forced script/domain reloads.
  • Verified each new listener handle had inheritance flags = 0.
  • Verified only the current Editor owned port 6400 after each reload; port 6401 was never used.

Documentation Updates

  • I have added/removed/modified tools or resources
  • If yes, I have updated all documentation files using:
    • The LLM prompt at tools/UPDATE_DOCS_PROMPT.md (recommended)
    • Manual review of the generated changes

No tool or resource surface changed.

Related Issues

None.

Additional Notes

The failure was caused by Windows socket-handle inheritance, not by the stdio server process itself. Clearing the inheritance bit immediately after socket creation prevents child processes from retaining the listener through Editor teardown.

Summary by CodeRabbit

  • Bug Fixes

    • Improved Windows TCP listener configuration to prevent unintended handle inheritance.
    • Enhanced listener cleanup when setup encounters an error, reducing the risk of lingering resources.
  • Tests

    • Added Windows-specific coverage to verify listener handles are created with inheritance disabled.

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e9642b58-663c-4df1-90d8-4a91dcb4e93f

📥 Commits

Reviewing files that changed from the base of the PR and between bd72241 and 096fcaf.

📒 Files selected for processing (3)
  • MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/Transport/StdioBridgeHostTests.cs
  • TestProjects/UnityMCPTests/Assets/Tests/EditMode/Transport/StdioBridgeHostTests.cs.meta

📝 Walkthrough

Walkthrough

CreateConfiguredListener now clears Windows socket handle inheritance, cleans up failed listener setup, and is accessible to an added Windows-only NUnit test.

Changes

Windows listener configuration

Layer / File(s) Summary
Listener configuration and cleanup
MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs
Adds Windows SetHandleInformation interop, clears the inheritance flag during listener creation, exposes the method internally, and disposes failed listeners.
Windows inheritance test
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Transport/StdioBridgeHostTests.cs, TestProjects/UnityMCPTests/Assets/Tests/EditMode/Transport/StdioBridgeHostTests.cs.meta
Adds Windows-only coverage for the cleared handle flag and Unity asset metadata.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: bug

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the main fix: preventing Windows stdio socket inheritance.
Description check ✅ Passed The description matches the template and fills the required sections with change details, compatibility, testing, and notes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant