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

ryanofsky commented at 5:34 PM on August 27, 2026: contributor

Problem: On Windows, bitcoin.exe spawns a child process and immediately exits rather than waiting for it to finish. This breaks capturing output and checking exit codes in scripts and tests.

Solution: Switch util::ExecVp from _execvp to _spawnvp(_P_WAIT), which blocks until the child exits and forwards its exit code. Also enable tool_bitcoin.py and interface_gui.py tests which were skipped due to previous behavior.

This fix requires small changes to src/util/exec.cpp and src/bitcoin.cpp and there is also an extra commit fixing errno message strings on Windows when ExecVp fails. Details in commit messages.

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 EINVAL regression from _execvp to _spawnvp switch

Treat EINVAL from _spawnvp as "not found" on msvcrt so the wrapper moves on
to the next candidate path. msvcrt's _spawnvp returns EINVAL rather than
ENOENT when the file name has a directory component and the file does not
exist (see code comment), so after the previous commit switched from _execvp
to _spawnvp, msvcrt builds threw on the first nonexistent candidate,
libexec/bitcoind, instead of trying the others. _execvp and ucrt's _spawnvp
return ENOENT, so ucrt builds (MinGW-ucrt and MSVC) are unaffected and the
change is guarded with !defined(_UCRT).

Measured by calling the CRT functions directly on GitHub's windows-2022 and
windows-2025 runners, through msvcrt.dll and ucrtbase.dll via ctypes and
through MinGW (msvcrt and ucrt) and MSVC builds:
https://github.com/ryanofsky/bitcoin/actions/runs/34365395636
https://github.com/ryanofsky/bitcoin/actions/runs/34367754505

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

68ae4a0f84

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>

ac15f5654a

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>

2b8779842a

DrahtBot commented at 5:34 PM on August 27, 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/36105.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Concept ACK	hebasto, hodlinator

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#36190 (test: Check bitcoin wrapper child exit status on windows by Bortlesboat)
#36106 (bitcoin wrapper: respect CMAKE_INSTALL_BINDIR/LIBEXECDIR by ryanofsky)
#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.

hebasto commented at 10:17 PM on August 27, 2026: member

Concept ACK.

hebasto commented at 10:23 PM on August 27, 2026: member

There is a preliminary understanding among the build crowd that #33593 might land shortly after branching off "32.x", which renders the "bitcoin: Fix msvcrt regressions from _execvp to _spawnvp switch" commit unnecessary.

bitcoin deleted a comment on Aug 31, 2026

in test/functional/test_framework/test_framework.py:1 in 6ca9199eec outdated

hodlinator commented at 12:06 PM on August 31, 2026:

In 6ca9199eecd9934563351c6b4b11468b75c6d088 - "test: Enable tool_bitcoin.py and interface_gui.py tests on Windows":

The commit message refers to _wspawnvp() and _wexecvp(), however we use the non-wide versions in code.

hodlinator commented at 8:59 AM on September 8, 2026:

https://github.com/bitcoin/bitcoin/blob/33a363ea250839ca31ea043b7500789d2e5d844b/doc/AI_POLICY.md#L23-L26

ryanofsky commented at 1:45 AM on September 9, 2026:

re: #36105 (review)

The commit message refers to _wspawnvp() and _wexecvp(), however we use the non-wide versions in code.

Good catch. These changes are older than #35704 and the commit message was not updated when it was merged.

ryanofsky commented at 1:46 AM on September 9, 2026:

re: #36105 (review)

Thanks. It's important to me to distinguish words and ideas which are my own from those that come from other sources, so I use co-author tags for this. I think the AI policy was written with a different kind of PR in mind than this one. I understand the changes in this PR and take responsibility for them.

in src/bitcoin.cpp:214 in 09f3531588 outdated

 209 |          exec_args[0] = exe_path_str.c_str();
 210 |          if (util::ExecVp(exec_args[0], (char*const*)exec_args.data()) == -1) {
 211 | +#if defined(WIN32) && !defined(_UCRT)
 212 | +            // msvcrt's _spawnvp returns EINVAL (not ENOENT) for any missing
 213 | +            // file, even after ".exe" is appended. ucrt returns ENOENT.
 214 | +            if (allow_notfound && (errno == ENOENT || errno == EINVAL)) return false;

hodlinator commented at 12:13 PM on August 31, 2026:

Checked https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/spawnvp-wspawnvp?view=msvc-170 but this difference in error codes between runtimes is not documented there. Is it admitted elsewhere?

ryanofsky commented at 2:14 AM on September 9, 2026:

re: #36105 (review)

Checked https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/spawnvp-wspawnvp?view=msvc-170 but this difference in error codes between runtimes is not documented there. Is it admitted elsewhere?

It's not documented anywhere and was just seen in CI. I was meaning to follow up and get more information about this though, so I'll try to do that soon.

ryanofsky commented at 4:13 PM on September 9, 2026:

re: #36105 (review)

this difference in error codes between runtimes is not documented there

Followed up and this difference does not seem to be documented anywhere, only observed in CI. Did some more testing though and was able to simplify and write a better comment which is in the latest push

hodlinator commented at 12:16 PM on August 31, 2026: contributor

Concept ACK 30ae05a21bdb30e63400385cf6d9dfd87dccfc70

ryanofsky force-pushed on Sep 9, 2026

ryanofsky commented at 2:18 AM on September 9, 2026: contributor

Updated 30ae05a21bdb30e63400385cf6d9dfd87dccfc70 -> c02747ea3706e0c028b4be61a4dff054d65f3a3e (pr/bitwin.8 -> pr/bitwin.9, compare) with exit code status fix and test from @Bortlesboat from https://github.com/bitcoin/bitcoin/pull/36190

ryanofsky force-pushed on Sep 9, 2026

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

Updated c02747ea3706e0c028b4be61a4dff054d65f3a3e -> 359a57d653406365a6158457e77ff926d6f58d0d (pr/bitwin.9 -> pr/bitwin.10, compare) just dropping test commit 00fe470459ff36f22ef4a2e7d3e3e8903dac207b as requested

Updated 359a57d653406365a6158457e77ff926d6f58d0d -> 5cfe9d873c7b3ca826467e33b6b650f43957d4e8 (pr/bitwin.10 -> pr/bitwin.11, compare) to fix lint (unused os/shutil/subprocess imports in tool_bitcoin.py) https://github.com/bitcoin/bitcoin/actions/runs/34364703981/job/102510308140

Updated 5cfe9d873c7b3ca826467e33b6b650f43957d4e8 -> 2b8779842a3fcb4afaa533a0490b9618ccc42c36 (pr/bitwin.11 -> pr/bitwin.12, compare) simplifying MSVCRT workaround and documenting it better

ryanofsky force-pushed on Sep 9, 2026

DrahtBot added the label CI failed on Sep 9, 2026

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

ryanofsky force-pushed on Sep 9, 2026

DrahtBot removed the label CI failed on Sep 9, 2026