-
Notifications
You must be signed in to change notification settings - Fork 741
fix: use SelectorEventLoop for asyncio on Windows to avoid the unity bridge closed #665
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
437dbf2
fix: use WindowsSelectorEventLoopPolicy for asyncio on Windows to res…
whatevertogo a1f4db1
test: add Windows event loop policy verification
whatevertogo cbad6f7
fix: use getattr() for cross-platform asyncio policy references
whatevertogo a25aff4
refactor: use get_running_loop() in async context (code review)
whatevertogo 146a352
Merge branch 'CoplayDev:beta' into beta
whatevertogo 7caf958
fix: improve Windows asyncio compatibility for WebSocket connections …
whatevertogo 60d0d60
fix: ensure non-Windows platforms retain their default event loop policy
whatevertogo 1d505ad
fix: comment out unused import and lambda for asyncio event loop in W…
whatevertogo caf6906
fix: update comment for Python 3.16+ SelectorEventLoop handling in Wi…
whatevertogo c688c37
fix: update comment for Python 3.16+ SelectorEventLoop handling in Wi…
whatevertogo 993043f
refactor: use anyio loop_factory for Windows asyncio fix
whatevertogo 221e343
Merge branch 'CoplayDev:beta' into beta
whatevertogo 624a85f
chore: update Unity package to beta version 9.4.5-beta.2
actions-user fb3c0ac
Merge pull request #8 from whatevertogo/beta-version-9.4.5-beta.2-219…
github-actions[bot] 616789d
chore: add auto-sync workflow for upstream beta branch
whatevertogo a16beac
Revert "chore: add auto-sync workflow for upstream beta branch"
whatevertogo c217f4c
Merge pull request #9 from whatevertogo/revert-sync-workflow
whatevertogo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,161 @@ | ||
| """ | ||
| Test event loop configuration for Windows. | ||
|
|
||
| This module verifies that the correct asyncio loop factory is selected | ||
| on different platforms to prevent WinError 64 on Windows. | ||
|
|
||
| WinError 64 occurs when using ProactorEventLoop with concurrent | ||
| WebSocket and HTTP connections. The fix is to use SelectorEventLoop | ||
| on Windows. | ||
|
|
||
| Related fix: Server/src/main.py | ||
| """ | ||
|
|
||
| import sys | ||
| import asyncio | ||
| from functools import partial | ||
| import pytest | ||
|
|
||
|
|
||
| @pytest.mark.skipif(sys.platform != "win32", reason="Windows-specific") | ||
| def test_windows_uses_selector_event_loop_factory(): | ||
| """ | ||
| Verify that Windows uses SelectorEventLoop via loop_factory. | ||
|
|
||
| This prevents WinError 64 when handling concurrent WebSocket and HTTP connections. | ||
|
|
||
| Regression test for Windows asyncio bug where ProactorEventLoop's IOCP | ||
| has race conditions with rapid connection changes. | ||
|
|
||
| The fix is applied in Server/src/main.py | ||
| """ | ||
| import importlib | ||
| import main # type: ignore[import] - conftest.py adds src to sys.path | ||
| importlib.reload(main) | ||
|
|
||
| loop_factory = main._get_asyncio_loop_factory() | ||
| assert loop_factory is asyncio.SelectorEventLoop, ( | ||
| "Expected SelectorEventLoop for Windows loop_factory" | ||
| ) | ||
|
|
||
|
|
||
| @pytest.mark.skipif(sys.platform == "win32", reason="Non-Windows only") | ||
| def test_non_windows_uses_default_loop_factory(): | ||
| """ | ||
| Verify that non-Windows platforms keep their default event loop behavior. | ||
|
|
||
| SelectorEventLoop should only be used on Windows to avoid the IOCP bug. | ||
| Other platforms should use their optimal default event loop. | ||
| """ | ||
| import importlib | ||
| import main # type: ignore[import] - conftest.py adds src to sys.path | ||
| importlib.reload(main) | ||
|
|
||
| loop_factory = main._get_asyncio_loop_factory() | ||
| assert loop_factory is None, "Non-Windows platforms should not set a loop_factory" | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_async_operations_use_correct_event_loop(): | ||
| """ | ||
| Smoke test to verify async operations work with the configured event loop. | ||
|
|
||
| This test creates a simple async operation to ensure the event loop | ||
| is functional. It doesn't test WinError 64 directly (which is a | ||
| timing-dependent race condition), but confirms the basic async | ||
| infrastructure works with the policy configured in main.py. | ||
| """ | ||
| # Import main to ensure the event loop policy is configured | ||
| import importlib | ||
| import main # type: ignore[import] - conftest.py adds src to sys.path | ||
| importlib.reload(main) | ||
|
|
||
| # Simple async operation | ||
| async def simple_task(): | ||
| await asyncio.sleep(0.01) | ||
| return "success" | ||
|
|
||
| # Should complete without errors | ||
| result = await simple_task() | ||
| assert result == "success" | ||
|
|
||
| # Verify we're using the expected event loop | ||
| # Use get_running_loop() as we're in an async context | ||
| loop = asyncio.get_running_loop() | ||
| assert loop is not None, "Event loop should be running" | ||
| assert loop.is_running(), "Event loop should be in running state" | ||
|
|
||
|
|
||
| def test_run_mcp_uses_fastmcp_run_when_no_loop_factory(monkeypatch: pytest.MonkeyPatch): | ||
| """When no loop factory is needed, _run_mcp should delegate to FastMCP.run.""" | ||
| import importlib | ||
| import main # type: ignore[import] - conftest.py adds src to sys.path | ||
| importlib.reload(main) | ||
|
|
||
| class DummyMCP: | ||
| def __init__(self) -> None: | ||
| self.run_called = False | ||
| self.run_kwargs = {} | ||
|
|
||
| def run(self, **kwargs): | ||
| self.run_called = True | ||
| self.run_kwargs = kwargs | ||
|
|
||
| monkeypatch.setattr(main, "_get_asyncio_loop_factory", lambda: None) | ||
|
|
||
| mcp = DummyMCP() | ||
| main._run_mcp(mcp, transport="stdio") | ||
|
|
||
| assert mcp.run_called | ||
| assert mcp.run_kwargs["transport"] == "stdio" | ||
|
|
||
|
|
||
| def test_run_mcp_uses_anyio_with_loop_factory(monkeypatch: pytest.MonkeyPatch): | ||
| """When loop factory exists, _run_mcp should use anyio.run with backend options.""" | ||
| import importlib | ||
| import main # type: ignore[import] - conftest.py adds src to sys.path | ||
| importlib.reload(main) | ||
|
|
||
| class DummyMCP: | ||
| async def run_async(self, transport, show_banner=True, **kwargs): | ||
| return None | ||
|
|
||
| captured = {} | ||
|
|
||
| def fake_anyio_run(func, *args, **kwargs): | ||
| captured["func"] = func | ||
| captured["args"] = args | ||
| captured["kwargs"] = kwargs | ||
| return None | ||
|
|
||
| monkeypatch.setattr(main, "anyio", type("AnyIOStub", (), {"run": staticmethod(fake_anyio_run)})) | ||
| monkeypatch.setattr(main, "_get_asyncio_loop_factory", lambda: asyncio.SelectorEventLoop) | ||
|
|
||
| mcp = DummyMCP() | ||
| main._run_mcp(mcp, transport="http", host="localhost", port=8080) | ||
|
|
||
| assert isinstance(captured["func"], partial) | ||
| assert captured["func"].args[0] == "http" | ||
| assert captured["func"].keywords["show_banner"] is True | ||
| assert captured["func"].keywords["host"] == "localhost" | ||
| assert captured["func"].keywords["port"] == 8080 | ||
| assert captured["kwargs"]["backend_options"]["loop_factory"] is asyncio.SelectorEventLoop | ||
|
|
||
|
|
||
| @pytest.mark.skipif(sys.platform != "win32", reason="Windows-specific") | ||
| def test_windows_loop_factory_prevents_proactor(): | ||
| """ | ||
| Verify that Windows loop_factory explicitly avoids ProactorEventLoop. | ||
|
|
||
| ProactorEventLoop uses IOCP which has known issues with WinError 64 | ||
| when handling rapid connection changes. This test confirms we're | ||
| not using ProactorEventLoop. | ||
| """ | ||
| import importlib | ||
| import main # type: ignore[import] - conftest.py adds src to sys.path | ||
| importlib.reload(main) | ||
|
|
||
| loop_factory = main._get_asyncio_loop_factory() | ||
| assert loop_factory is asyncio.SelectorEventLoop, ( | ||
| "SelectorEventLoop should be used on Windows (prevents WinError 64)" | ||
| ) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
anyio.run backend_options loop_factory asyncio💡 Result:
anyio.run()accepts backend-specific kwargs viabackend_options. For the asyncio backend, you can pass an event loop factory withloop_factory(a callable that returns a new event loop). (deepwiki.com)Example: use
uvloop(AnyIO 4+)AnyIO’s migration guide explicitly says to replace the old “event loop policy” approach with
loop_factory. (anyio.readthedocs.io)Notes
debug,loop_factory, anduse_uvloop(which internally selectsuvloop.new_event_loopwhen available). (deepwiki.com)Citations:
🏁 Script executed:
Repository: CoplayDev/unity-mcp
Length of output: 1177
Remove trailing whitespace on lines 68–69.
The
anyio.runapproach withbackend_options={"loop_factory": loop_factory}is correct and follows AnyIO's recommended migration away from deprecated event loop policies.The
show_banner=Trueparameter is only passed in theanyio.runpath (line 63), not in themcp.run()path (line 56). This asymmetry is not problematic sincemcp.run()has its own defaults, but note the difference.Lines 68–69 contain trailing whitespace that should be removed.
🤖 Prompt for AI Agents