util: preserve Windows child exit status -1 #36190

pull Bortlesboat wants to merge 5 commits into bitcoin:master from Bortlesboat:review/windows-launcher-36105-20260907 changing 6 files +63 −20

Bortlesboat commented at 5:16 AM on September 8, 2026: contributor
Preserve the Windows wrapper's child exit status when the child exits with 0xffffffff, rather than reporting a startup error and exiting with 1.

Draft follow-up to #36105. @ryanofsky, 1300cb1a5b is the only additional commit and can be folded into your branch. This branch includes the four prerequisite commits and should not be merged independently.

At 30ae05a21bdb30e63400385cf6d9dfd87dccfc70, _spawnvp(_P_WAIT) uses the same return value for a valid child status of -1 and a spawn failure. Starting with _P_NOWAIT and waiting with _cwait separates the wait result from the child's exit status. The regression uses a temporary copy of the Windows command interpreter as the child, with fixed exit codes 0, 1 and -1.

AI disclosure: Codex prepared the patch, ran the checks below, and wrote/submitted this draft at the account owner's request. Human review is pending.

Validation on native Windows 11 build 26200, Clang 22.1.8 / UCRT, Release, warnings as errors:
- The new regression with the original PR launcher fails at not(1 == 4294967295).
- With the correction, tool_bitcoin.py passes, including its existing non-IPC checks.
- Native bitcoin, bitcoind and bitcoin-cli builds, eight controlled launcher comparisons, and changed-file Ruff checks pass.
For a configured native build:
```
cmake --build build --target bitcoin bitcoind bitcoin-cli
python test/functional/tool_bitcoin.py --configfile=build/test/config.ini --tmpdir=build/tool-bitcoin-exit-status
```
Use a fresh test directory on repeated runs. Wallet, GUI and IPC were disabled. The full unit suite, GUI test and a fully MSVCRT-linked build were not tested.

util: Fix ExecVp on Windows to wait for child process

Switch from _execvp to _spawnvp(_P_WAIT) so bitcoin.exe waits for the child
process to finish. Previously _execvp would exit the parent as soon as the
child started, making it impossible for anything waiting on bitcoin.exe (such
as a test framework) to track whether the child succeeded or failed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

80fc82c4ae

bitcoin: Fix msvcrt regressions from _execvp to _spawnvp switch

Fix two msvcrt-specific issues introduced by switching from _execvp to
_spawnvp in the previous commit. _execvp (via the underlying CreateProcess)
handled both correctly; msvcrt's _spawnvp does not.

1. msvcrt's _spawnvp does not append ".exe" to paths without an extension,
   so any attempt to launch "bitcoind" (no extension) fails with EINVAL.

2. msvcrt's _spawnvp returns EINVAL (not ENOENT) for any missing file. The
   allow_notfound fallback only checked ENOENT, so on msvcrt it never tried
   the next search path.

Both issues are absent in ucrt (MinGW-ucrt and MSVC since VS 2015), guarded
with !defined(_UCRT).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

09f3531588

test: Enable tool_bitcoin.py and interface_gui.py tests on Windows

Now that util::ExecVp on Windows uses _wspawnvp(_P_WAIT) instead of
_wexecvp, the bitcoin wrapper process blocks until the child exits and
Python can capture its stdout/stderr and exit code normally. Remove the
Windows skips added in #33229 and #35551.

The interface_gui.py test is still skipped in vcpkg Qt builds. vcpkg builds Qt
with -opengl dynamic, making the minimal platform plugin unusable due to
internal Qt bugs. This matches existing logic in src/qt/test/CMakeLists.txt
avoiding the minimal platform plugin with test_bitcoin-qt.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

6ca9199eec

bitcoin: Use generic_category for errno from exec/spawn CRT functions

Switch from system_category to generic_category when throwing from
ExecVp failure, so errno is read as a POSIX value. On Windows,
system_category interprets codes as Win32 errors, so EINVAL=22 would
produce "The device does not recognize the command" instead of
"Invalid argument".

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

30ae05a21b

util: Preserve Windows child exit status -1 1300cb1a5b
DrahtBot added the label Utils/log/libs on Sep 8, 2026
DrahtBot commented at 5:16 AM on September 8, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36190.



Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #36106 (bitcoin wrapper: respect CMAKE_INSTALL_BINDIR/LIBEXECDIR by ryanofsky)
- #36022 (test: add coverage for bitcoin wrapper argument handling by cyb3ralbert)
- #32387 (ipc: add windows support by ryanofsky)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
ryanofsky commented at 2:21 AM on September 9, 2026: contributor

Thanks @Bortlesboat! I added your fix and test to #36105 since I think it makes more sense to change the execvp code once instead of multiple times. I think with those changes it probably makes sense to close this PR. I can also revert or partially revert though if you'd prefer this PR to be separate.
Bortlesboat commented at 4:51 AM on September 9, 2026: contributor

Thanks Ryan. I'd prefer to keep the exit-status regression here as a test-only follow-up, with the runtime fix staying in #36105. Would you be happy to leave the test and its imports out of your branch? I can rebase this PR once that's done.

Contributors

Labels

Linked (view graph)

#36105 bitcoin wrapper: Fix Windows exec so wrapper waits for child process

util: preserve Windows child exit status -1 #36190

Code Coverage & Benchmarks

Reviews

Conflicts