MagicBeanDev/Longshot

Watch

Fork 99

fix(control): wait for busy Windows pipes using the native error code #494

Open

monsterlag wants to merge 3 commits from monsterlag/Longshot:fix/windows-bridge-pipe-busy into main

monsterlag commented

2026-11-14 16:40:59 +00:00

First-time contributor

Why

Back-to-back control clients on Windows can report "no game listens (Invalid argument)" while the bridge server is running. The CRT converts a busy named-pipe open into errno22 without retaining Win32 error231, so the client's intended busy retry never happens.

What changed

scripts/longshot_ctl.py: use typed CreateFileW for Windows pipe opens and preserve the native error. Retry only ERROR_PIPE_BUSY within the existing deadline; missing and denied pipes fail immediately. Transfer the handle to the CRT file object and close the appropriate owner on conversion failure, preserving the original exception.
tests/test_windows_pipe_busy.py: five native test methods cover busy timeout/recovery, missing pipes, simulated access denial, handle-conversion failure and file-wrapper failure before/after descriptor ownership changes.
Four docs/tested/ files record the held-pipe regression comparisons; one more records the actual-game title interoperability check. POSIX transport and the JSON protocol are unchanged.

Flags and settings

None. Connection timeout and CLI arguments are unchanged.

Measurements

main: 9c9d69ed09 (2026-11-14)
this PR: fix/windows-bridge-pipe-busy @ 70c9af2731
Hardware: Ryzen7 9800X3D, 96GiB RAM, Windows11 build26300, native Python3.14.7. Named-pipe fixtures use no GPU. The separate live check used RTX5090, driver617.42.
Scene: one held real Windows pipe, then rearming it; separate live check at the title menu in an independent headless game profile.
Method: focused checks as warm-up, then two sequential interleaved main/PR/main/PR runs of the same deterministic held-pipe timeout/recovery method against the actual production client modules. Report median failures; this is behavior evidence, not a speed claim. A separate raw probe compared CRT open with native CreateFileW on the same busy pipe.
How to reproduce: run python -I -B tests/test_windows_pipe_busy.py on Windows. For main, run the same test_busy_timeout_and_recovery fixture with main's client module. The fixture occupies the pipe, checks a bounded busy timeout, then rearms its server and verifies a connection succeeds.

Check	main @ `9c9d69ed`	this PR @ `70c9af27`
Held-pipe method failures, run1 / run2	1 / 1	0 / 0
Median failures	1	0
Errors / skipped methods, each run	0 / 0	0 / 0

raw busy-pipe probe: CRT errno22, winerror absent; CreateFileW error231
raw missing-pipe probe: CRT errno2, winerror absent; CreateFileW error2
main, each held-pipe run: methods1, failures1, errors0, skips0
this PR, each held-pipe run: methods1, failures0, errors0, skips0
new native suite: five methods pass, including both file-wrapper ownership subcases

The measured source commit wasd0340ee9; later commits add only the five run records, with production/test files unchanged. Four consecutive fixed-client connections to the actual matched API22 game each returned a protocol1 status reply at the title menu (frames1339/1354/1367/1382, state=menu). Game and guard exited0. That is live interoperability evidence; it does not establish that the busy condition occurred in the game. The deterministic held-pipe fixture above is the before/after proof.

Testing

Tested: raw CRT/Win32 busy and missing probe; two native held-pipe regression pairs; all five new native methods; existing test_oot_state_tap.py and test_longshot_ctl.py pass under the same toolchain as the original failure; independent read-only review of handle types, last-error capture and ownership/cleanup. Full python -I -B tests/run_all.py --jobs8 on f0645813 (the final commit adds only the separately validated live-run record): 535 passed, 2 failed, 34 skipped in161s. Both failures also occur on untouched main9c9d69ed in fresh focused checks: test_tested_configurations.py (newline assumption, fixed by PR493) and test_walk_key.py (missing stand-in after Skip-title merged, already claimed by mooki). The original test_oot_state_tap.py failure passes here. Existing assertions/skips are unchanged; the new pipe suite is Windows-specific. Run-fragment validation and git diff --check pass.

The independent title-menu check above used executable/renderer built together from main9c9d69ed and only this PR's client script. All23 protected player hashes remained unchanged. No binaries, game files or saves were transferred.

Not tested: a real ACL denial (access denied was simulated and asserts one attempt), high-contention/stress or nonfinite timeouts, Python versions besides3.14.7, live Linux/macOS transport. The POSIX branch is unchanged. The live game check covers status/quit, not gameplay input, audio or a save/checkpoint barrier. Native failures are tested with authored pipes; no security control was bypassed.

