From b5f25085fa3b658eb63f142bdbca558dd7709ce4 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Sat, 23 May 2026 21:58:20 +0800 Subject: [PATCH 1/2] fix: seal child-process stdin on Windows (first-run hang) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mcpp's first-run flow on Windows was hanging at xlings / xim / curl / git grandchildren that block on terminal stdin, forcing users to press Enter repeatedly to advance bootstrap and toolchain install. Root cause: process::seal_stdin was a no-op on Windows, and install_with_progress's direct-install path deliberately bypassed it. The POSIX side has had /dev/null 2>&1) and did not touch stdin. Changes: - process.cppm: seal_stdin now appends " +#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"; +} From 46b1addb9a7b25e981cfa2b6624aa984050af3bb Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Sat, 23 May 2026 22:12:00 +0800 Subject: [PATCH 2/2] =?UTF-8?q?ci(windows):=20convert=20MCPP=5FSELF=20MSYS?= =?UTF-8?q?=20path=20=E2=86=92=20Windows=20path;=20rename=20pwsh=20$Args?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues with the regression step from the previous commit (both showed up only on the actual Windows runner, not in local validation): 1. MCPP_SELF was set in an earlier bash step via `pwd` (git-bash) so the value is MSYS-style (e.g. /d/a/mcpp/...). Bash steps tolerate it but pwsh's `&` operator can't exec it ("not recognized as a name of a cmdlet, function, script file, or executable program"). Convert via cygpath -w before use. 2. `$Args` is a PowerShell automatic variable inside function scope; a `param([string]$Args)` does not bind cleanly. Renamed to $McppArgs to avoid the collision (also updated call sites). --- .github/workflows/ci-windows.yml | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci-windows.yml b/.github/workflows/ci-windows.yml index 4636342..c74b862 100644 --- a/.github/workflows/ci-windows.yml +++ b/.github/workflows/ci-windows.yml @@ -103,18 +103,28 @@ jobs: 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 - & $env:MCPP_SELF new hello_stdin + & $mcppExe new hello_stdin Set-Location hello_stdin function Invoke-McppWithOpenStdin { - param([string]$Args, [int]$TimeoutSeconds = 300) + param([string]$McppPath, [string]$McppArgs, [int]$TimeoutSeconds = 300) $psi = [System.Diagnostics.ProcessStartInfo]::new() - $psi.FileName = $env:MCPP_SELF - $psi.Arguments = $Args + $psi.FileName = $McppPath + $psi.Arguments = $McppArgs $psi.WorkingDirectory = (Get-Location).Path $psi.UseShellExecute = $false $psi.RedirectStandardInput = $true # parent holds child's stdin @@ -142,7 +152,7 @@ jobs: Write-Host $stdoutTask.Result Write-Host "----- captured stderr -----" Write-Host $stderrTask.Result - throw "REGRESSION: 'mcpp $Args' HUNG with open-empty stdin after ${TimeoutSeconds}s. The Windows seal_stdin fix is not effective." + throw "REGRESSION: 'mcpp $McppArgs' HUNG with open-empty stdin after ${TimeoutSeconds}s. The Windows seal_stdin fix is not effective." } Write-Host "----- stdout -----" @@ -151,18 +161,18 @@ jobs: Write-Host $stderrTask.Result if ($p.ExitCode -ne 0) { - throw "'mcpp $Args' exited with code $($p.ExitCode) (no hang, but failed)." + throw "'mcpp $McppArgs' exited with code $($p.ExitCode) (no hang, but failed)." } } Write-Host '=== T1: mcpp --version (sanity, fast path) ===' - Invoke-McppWithOpenStdin -Args '--version' -TimeoutSeconds 30 + Invoke-McppWithOpenStdin -McppPath $mcppExe -McppArgs '--version' -TimeoutSeconds 30 Write-Host '=== T2: mcpp build (full bootstrap + toolchain + dep resolve + compile) ===' - Invoke-McppWithOpenStdin -Args 'build' -TimeoutSeconds 600 + Invoke-McppWithOpenStdin -McppPath $mcppExe -McppArgs 'build' -TimeoutSeconds 600 Write-Host '=== T3: mcpp run (post-build run path) ===' - Invoke-McppWithOpenStdin -Args 'run' -TimeoutSeconds 120 + Invoke-McppWithOpenStdin -McppPath $mcppExe -McppArgs 'run' -TimeoutSeconds 120 Write-Host 'SUCCESS: mcpp completes with open-empty stdin → Windows seal_stdin fix verified.'