fix: avoid asyncio.get_event_loop() RuntimeError on Python >=3.12#7
Open
pollockjj wants to merge 29 commits into
Open
fix: avoid asyncio.get_event_loop() RuntimeError on Python >=3.12#7pollockjj wants to merge 29 commits into
pollockjj wants to merge 29 commits into
Conversation
…l absent When pyisolate is installed from PyPI (non-editable), _pyisolate_source_path() resolves to site-packages/ which has no pyproject.toml, causing pixi install to fail. Now checks for pyproject.toml existence and falls back to a version-pinned PyPI dependency matching the uv code path pattern.
Remove 3 non-production ][ DIAG: log statements from the RPC deserialization path that printed on every type-tagged object crossing the wire.
Add _internal/pixi_provisioner.py that downloads, caches, and integrity-verifies the pixi binary from prefix-dev GitHub releases. Supports Linux x86_64/aarch64, macOS x86_64/arm64, Windows x86_64. Version pinned to 0.67.0. 11 tests covering platform detection, checksum verification, cache hit, and full download path.
Replace shutil.which("pixi") checks in environment.py and
environment_conda.py with ensure_pixi() from pixi_provisioner.
The conda backend now auto-downloads, caches, and verifies the pixi
binary on first use. Users never need to install pixi manually.
…ilable _deserialize_json_tensor previously returned raw dicts when torch was absent. Now falls back to numpy arrays. Also registers TensorValue deserializer in the child bootstrap for sealed workers without torch.
Add TestPlatformCoverage class with parametrized tests for all 5 platform combinations, Windows binary name (pixi.exe), and URL construction verification against GitHub release naming convention.
Sealed workers never run adapter.register_serializers() because the adapter can't be rehydrated. This caused ndarray results to be wrapped as RemoteObjectHandle instead of crossing the wire as inline data. Register ndarray as a data_type serializer directly in SealedNodeExtension.__init__. The serializer converts ndarray to TensorValue dict format, which the host's existing TensorValue fast-path in rpc_transports.py reconstructs as torch.Tensor.
…worker ndarray fix
Add extra_index_urls field to ExtensionConfig and plumb through install_dependencies() as --extra-index-url args to uv pip install. Enables packages from non-standard PyPI indexes (e.g., fbxsdkpy from gitlab.inria.fr) to resolve during isolated venv provisioning.
There was a problem hiding this comment.
Pull request overview
This PR updates PyIsolate’s async initialization and tests to avoid Python 3.12+ failures caused by removed implicit main-thread event loop creation.
Changes:
- Adjusts
AsyncRPC.__init__event-loop acquisition to avoidasyncio.get_event_loop()raising when constructed outside a running loop. - Updates synchronous event-channel tests to use
asyncio.run(...)instead ofget_event_loop().run_until_complete(...).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pyisolate/_internal/rpc_protocol.py |
Changes how AsyncRPC determines/creates its default event loop during construction. |
tests/test_event_channel.py |
Replaces deprecated sync-loop driving with asyncio.run(...) in unit tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+171
to
+175
| try: | ||
| self.default_loop = asyncio.get_running_loop() | ||
| except RuntimeError: | ||
| self.default_loop = asyncio.new_event_loop() | ||
| asyncio.set_event_loop(self.default_loop) |
Comment on lines
+78
to
+79
| asyncio.run(bridge.dispatch("progress", {"value": 1})) | ||
| asyncio.run(bridge.dispatch("preview", {"image": "data"})) |
pollockjj
force-pushed
the
fix/event-loop-modern-python
branch
from
db6fb9b to
85ad675
Compare
Codex Review -- Round 1 -- KICKOFFRunner: Prompt sent to codex reviewAwaiting codex output. The result will be posted as a separate comment on this PR when codex exits. |
Codex Review -- Round 1 -- RESULTPR: #7 Prompt sent to codex reviewVerbatim codex review outputThis PR was reviewed using The patch fixes construction with no current event loop, but it introduces a regression for synchronous callers that pre-set a non-running event loop before constructing AsyncRPC. Review comment:
Codex runner stderr (non-evidence) |
Comment on lines
+171
to
+175
| try: | ||
| self.default_loop = asyncio.get_running_loop() | ||
| except RuntimeError: | ||
| self.default_loop = asyncio.new_event_loop() | ||
| asyncio.set_event_loop(self.default_loop) |
pollockjj
force-pushed
the
fix/event-loop-modern-python
branch
from
85ad675 to
b495d9b
Compare
Codex Review -- Round 2 -- KICKOFFRunner: Prompt sent to codex reviewAwaiting codex output. The result will be posted as a separate comment on this PR when codex exits. |
Codex Review -- Round 2 -- RESULTPR: #7 Prompt sent to codex reviewVerbatim codex review outputThis PR was reviewed using No merge-blocking issues or actionable P3 findings were identified in the latest PR state. Codex runner stderr (non-evidence)github_issues/{ISSUE_NUMBER}/pr_review_outputs/round{ROUND}_copilot_review.md gh api -X POST /repos/{OWNER/REPO}/pulls/{PR_NUMBER}/comments cycle = 0 |
Comment on lines
+165
to
+171
| # Acquire the loop without raising when constructed outside a running loop. | ||
| # Python >=3.10 deprecated and >=3.12 removed implicit main-thread event loop | ||
| # creation, so an eager asyncio.get_event_loop() raised here in sync construction | ||
| # paths. Preserve the historical get_event_loop() semantics: prefer the running | ||
| # loop, then the thread's installed loop (set via asyncio.set_event_loop, e.g. a | ||
| # synchronous caller that constructs the RPC before running its own loop), and only | ||
| # create+install a new loop as a last resort. update_event_loop() may replace it. |
Owner
Author
There was a problem hiding this comment.
Fixed — updated the PR description to state the accurate scope (three files: 1 production + 2 tests). Thanks.
AsyncRPC.__init__ eagerly called asyncio.get_event_loop(), which raises
RuntimeError("There is no current event loop") on Python >=3.12 when the RPC is
constructed outside a running loop. The host launches extensions from a synchronous
path (host._launch_with_uds -> AsyncRPC(transport=...)), so this is a real production
crash risk on 3.12, not only a test artifact; it broke every non-torch CI job on
develop ("There is no current event loop in thread 'MainThread'").
AsyncRPC.__init__ now resolves the loop preserving get_event_loop() semantics without
the >=3.12 RuntimeError: prefer the running loop, then the thread's installed loop (set
via asyncio.set_event_loop before it is running), and only create+install a new loop as
a last resort. Behavior is unchanged when a loop is running or installed; only the
truly-no-loop construction case changes (was: crash on 3.12).
- tests/test_rpc_contract.py: regression tests for construct-without-loop and
reuse-installed-loop.
- tests/test_event_channel.py: asyncio.get_event_loop().run_until_complete(...) -> asyncio.run(...).
pollockjj
force-pushed
the
fix/event-loop-modern-python
branch
from
b495d9b to
e572f2c
Compare
Codex Review -- Round 3 -- KICKOFFRunner: Prompt sent to codex reviewAwaiting codex output. The result will be posted as a separate comment on this PR when codex exits. |
Codex Review -- Round 3 -- RESULTPR: #7 Prompt sent to codex reviewVerbatim codex review outputThis PR was reviewed using No merge-blocking defects or actionable P3 issues were identified in the latest PR state. The event-loop acquisition now preserves running and pre-set loop behavior while covering the no-current-loop construction case. Codex runner stderr (non-evidence) |
pollockjj
force-pushed
the
develop
branch
2 times, most recently
from
0e15c5f to
2fda457
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fixes the pre-existing
developCI breakage: every non-torch test job failed withRuntimeError: There is no current event loop in thread 'MainThread'.Root cause
AsyncRPC.__init__eagerly calledasyncio.get_event_loop(). On Python >=3.12that raises when the RPC is constructed outside a running loop. The host launches
extensions from a synchronous path (
host._launch_with_uds -> AsyncRPC(transport=...)),so this is a real production crash risk on 3.12 — not only a test artifact.
tests/test_event_channel.pyused the same deprecatedasyncio.get_event_loop().run_until_complete(...)pattern.Fix
AsyncRPC.__init__: resolve the loop preservingget_event_loop()semantics withoutthe >=3.12
RuntimeError— prefer the running loop, then the thread's installedloop (
asyncio.set_event_loopbefore it is running), and only create+install a newloop as a last resort. Behavior is unchanged when a loop is running or installed; only
the truly-no-loop construction case changes (was: crash on 3.12).
tests/test_event_channel.py:asyncio.get_event_loop().run_until_complete(...)→asyncio.run(...).Verification
tests/test_rpc_contract.py: construct-without-loop andreuse-installed-loop (both fail on
develop, pass with the fix).ruff+mypyclean on changed files.Scope
Three files: 1 production (
pyisolate/_internal/rpc_protocol.py) + 2 tests(
tests/test_rpc_contract.py,tests/test_event_channel.py). No public API change;no behavior change when a running or installed loop is present.