Needs other testers: None for this Windows connection correction.

Screenshots

Nothing on screen; diagnostic output is in Measurements.

Risk and rollback

Windows connection errors now retain their native code, so their text can differ from the old CRT error. The main risk is handle ownership; actual-pipe tests cover success and both conversion cleanup paths. Reverting this change restores the CRT open. There is no retry for missing pipes, access denial or arbitrary EINVAL.

Reviewer notes

Start at open_windows_pipe. use_last_error=True and immediate capture preserve the error that the old open() path lost. After open_osfhandle succeeds the CRT owns the handle; after fdopen succeeds the file object owns the descriptor. Cleanup suppression is confined to a descriptor close after file-wrapper construction has already failed, so that it cannot hide the original exception.

Issues

Fixes #481

Checklist

For everyone

One topic.
The title is a Conventional Commit (type(scope): description).
Rebased on current main.
python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
Tested in the game, or what wasn't is named under Testing.
Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
No game files, dumps, build output or generated files.
third_party/ untouched (a new folder with a PROVENANCE.md instead).
The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
A file in docs/tested/ for each new measured run (format in docs/tested-configurations.md; no appended row or assembled view).
A game-free test added for the fix, failing on main and passing here.
A check under tests/ that reads changed source updated in this PR, not weakened.
Docs updated.

## Why Back-to-back control clients on Windows can report "no game listens (Invalid argument)" while the bridge server is running. The CRT converts a busy named-pipe open into errno22 without retaining Win32 error231, so the client's intended busy retry never happens. ## What changed - `scripts/longshot_ctl.py`: use typed `CreateFileW` for Windows pipe opens and preserve the native error. Retry only `ERROR_PIPE_BUSY` within the existing deadline; missing and denied pipes fail immediately. Transfer the handle to the CRT file object and close the appropriate owner on conversion failure, preserving the original exception. - `tests/test_windows_pipe_busy.py`: five native test methods cover busy timeout/recovery, missing pipes, simulated access denial, handle-conversion failure and file-wrapper failure before/after descriptor ownership changes. - Four `docs/tested/` files record the held-pipe regression comparisons; one more records the actual-game title interoperability check. POSIX transport and the JSON protocol are unchanged. ## Flags and settings None. Connection timeout and CLI arguments are unchanged. ## Measurements main: 9c9d69ed09e2c24965d35f1696aa5c8dd8fe6e40 (2026-11-14) this PR: fix/windows-bridge-pipe-busy @ 70c9af27319b8bbf172498636b7a67bcd616f78e Hardware: Ryzen7 9800X3D, 96GiB RAM, Windows11 build26300, native Python3.14.7. Named-pipe fixtures use no GPU. The separate live check used RTX5090, driver617.42. Scene: one held real Windows pipe, then rearming it; separate live check at the title menu in an independent headless game profile. Method: focused checks as warm-up, then two sequential interleaved main/PR/main/PR runs of the same deterministic held-pipe timeout/recovery method against the actual production client modules. Report median failures; this is behavior evidence, not a speed claim. A separate raw probe compared CRT open with native CreateFileW on the same busy pipe. How to reproduce: run `python -I -B tests/test_windows_pipe_busy.py` on Windows. For main, run the same `test_busy_timeout_and_recovery` fixture with main's client module. The fixture occupies the pipe, checks a bounded busy timeout, then rearms its server and verifies a connection succeeds. | Check | main @ 9c9d69ed | this PR @ 70c9af27 | |---|---|---| | Held-pipe method failures, run1 / run2 | 1 / 1 | 0 / 0 | | Median failures | 1 | 0 | | Errors / skipped methods, each run | 0 / 0 | 0 / 0 | ```text raw busy-pipe probe: CRT errno22, winerror absent; CreateFileW error231 raw missing-pipe probe: CRT errno2, winerror absent; CreateFileW error2 main, each held-pipe run: methods1, failures1, errors0, skips0 this PR, each held-pipe run: methods1, failures0, errors0, skips0 new native suite: five methods pass, including both file-wrapper ownership subcases ``` The measured source commit wasd0340ee9; later commits add only the five run records, with production/test files unchanged. Four consecutive fixed-client connections to the actual matched API22 game each returned a protocol1 status reply at the title menu (frames1339/1354/1367/1382, state=menu). Game and guard exited0. That is live interoperability evidence; it does not establish that the busy condition occurred in the game. The deterministic held-pipe fixture above is the before/after proof. ## Testing **Tested:** raw CRT/Win32 busy and missing probe; two native held-pipe regression pairs; all five new native methods; existing `test_oot_state_tap.py` and `test_longshot_ctl.py` pass under the same toolchain as the original failure; independent read-only review of handle types, last-error capture and ownership/cleanup. Full `python -I -B tests/run_all.py --jobs8` on f0645813 (the final commit adds only the separately validated live-run record): **535 passed, 2 failed, 34 skipped in161s**. Both failures also occur on untouched main9c9d69ed in fresh focused checks: `test_tested_configurations.py` (newline assumption, fixed by PR493) and `test_walk_key.py` (missing stand-in after Skip-title merged, already claimed by mooki). The original `test_oot_state_tap.py` failure passes here. Existing assertions/skips are unchanged; the new pipe suite is Windows-specific. Run-fragment validation and `git diff --check` pass. The independent title-menu check above used executable/renderer built together from main9c9d69ed and only this PR's client script. All23 protected player hashes remained unchanged. No binaries, game files or saves were transferred. **Not tested:** a real ACL denial (access denied was simulated and asserts one attempt), high-contention/stress or nonfinite timeouts, Python versions besides3.14.7, live Linux/macOS transport. The POSIX branch is unchanged. The live game check covers status/quit, not gameplay input, audio or a save/checkpoint barrier. Native failures are tested with authored pipes; no security control was bypassed. **Needs other testers:** None for this Windows connection correction. ## Screenshots Nothing on screen; diagnostic output is in Measurements. ## Risk and rollback Windows connection errors now retain their native code, so their text can differ from the old CRT error. The main risk is handle ownership; actual-pipe tests cover success and both conversion cleanup paths. Reverting this change restores the CRT open. There is no retry for missing pipes, access denial or arbitrary EINVAL. ## Reviewer notes Start at `open_windows_pipe`. `use_last_error=True` and immediate capture preserve the error that the old `open()` path lost. After `open_osfhandle` succeeds the CRT owns the handle; after `fdopen` succeeds the file object owns the descriptor. Cleanup suppression is confined to a descriptor close after file-wrapper construction has already failed, so that it cannot hide the original exception. ## Issues Fixes #481 ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A file in `docs/tested/` for each new measured run (format in `docs/tested-configurations.md`; no appended row or assembled view). - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. - [x] Docs updated.

