diff --git a/.github/workflows/ci-windows.yml b/.github/workflows/ci-windows.yml index 9c50b17..c74b862 100644 --- a/.github/workflows/ci-windows.yml +++ b/.github/workflows/ci-windows.yml @@ -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: | diff --git a/src/platform/process.cppm b/src/platform/process.cppm index 23323e5..e258f36 100644 --- a/src/platform/process.cppm +++ b/src/platform/process.cppm @@ -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 @@ -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: " +#include + +#if defined(_WIN32) +#include +#include +#include +#else +#include +#include +#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(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 +double time_seconds(F&& fn) { + auto t0 = std::chrono::steady_clock::now(); + fn(); + auto t1 = std::chrono::steady_clock::now(); + return std::chrono::duration(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(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(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(kMaxSealedSeconds)) + << "run_streaming took " << elapsed + << "s — child blocked on stdin → seal_stdin is broken or not applied"; +}