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 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>

416b3103bf

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:

#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.

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

Concept ACK 30ae05a21bdb30e63400385cf6d9dfd87dccfc70

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>

00fe470459

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>

c02747ea37

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