monsterlag added 2 commits

2026-11-14 16:41:00 +00:00

fix(control): wait for busy Windows pipes with the native error code d0340ee971

Open the client handle with CreateFileW so ERROR_PIPE_BUSY survives the
CRT's errno conversion. Retry only that code within the existing deadline;
missing or denied pipes fail immediately. Transfer handle ownership to
the CRT file object and preserve conversion failures during cleanup.

Add native pipe regressions for timeout/recovery, missing pipes, simulated
access denial and both handle/file-wrapper failure cleanup paths.

Assisted-by: OpenAI Codex
Co-Authored-By: OpenAI Codex <[email protected]>

docs(tested): record current game-free regression comparisons

PR description / Description follows the template (pull_request_target) Successful in 3s

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

Website / Website on Cloudflare (pull_request) Has been cancelled

Details

f0645813dc

Keep each measured native regression run self-contained under docs/tested
using the current contribution format. These are CPU-only tool checks;
they make no gameplay, GPU compatibility or performance claim.

Assisted-by: OpenAI Codex
Co-Authored-By: OpenAI Codex <[email protected]>

monsterlag changed title from ~~WIP: fix(control): wait for busy Windows pipes using the native error code~~ to fix(control): wait for busy Windows pipes using the native error code

2026-11-14 16:42:09 +00:00

monsterlag commented

2026-11-14 16:46:59 +00:00

Author

First-time contributor

Added the self-contained actual-game title interoperability record alongside the four authored-pipe regression records. Production and test code are unchanged; fragment validation passes after the data-only commit. The four live replies prove interoperability, while the deterministic held-pipe pairs remain the before/after busy-condition proof. Full-suite commit and remaining baseline failures are named explicitly.

monsterlag added 1 commit

2026-11-14 16:46:59 +00:00

docs(tested): record current game-free regression comparisons

CI / Conventional Commit title (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions

Details

CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions

Details

Third-party parts / As their notes say (pull_request) Blocked by required conditions

Details

Third-party parts / The same as upstream (pull_request) Blocked by required conditions

Details

Website / Website on Cloudflare (pull_request) Blocked by required conditions

Details

PR description / Description follows the template (pull_request_target) Successful in 5s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

70c9af2731

Keep each measured native regression run self-contained under docs/tested
using the current contribution format. These are CPU-only tool checks;
they make no gameplay, GPU compatibility or performance claim.

Assisted-by: OpenAI Codex
Co-Authored-By: OpenAI Codex <[email protected]>