Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions .github/workflows/ci-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,92 @@ jobs:
export MCPP_VENDORED_XLINGS=$(cygpath -w "$USERPROFILE/.xlings/subos/default/bin/xlings.exe")
"$MCPP_SELF" test

# Regression test for the Windows first-run "press Enter to advance" hang.
# Launches mcpp with an OPEN, EMPTY, never-closing stdin pipe. Without
# seal_stdin's Windows fix, any grandchild that reads stdin would inherit
# our pipe and block forever — caught by the timeout below. With the fix,
# every subprocess stdin is redirected from NUL → no possibility of hang.
- name: "Regression: mcpp survives open-empty-stdin (Windows hang fix)"
shell: pwsh
timeout-minutes: 15
env:
MCPP_VENDORED_XLINGS: ${{ env.XLINGS_BIN }}
run: |
$ErrorActionPreference = 'Stop'

# MCPP_SELF was set in a bash step as an MSYS-style path
# (e.g. /d/a/mcpp/...). PowerShell can't exec that — convert it
# to a native Windows path via the git-bash cygpath that ships
# on the runner.
$mcppExe = (& 'C:\Program Files\Git\usr\bin\cygpath.exe' -w $env:MCPP_SELF).Trim()
Write-Host "Resolved MCPP_SELF (Windows form): $mcppExe"
if (-not (Test-Path $mcppExe)) {
throw "MCPP_SELF after cygpath not found: $mcppExe"
}

$tmp = Join-Path $env:RUNNER_TEMP ("stdin-hang-test-" + [guid]::NewGuid().ToString('N'))
New-Item -ItemType Directory -Path $tmp | Out-Null
Set-Location $tmp
& $mcppExe new hello_stdin
Set-Location hello_stdin

function Invoke-McppWithOpenStdin {
param([string]$McppPath, [string]$McppArgs, [int]$TimeoutSeconds = 300)

$psi = [System.Diagnostics.ProcessStartInfo]::new()
$psi.FileName = $McppPath
$psi.Arguments = $McppArgs
$psi.WorkingDirectory = (Get-Location).Path
$psi.UseShellExecute = $false
$psi.RedirectStandardInput = $true # parent holds child's stdin
$psi.RedirectStandardOutput = $true
$psi.RedirectStandardError = $true
# By default the child inherits the parent's env (we did not
# touch $psi.Environment) so MCPP_VENDORED_XLINGS / PATH / etc.
# propagate.

$p = [System.Diagnostics.Process]::Start($psi)

# Async-drain stdout/stderr so a full output buffer doesn't
# itself deadlock the child (separate failure mode from the
# stdin hang we're testing).
$stdoutTask = $p.StandardOutput.ReadToEndAsync()
$stderrTask = $p.StandardError.ReadToEndAsync()

# NEVER write or close $p.StandardInput — the pipe stays open
# and empty for the lifetime of the child. Any grandchild that
# reads stdin will block on this pipe → caught by WaitForExit.

if (-not $p.WaitForExit($TimeoutSeconds * 1000)) {
try { $p.Kill($true) } catch {}
Write-Host "----- captured stdout -----"
Write-Host $stdoutTask.Result
Write-Host "----- captured stderr -----"
Write-Host $stderrTask.Result
throw "REGRESSION: 'mcpp $McppArgs' HUNG with open-empty stdin after ${TimeoutSeconds}s. The Windows seal_stdin fix is not effective."
}

Write-Host "----- stdout -----"
Write-Host $stdoutTask.Result
Write-Host "----- stderr -----"
Write-Host $stderrTask.Result

if ($p.ExitCode -ne 0) {
throw "'mcpp $McppArgs' exited with code $($p.ExitCode) (no hang, but failed)."
}
}

Write-Host '=== T1: mcpp --version (sanity, fast path) ==='
Invoke-McppWithOpenStdin -McppPath $mcppExe -McppArgs '--version' -TimeoutSeconds 30

Write-Host '=== T2: mcpp build (full bootstrap + toolchain + dep resolve + compile) ==='
Invoke-McppWithOpenStdin -McppPath $mcppExe -McppArgs 'build' -TimeoutSeconds 600

Write-Host '=== T3: mcpp run (post-build run path) ==='
Invoke-McppWithOpenStdin -McppPath $mcppExe -McppArgs 'run' -TimeoutSeconds 120

Write-Host 'SUCCESS: mcpp completes with open-empty stdin → Windows seal_stdin fix verified.'

