test: Check bitcoin wrapper child exit status on windows #36190

Bortlesboat commented at 5:16 AM on September 8, 2026: contributor

Follow-up to #36105, which makes the Windows bitcoin.exe wrapper wait for its child and forward the child's exit code. This adds the functional test for that.

The case worth covering is exit code -1. The first version of the wrapper used _spawnvp(_P_WAIT), which returns -1 both when the spawn fails and when the child started fine and exited with -1, so bitcoin.exe reported a startup error and exited 1 instead of passing -1 through. #36105 now uses _P_NOWAIT + _cwait, which keeps the spawn result and the child status separate. Against the old launcher this test fails at not(1 == 4294967295).

The test copies bitcoin.exe into a temp dir next to a copy of cmd.exe named bitcoind.exe, so bitcoin -M node /d /c exit N runs the fake child, and checks 0, 1 and -1 come back unchanged. Windows reports exit codes as unsigned 32-bit, so -1 arrives as 4294967295. It also checks the wrapper adds nothing to stdout/stderr. Windows only.

ryanofsky preferred keeping the runtime fix in #36105 and reviewing the test here (https://github.com/bitcoin/bitcoin/pull/36190#issuecomment-5603692520), so this is the same commit he briefly carried in his branch. The branch still contains #36105's commits and should land after it.

Checked on a native Windows 11 clang/UCRT release build, wallet/GUI/IPC off:

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

Fails against the pre-fix launcher, passes against the fixed one, and the rest of tool_bitcoin.py still passes. Use a fresh --tmpdir when re-running.

util: Fix ExecVp on Windows to wait for child process

Switch from _execvp to _spawnvp(_P_NOWAIT) + _cwait so bitcoin.exe
waits for the child process to finish and forwards its exit code.
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. _P_NOWAIT is
used instead of _P_WAIT so that a child exit code of -1 (0xffffffff) is
not confused with a spawn failure: _spawnvp(_P_WAIT) returns -1 for both,
but _spawnvp(_P_NOWAIT) returns the child handle on success, and _cwait
fills a separate status that is forwarded via _exit.

Co-Authored-By: Bortlesboat <169967362+Bortlesboat@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

6bce70ad03

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>

94d3855531

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

Now that util::ExecVp on Windows uses _spawnvp(_P_NOWAIT) + _cwait
instead of _execvp, 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: Hodlinator <172445034+hodlinator@users.noreply.github.com>

543bf0b1fe

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>

5cfe9d873c

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)
#36105 (bitcoin wrapper: Fix Windows exec so wrapper waits for child process 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.

ryanofsky commented at 2:38 PM on September 9, 2026: contributor

re: #36190 (comment)

Would you be happy to leave the test and its imports out of your branch?

No problem, dropped the test and happy to review it here

Bortlesboat referenced this in commit 57b837251e on Sep 9, 2026

Bortlesboat force-pushed on Sep 9, 2026

Bortlesboat renamed this:
~~util: preserve Windows child exit status -1~~
test: Check Windows wrapper child exit status
on Sep 9, 2026

Bortlesboat marked this as ready for review on Sep 9, 2026

Bortlesboat renamed this:
~~test: Check Windows wrapper child exit status~~
test: Check bitcoin wrapper child exit status on windows
on Sep 9, 2026

test: Check bitcoin wrapper child exit status on windows

This detects a bug that was present in the original implementation of the
earlier commit "Fix ExecVp on Windows to wait for child process"

Co-Authored-By: Ryan Ofsky <ryan@ofsky.org>

fcc3b9c7ae

Bortlesboat force-pushed on Sep 9, 2026

DrahtBot added the label CI failed on Sep 9, 2026

DrahtBot removed the label CI failed on Sep 9, 2026