Restore Windows support (optional UDS + SIGUSR1)#475
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to make Tractor’s POSIX-specific IPC pieces (UDS transport selection and SIGUSR1 stackscope dumps) optional so the codebase can import and run on Windows without failing.
Changes:
- Made the UDS backend import conditional (based on platform /
AF_UNIXavailability) and adjusted transport registries accordingly. - Updated discovery address registries/defaults to omit UDS when unavailable.
- Made
SIGUSR1platform-dependent to avoid importing unavailable signals on Windows.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
tractor/ipc/_types.py |
Conditionalizes UDS backend availability and registers transports accordingly. |
tractor/ipc/_server.py |
Makes UDS import optional in the IPC server module. |
tractor/discovery/_addr.py |
Conditionalizes UDS address type registration/defaults for discovery. |
tractor/devx/_stackscope.py |
Makes SIGUSR1 optional/conditional for stackscope signal-based dumps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except Exception as e: | ||
| log.warning("UDS backend import failed: %s", e) | ||
| else: | ||
| log.warning("UDS backend disabled on this platform.") |
There was a problem hiding this comment.
🤖 response authored by
claude-code
Good catch — these import-time log.warnings are gone. UDS availability is now a single HAS_UDS = trio.has_unix flag and the transport/address registries derive from it, so there's no per-import warning noise on Windows/no-AF_UNIX hosts.
📎 already addressed in
83d45435
| if UDSAddress is not None: | ||
| _address_types['uds'] = UDSAddress | ||
| else: | ||
| log.warning("Skipping UDS address type: no UDS backend available.") |
There was a problem hiding this comment.
🤖 response authored by
claude-code
Good catch — these import-time log.warnings are gone. UDS availability is now a single HAS_UDS = trio.has_unix flag and the transport/address registries derive from it, so no per-import warning noise on Windows/no-AF_UNIX hosts.
📎 already addressed in
83d45435
| if UDSAddress is not None: | ||
| _default_lo_addrs['uds'] = UDSAddress.get_root().unwrap() | ||
| else: | ||
| log.warning("Skipping UDS default loopback address: no UDS backend available.") |
There was a problem hiding this comment.
🤖 response authored by
claude-code
Good catch — these import-time log.warnings are gone. UDS availability is now a single HAS_UDS = trio.has_unix flag and the transport/address registries derive from it, so no per-import warning noise on Windows/no-AF_UNIX hosts.
📎 already addressed in
83d45435
| from ._tcp import TCPAddress | ||
|
|
||
| log = log.get_logger() | ||
|
|
||
| UDSAddress = None | ||
|
|
||
| if getattr(socket, "AF_UNIX", None) is not None and platform.system() != "Windows": | ||
| from ._uds import UDSAddress | ||
| else: | ||
| pass |
There was a problem hiding this comment.
🤖 response authored by
claude-code
Fixed at the root: tractor.ipc._uds is now import-safe (only its AF_UNIX import is guarded), so UDSAddress stays a real class on every platform — never None. wrap_address() and the UDSAddress|TCPAddress annotations are fine; actual UDS use is gated on HAS_UDS / trio.has_unix.
📎 already addressed in
83d45435
| if platform.system() != "Windows": | ||
| from signal import SIGUSR1 | ||
| else: | ||
| SIGUSR1 = None |
There was a problem hiding this comment.
🤖 response authored by
claude-code
Addressed — enable_stack_on_sig() now early-returns (no-op + warning) when sig is None, so a missing SIGUSR1 on Windows degrades gracefully instead of hitting getsignal(None) / signal(None, ...).
📎 already addressed in
83d45435
|
|
||
| # if TYPE_CHECKING: | ||
| # from tractor._addr import Address | ||
| log = logging.getLogger(__name__) |
There was a problem hiding this comment.
🤖 response authored by
claude-code
Addressed — the module no longer uses the stdlib logging; the whole gating block (and its logger) was dropped when the registries became data-driven, so no stray logging.getLogger() remains.
📎 already addressed in
83d45435
| except Exception as e: | ||
| log.warning("UDS backend unavailable (%s); continuing without it.", e) |
There was a problem hiding this comment.
🤖 response authored by
claude-code
Addressed — there is no longer a try/except Exception around the UDS import. Since _uds is import-safe, _types imports UDSAddress / MsgpackUDSStream / HAS_UDS unconditionally, so a real _uds bug now surfaces loudly instead of being swallowed into a silent TCP-only fallback.
📎 already addressed in
83d45435
| HAS_AF_UNIX = getattr(socket, "AF_UNIX", None) is not None | ||
| IS_WINDOWS = platform.system() == "Windows" | ||
|
|
||
| UDSAddress = None # so references exist but do nothing on Windows |
There was a problem hiding this comment.
🤖 response authored by
claude-code
Fixed at the root: tractor.ipc._uds is now import-safe (only its AF_UNIX import is guarded), so UDSAddress stays a real class on every platform — never None. wrap_address() and the UDSAddress|TCPAddress annotations are fine; actual UDS use is gated on HAS_UDS / trio.has_unix.
📎 already addressed in
83d45435
goodboy
force-pushed
the
windows_support_round2
branch
2 times, most recently
from
2743b81 to
5bf6417
Compare
goodboy
added
windows_related
IPC and transport
non-linux
| UDSAddress = None | ||
|
|
||
| if getattr(socket, "AF_UNIX", None) is not None and platform.system() != "Windows": | ||
| from ._uds import UDSAddress | ||
| else: |
There was a problem hiding this comment.
🤖 response authored by
claude-code
Fixed at the root: tractor.ipc._uds is now import-safe (only its AF_UNIX import is guarded), so UDSAddress stays a real class on every platform — never None. wrap_address() and the UDSAddress|TCPAddress annotations are fine; actual UDS use is gated on HAS_UDS / trio.has_unix.
📎 already addressed in
83d45435
| UDSAddress = None # so references exist but do nothing on Windows | ||
|
|
||
| if HAS_AF_UNIX and not IS_WINDOWS: | ||
| try: | ||
| from ..ipc._uds import UDSAddress as _UDSAddress |
There was a problem hiding this comment.
🤖 response authored by
claude-code
Fixed at the root: tractor.ipc._uds is now import-safe (only its AF_UNIX import is guarded), so UDSAddress stays a real class on every platform — never None. wrap_address() and the UDSAddress|TCPAddress annotations are fine; actual UDS use is gated on HAS_UDS / trio.has_unix.
📎 already addressed in
83d45435
| if platform.system() != "Windows": | ||
| from signal import SIGUSR1 | ||
| else: | ||
| SIGUSR1 = None |
There was a problem hiding this comment.
🤖 response authored by
claude-code
Addressed — enable_stack_on_sig() now early-returns (no-op + warning) when sig is None, so a missing SIGUSR1 on Windows degrades gracefully instead of hitting getsignal(None) / signal(None, ...).
📎 already addressed in
83d45435
| except Exception as e: | ||
| log.warning("UDS backend unavailable (%s); continuing without it.", e) |
There was a problem hiding this comment.
🤖 response authored by
claude-code
Addressed — there is no longer a try/except Exception around the UDS import. Since _uds is import-safe, _types imports UDSAddress / MsgpackUDSStream / HAS_UDS unconditionally, so a real _uds bug now surfaces loudly instead of being swallowed into a silent TCP-only fallback.
📎 already addressed in
83d45435
Self-review of #475 (xhigh, source-traced)Ran a deep multi-angle review. Headline: as it stood, Every eager
🔴 Critical —
|
The prior round gated UDS in four modules but `import tractor` still crashed on Windows: `tractor.ipc._uds` does `from socket import AF_UNIX` at module top, and several modules in the import graph (`discovery._api`, `spawn._reap`, `discovery._multiaddr`, `_testing.addr`) import `_uds` unconditionally. Instead of guarding every importer, fix the root and collapse the per-module probes to one capability flag. - in `ipc/_uds.py`, guard the lone `AF_UNIX` import so the module stays importable everywhere; expose `HAS_UDS = trio.has_unix` as the single source of truth (the same predicate that gates `trio.open_unix_socket()`). - `ipc/_types.py`, `discovery/_addr.py` and `ipc/_server.py` now import `UDSAddress`/`MsgpackUDSStream`/`HAS_UDS` directly and gate the transport + address registries on `HAS_UDS`; drop the duplicated `getattr(socket,'AF_UNIX')` / `platform.system()` probes, the dead `HAS_AF_UNIX` conjunct, and the import-time `log.warning()` spam. - `devx/_stackscope.py` `enable_stack_on_sig()` early-returns when `sig is None`, so a missing `SIGUSR1` (Windows) degrades to a no-op instead of a `TypeError` from `getsignal()` / `signal()`. - add a `windows-latest` CI leg (UDS excluded; informational via `continue-on-error` while support matures) plus an `import tractor` smoke step as the hard signal for the import fix. Because `_uds` is importable everywhere `UDSAddress` stays a real class, so `isinstance()` checks and `wrap_address()` no longer `AttributeError` on no-UDS hosts; actual socket use stays gated on `has_unix`. Review: #475 (this patch was generated in some part by [`claude-code`][claude-code-gh]) [claude-code-gh]: https://github.com/anthropics/claude-code
Windows (and any CPython that doesn't expose `socket.AF_UNIX`) can't import the UDS transport backend nor `signal.SIGUSR1`, so the unconditional imports break `import tractor` outright on those hosts. Guard the platform-specific bits behind capability probes and fall back to a TCP-only runtime when the UDS backend is unavailable. - across `discovery/_addr.py`, `ipc/_server.py` and `ipc/_types.py`, gate on `getattr(socket, 'AF_UNIX', None)` + `platform.system()` and import `UDSAddress` / `MsgpackUDSStream` only when supported, leaving the names as `None` otherwise. - register the `'uds'` key in `_address_types`, its default-loopback addr, and the transport lookup maps only when the backend actually loads, so TCP keeps working standalone. - in `devx/_stackscope.py`, import `SIGUSR1` conditionally and set it to `None` on Windows. Rebased onto the post-reorg tree where `_addr.py` now lives under `tractor/discovery/`; adapt the relocated imports to the package's `..ipc._uds` / `..ipc._tcp` paths (the original single-dot paths would silently disable UDS on POSIX) and drop a duplicated `TYPE_CHECKING` block and dead `import logging` left by the move. (this patch was generated in some part by [`claude-code`][claude-code-gh]) [claude-code-gh]: https://github.com/anthropics/claude-code
The prior round gated UDS in four modules but `import tractor` still crashed on Windows: `tractor.ipc._uds` does `from socket import AF_UNIX` at module top, and several modules in the import graph (`discovery._api`, `spawn._reap`, `discovery._multiaddr`, `_testing.addr`) import `_uds` unconditionally. Instead of guarding every importer, fix the root and collapse the per-module probes to one capability flag. - in `ipc/_uds.py`, guard the lone `AF_UNIX` import so the module stays importable everywhere; expose `HAS_UDS = trio.has_unix` as the single source of truth (the same predicate that gates `trio.open_unix_socket()`). - `ipc/_types.py`, `discovery/_addr.py` and `ipc/_server.py` now import `UDSAddress`/`MsgpackUDSStream`/`HAS_UDS` directly and gate the transport + address registries on `HAS_UDS`; drop the duplicated `getattr(socket,'AF_UNIX')` / `platform.system()` probes, the dead `HAS_AF_UNIX` conjunct, and the import-time `log.warning()` spam. - `devx/_stackscope.py` `enable_stack_on_sig()` early-returns when `sig is None`, so a missing `SIGUSR1` (Windows) degrades to a no-op instead of a `TypeError` from `getsignal()` / `signal()`. - add a `windows-latest` CI leg (UDS excluded; informational via `continue-on-error` while support matures) plus an `import tractor` smoke step as the hard signal for the import fix. Because `_uds` is importable everywhere `UDSAddress` stays a real class, so `isinstance()` checks and `wrap_address()` no longer `AttributeError` on no-UDS hosts; actual socket use stays gated on `has_unix`. Review: #475 (this patch was generated in some part by [`claude-code`][claude-code-gh]) [claude-code-gh]: https://github.com/anthropics/claude-code
`tests/test_ringbuf.py` imports `tractor.ipc._ringbuf` at module top, which pulls in `tractor.ipc._linux` whose module-level `ffi.dlopen(None)` raises `OSError` on Windows (and any non-linux host). That fires at COLLECTION, before the module's existing `pytestmark = pytest.mark.skip` can apply, so it aborts the whole pytest session — the new `windows-latest` CI leg never gets past collection. - guard the module with `pytest.skip(allow_module_level=True)` gated on `platform.system() != 'Linux'`, placed before the crashing import — same idiom as `tests/devx/test_debugger.py`. - the `eventfd`-based ringbuf backend is linux-only by design, so macOS skips cleanly too (previously it only skipped incidentally via the absent `cffi` optional dep). (this patch was generated in some part by [`claude-code`][claude-code-gh]) [claude-code-gh]: https://github.com/anthropics/claude-code
goodboy
force-pushed
the
windows_support_round2
branch
from
f22bfdd to
08f7a3a
Compare
goodboy
changed the title
SIGUSR1 optional for WindowsSIGUSR1)
`test_lifetime_stack_wipes_tmpfile` guards spawn+teardown with a hard-coded `trio.move_on_after()` (1.6s / 1s) that isn't scaled for slow CI. On a noisy macOS runner the `error_in_child=True` case times out before the child error propagates, so the scope cancels and `assert not cs.cancel_called` flips — reddening the (required) macOS leg. Same unscaled-deadline class `main` already fixed for `test_dynamic_pub_sub`. - multiply the budget by `cpu_perf_headroom()` (`tests/conftest`), the established deadline-headroom helper (3x on macOS CI, a 1.0 no-op locally / on un-throttled linux). (this patch was generated in some part by [`claude-code`][claude-code-gh]) [claude-code-gh]: https://github.com/anthropics/claude-code
`tractor`'s infect-asyncio mode runs an `asyncio` loop under `trio` guest-mode; on Windows the default `ProactorEventLoop` is incompatible and the suite hangs/crashes mid-run (orphaned py procs), so the `windows-latest` CI leg never finishes reporting. - add a module-level `pytest.skip(allow_module_level=True)` gated on `platform.system() == 'Windows'` to `test_infected_asyncio` and `test_root_infect_asyncio`, before their asyncio-interop imports. macOS/linux are unaffected (they run these fine). (this patch was generated in some part by [`claude-code`][claude-code-gh]) [claude-code-gh]: https://github.com/anthropics/claude-code
The five transport/address lookup maps each hand-guarded `uds` with its own `if HAS_UDS:` block (3 in `ipc/_types`, 2 in `discovery/_addr`) — easy to let drift so a backend half-registers (known by address but not by key, listed but unroutable, &c). - build one `_msg_transports` / `_address_protos` list per module (TCP always, UDS only when `HAS_UDS`), then DERIVE every map from it via each backend's ClassVars (`codec_key`, `address_type`, `proto_key`). Adding a backend now touches one list, and the maps can't disagree. (this patch was generated in some part by [`claude-code`][claude-code-gh]) [claude-code-gh]: https://github.com/anthropics/claude-code
`test_parent_writer_child_reader` deadlocks on Windows (the parent/child shared-mem transfer hangs at the larger frame size), so the `windows-latest` CI leg ran to the 16-min job cap instead of completing. It's a genuine nascent-Windows shm bug, not a clean "unsupported", so it's `skipif`'d (not removed) and tracked under #404; `test_child_attaches_alot` still runs on Windows. - `@pytest.mark.skipif(platform.system() == 'Windows', ...)` on the parametrized `test_parent_writer_child_reader` so the leg completes + reports. linux/macOS unaffected (all 6 variants still run). (this patch was generated in some part by [`claude-code`][claude-code-gh]) [claude-code-gh]: https://github.com/anthropics/claude-code
Restore Windows support (optional UDS +
SIGUSR1)Replacement for #408, towards #404; resolves #405.
Motivation
import tractorcurrently raises on Windows (and any CPython builtwithout
socket.AF_UNIX): the UDS transport backend(
tractor.ipc._uds) importsAF_UNIXat module top, andtractor.devx._stackscopeimportssignal.SIGUSR1— neither existson Windows. Because several core modules pull
_udsinto theimport tractorgraph, the package won't even import there, which blocks alldownstream Windows use (see #405).
This makes both the UDS backend and the
SIGUSR1trace-dump hookoptional: where they're unavailable the runtime degrades to a
TCP-only, no-
SIGUSR1configuration instead of crashing. Built onthe contributor
fix/windowswork, rebased onto the post-reorg treeand hardened so
import tractoractually succeeds on Windows.Summary of changes
ipc/_uds.pyguards its loneAF_UNIXimport so the module stays importable everywhere, whileuse of the backend stays gated on
trio'shas_unix.HAS_UDScapability flag (= trio.has_unix) asthe one source of truth;
ipc/_types,discovery/_addrandipc/_serverimport it and register theudstransport / address/ loopback entries only when it's
True.signal.SIGUSR1optional indevx/_stackscope:enable_stack_on_sig()no-ops gracefully when noSIGUSR1existsinstead of raising deep in
signal()setup.windows-latestCI leg (UDS excluded as POSIX-only;informational via
continue-on-errorwhile support matures) plusan
import tractorsmoke step as the hard signal.Testing on Windows (step-by-step)
CI can't fully prove Windows support yet, so hands-on community
testing is very welcome. These steps assume no prior experience
with our packaging tool (
uv) orpytest. Run them inPowerShell.
Install Python 3.13 from https://www.python.org/downloads/ (tick
"Add python.exe to PATH" in the installer).
Install
uv, our project/venv manager:Grab the PR branch (install the GitHub CLI
ghfirst if needed:https://cli.github.com/):
Create the virtualenv and install everything (reads
pyproject.tomland builds an isolated.venv):Smoke test — this is the core fix. It MUST print
okand must NOTraise
ImportError:Run the test suite over TCP (UDS is POSIX-only, so it's skipped on
Windows).
uv runexecutes inside the venv from step 4:What to paste back into this PR:
pytestsummary line (e.g.N passed, M failed),Don't worry if some tests fail — Windows support is new and any
failure list is useful signal for follow-ups.
Future follow up
The clean transport-unavailable error and the data-driven transport
registries are done in this PR. The remaining Windows-suite work —
getting the
windows-latestleg fully green (it currently runscontinue-on-errorand reports 44 failures), then promoting it to arequired check — is tracked in #476.
Links
(this pr content was generated in some part by
claude-code)