- name: E2E suite
shell: bash
run: |
Expand Down
24 changes: 16 additions & 8 deletions src/platform/process.cppm
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
// mcpp.platform.process — platform-aware process runner.
//
// Centralises all popen/system usage so callers do not scatter #if _WIN32
// guards or duplicate the popen-read loop. On POSIX, all functions
// automatically redirect stdin from /dev/null to prevent interactive
// prompts from child processes (fixes macOS first-run hangs where xcrun
// or xcode-select would block waiting for user input).
// guards or duplicate the popen-read loop. All functions automatically
// seal stdin (redirect from /dev/null on POSIX, from NUL on Windows) to
// prevent interactive prompts from child processes:
// - POSIX: fixes macOS first-run hangs where xcrun / xcode-select would
// block waiting for user input.
// - Windows: fixes first-run hangs where xlings / xim / curl / git child
// processes would block on terminal stdin, forcing the user to press
// Enter repeatedly to advance bootstrap / toolchain install.
//
// Entry points:
// capture — run a command, capture stdout
Expand Down Expand Up @@ -73,12 +77,16 @@ namespace mcpp::platform::process {

namespace {

// On POSIX, append "< /dev/null" to prevent child processes from reading
// stdin. This fixes macOS first-run hangs where tools like xcrun or
// xcode-select block waiting for user input.
// Append a non-interactive stdin redirect to prevent child processes from
// blocking on terminal input.
// - POSIX: "< /dev/null" — fixes macOS xcrun / xcode-select hangs.
// - Windows: "<NUL" — fixes xlings / xim / curl / git hangs on
// first-run toolchain install (user otherwise
// had to press Enter repeatedly to advance).
// `cmd.exe` accepts `<NUL` as a redirect for an immediately-EOF stdin.
std::string seal_stdin(std::string_view cmd) {
#if defined(_WIN32)
return std::string(cmd);
return std::string(cmd) + " <NUL";
#else
return std::string(cmd) + " </dev/null";
#endif
Expand Down
5 changes: 4 additions & 1 deletion src/platform/shell.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ export namespace mcpp::platform::shell {
// Platform-aware shell argument quoting.
std::string quote(std::string_view s);

// Full silent redirect (stdin + stdout + stderr → /dev/null).
// Silent redirect — stdout + stderr → /dev/null (or NUL on Windows).
// stdin is NOT touched here; that's the responsibility of
// mcpp::platform::process::seal_stdin, which is auto-applied by capture /
// run_silent / run_streaming on all platforms.
#if defined(_WIN32)
constexpr std::string_view silent_redirect = ">nul 2>&1";
#else
Expand Down
13 changes: 10 additions & 3 deletions src/xlings.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -681,9 +681,16 @@ int install_with_progress(const Env& env, std::string_view target,
{
auto directCmd = build_command_prefix(env) +
std::format(" install {} -y {}", target, mcpp::platform::shell::silent_redirect);
// Use std::system() directly — do NOT redirect stdin via </dev/null
// because xlings may need stdin for subprocess coordination during
// large package extraction.
// Windows: explicitly seal stdin (<NUL) so xlings and any grandchildren
// (curl, git, 7z, etc.) cannot block on a terminal read. The earlier
// comment claimed POSIX must keep stdin open for "subprocess
// coordination" — that's never been observed in practice on Linux/macOS,
// and on Windows it caused users to press Enter repeatedly during first-
// run toolchain install. POSIX keeps the original behavior to stay
// conservative.
if constexpr (mcpp::platform::is_windows) {
directCmd += " <NUL";
}
int directRc = mcpp::platform::process::extract_exit_code(
std::system(directCmd.c_str()));
if (directRc == 0) return 0;
Expand Down
164 changes: 164 additions & 0 deletions tests/unit/test_process_seal_stdin.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
// Regression test for the Windows first-run hang where xlings / xim / curl /
// git child processes blocked on terminal stdin, forcing the user to press
// Enter repeatedly to advance bootstrap / toolchain install.
//
// `mcpp::platform::process::{capture, run_silent, run_streaming}` MUST seal
// stdin so any child reading stdin gets immediate EOF, not a blocking read.
// POSIX: appends "</dev/null"
// Windows: appends "<NUL"
//
// Test strategy: rebind this test process's own stdin to an open, empty,
// never-closing pipe. Then invoke run_silent / capture with a child that
// reads stdin. Without seal_stdin, the child would inherit our pipe stdin
// and block forever; the gtest runner would then hang until CI timeout.
// With the fix, the child reads from NUL / /dev/null and exits immediately.

#include <gtest/gtest.h>
#include <chrono>

#if defined(_WIN32)
#include <windows.h>
#include <io.h>
#include <fcntl.h>
#else
#include <fcntl.h>
#include <unistd.h>
#endif

import std;
import mcpp.platform.process;

namespace {

// Maximum seconds a sealed-stdin command may take before we declare it
// "hung". Real child runs (cat / more reading from EOF stdin) complete in
// well under 100ms on any modern machine, so 5s is a very generous bound.
constexpr int kMaxSealedSeconds = 5;

// RAII: rebind STDIN to an open, empty, never-closing pipe for the duration
// of one test. Restores the original stdin on destruction.
class OpenEmptyStdinScope {
public:
OpenEmptyStdinScope() {
#if defined(_WIN32)
SECURITY_ATTRIBUTES sa{};
sa.nLength = sizeof(sa);
sa.bInheritHandle = TRUE;
if (!CreatePipe(&hRead_, &hWrite_, &sa, 0)) {
std::abort();
}
// Make the read end inheritable (already is via sa, but be explicit).
SetHandleInformation(hRead_, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT);

// Save the original stdin (both Win32 handle + CRT fd) so we can
// restore in the destructor.
origStdinHandle_ = GetStdHandle(STD_INPUT_HANDLE);
origStdinFd_ = _dup(0);

// Bind the pipe-read-end as our process's stdin at both layers.
SetStdHandle(STD_INPUT_HANDLE, hRead_);
int newFd = _open_osfhandle(reinterpret_cast<intptr_t>(hRead_),
_O_RDONLY | _O_BINARY);
if (newFd >= 0) {
_dup2(newFd, 0);
_close(newFd); // _dup2 keeps a reference; we're done with newFd
}
#else
if (::pipe(pipeFds_) != 0) std::abort();
origStdinFd_ = ::dup(0);
::dup2(pipeFds_[0], 0);
#endif
}

~OpenEmptyStdinScope() {
#if defined(_WIN32)
// Restore original stdin.
if (origStdinFd_ >= 0) {
_dup2(origStdinFd_, 0);
_close(origStdinFd_);
}
SetStdHandle(STD_INPUT_HANDLE, origStdinHandle_);
if (hWrite_) CloseHandle(hWrite_);
if (hRead_) CloseHandle(hRead_);
#else
if (origStdinFd_ >= 0) {
::dup2(origStdinFd_, 0);
::close(origStdinFd_);
}
::close(pipeFds_[0]);
::close(pipeFds_[1]);
#endif
}

OpenEmptyStdinScope(const OpenEmptyStdinScope&) = delete;
OpenEmptyStdinScope& operator=(const OpenEmptyStdinScope&) = delete;

private:
#if defined(_WIN32)
HANDLE hRead_ = nullptr;
HANDLE hWrite_ = nullptr; // intentionally never written → reader blocks
HANDLE origStdinHandle_ = nullptr;
int origStdinFd_ = -1;
#else
int pipeFds_[2] = {-1, -1}; // intentionally never written → reader blocks
int origStdinFd_ = -1;
#endif
};

// A child command that reads stdin to EOF and exits.
// With seal_stdin in effect → stdin is NUL / /dev/null → child exits immediately.
// Without seal_stdin AND with an open-empty parent stdin → child blocks forever.
constexpr std::string_view kStdinReaderCmd =
#if defined(_WIN32)
"more >nul 2>&1"
#else
"cat >/dev/null"
#endif
;

template <class F>
double time_seconds(F&& fn) {
auto t0 = std::chrono::steady_clock::now();
fn();
auto t1 = std::chrono::steady_clock::now();
return std::chrono::duration<double>(t1 - t0).count();
}

} // namespace

// run_silent: must seal stdin so the child does not inherit our pipe stdin
// and block forever.
TEST(ProcessSealStdin, RunSilentDoesNotHangWhenParentStdinIsOpenPipe) {
OpenEmptyStdinScope scope;
double elapsed = time_seconds([] {
(void)mcpp::platform::process::run_silent(kStdinReaderCmd);
});
EXPECT_LT(elapsed, static_cast<double>(kMaxSealedSeconds))
<< "run_silent took " << elapsed
<< "s — child blocked on stdin → seal_stdin is broken or not applied";
}

// capture: must also seal stdin (it shares seal_stdin with run_silent).
TEST(ProcessSealStdin, CaptureDoesNotHangWhenParentStdinIsOpenPipe) {
OpenEmptyStdinScope scope;
double elapsed = time_seconds([] {
(void)mcpp::platform::process::capture(kStdinReaderCmd);
});
EXPECT_LT(elapsed, static_cast<double>(kMaxSealedSeconds))
<< "capture took " << elapsed
<< "s — child blocked on stdin → seal_stdin is broken or not applied";
}

// run_streaming: same property — children spawned via popen-streaming must
// not inherit a live stdin.
TEST(ProcessSealStdin, RunStreamingDoesNotHangWhenParentStdinIsOpenPipe) {
OpenEmptyStdinScope scope;
double elapsed = time_seconds([] {
(void)mcpp::platform::process::run_streaming(
kStdinReaderCmd,
[](std::string_view) {});
});
EXPECT_LT(elapsed, static_cast<double>(kMaxSealedSeconds))
<< "run_streaming took " << elapsed
<< "s — child blocked on stdin → seal_stdin is broken or not applied";
}
Loading