Skip to content

Process: use overlapped I/O for parent end of stdout/stderr pipes on Windows#125643

Merged
adamsitnik merged 7 commits intomainfrom
copilot/extend-process-createpipe-with-asyncreads
Mar 18, 2026
Merged

Process: use overlapped I/O for parent end of stdout/stderr pipes on Windows#125643
adamsitnik merged 7 commits intomainfrom
copilot/extend-process-createpipe-with-asyncreads

Conversation

Copy link
Contributor

Copilot AI commented Mar 17, 2026

Anonymous pipes used for stdout/stderr redirection were always created synchronous on Windows, causing a blocked thread-pool thread per pipe read. This change opens the parent's end of stdout/stderr pipes with overlapped (async) I/O unconditionally, while keeping the child's end synchronous so that child processes using GetStdHandle() continue to work correctly.

Description

  • CreatePipe simplified: the bool parentInputs parameter directly drives asyncRead: !parentInputs — stdout/stderr (parentInputs=false) unconditionally get an overlapped read handle; stdin (parentInputs=true) remains fully synchronous. No separate asyncReads parameter is needed.
  • stdin always uses sync I/O — no change in behavior for the write side.
  • stdout/stderr always use overlapped I/O on the parent's read end.
  • FileStream construction updated to use handle.IsAsync instead of hardcoded false, so the stream's async dispatch mode matches the handle's actual overlapped mode.
  • AppContainer support: the named pipe used for async I/O now uses the \\.\pipe\LOCAL\ prefix, which is required for Windows AppContainers (Windows 10 v1709+). Without this prefix, pipe creation fails in UWP/AppContainer processes.
  • Named pipe buffer size increased to 16 KB (4 × 4096) for better I/O throughput compared to the 4 KB default of CreatePipe.
  • defaultTimeout set to 120 seconds to match the actual internal default used by CreatePipe (verified via Windows source code).
  • Comments updated to clarify that only the parent end of the pipe is opened for overlapped I/O, the child end remains synchronous (it is a duplicate of the write handle opened without FILE_FLAG_OVERLAPPED), and why the LOCAL namespace is used.
  • New test (Start_Redirect_StandardHandles_UseRightAsyncMode) verifies that the parent's stdout/stderr handles are async on Windows and that the child's standard handles are always synchronous.
// stdin: always synchronous (parentInputs=true → asyncRead=false)
CreatePipe(out parentInputPipeHandle, out childInputPipeHandle, true);

// stdout/stderr: always async/overlapped on the parent read end (parentInputs=false → asyncRead=true)
CreatePipe(out parentOutputPipeHandle, out childOutputPipeHandle, false);
CreatePipe(out parentErrorPipeHandle, out childErrorPipeHandle, false);

The Unix part does not need this fix because it uses System.IO.Pipes backed by a socket, which enables non-blocking I/O with the first call to any Async method.

Benchmark Results

Throughput roughly doubles when many processes with redirected stdout are run concurrently:

Total time (before):  00:00:19.7916593
Total time (after):   00:00:09.1419538

Micro-benchmark results: EgorBot/Benchmarks#44 (comment)

Note: ReadOutputToEnd (synchronous read on an async handle) is slightly slower due to overlapped I/O bookkeeping overhead; ReadOutputLineByLine and ReadOutputToEndAsync show no regression.


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot AI and others added 2 commits March 17, 2026 09:21
…/stderr pipes

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modulo comments, let's wait for CI results and get some benchmark numbers.

Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>
@adamsitnik

This comment was marked as resolved.

- we have to specify inBufferSize, otherwise the first write always blocks until reading starts
- set the same timeout as CreatePipe (120s)
@adamsitnik

This comment was marked as resolved.

@adamsitnik
Copy link
Member

adamsitnik commented Mar 17, 2026

For a following app:

Stopwatch stopwatch = Stopwatch.StartNew();

Parallel.For(0, 1_000, static (_, _) =>
{
    using (Process process = new())
    {
        process.StartInfo = Benchmarks.s_outputStartInfo;
        process.OutputDataReceived += Benchmarks.s_ignoreOutputLine;
        process.Start();
        process.BeginOutputReadLine();
        process.WaitForExit();
        process.OutputDataReceived -= Benchmarks.s_ignoreOutputLine;
    }
});

Console.WriteLine($"Total time:  {stopwatch.Elapsed}");

The throughput doubles:

Total time (before):  00:00:19.7916593
Total time (after):   00:00:09.1419538

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Enables overlapped (async) I/O for the parent’s read end of redirected stdout/stderr pipes on Windows to avoid consuming a thread-pool thread per pipe for blocked reads, while keeping the child’s std handles synchronous for compatibility with GetStdHandle() usage.

Changes:

  • Add an opt-out switch (DOTNET_SYSTEM_DIAGNOSTICS_PROCESS_DISABLE_ASYNC_READS) to disable overlapped reads for redirected stdout/stderr.
  • Extend Process.CreatePipe to control async behavior and route stdout/stderr through SafeFileHandle.CreateAnonymousPipe(..., asyncRead: true).
  • Adjust the named-pipe creation parameters used by SafeFileHandle.CreateAnonymousPipe when async is requested (buffer sizing / timeout arguments).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs Tweaks named-pipe handle creation parameters used when creating async-capable anonymous pipes on Windows.
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs Adds opt-out env var and uses async/overlapped reads for the parent side of redirected stdout/stderr pipes.

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All Windows CI legs are green (also tested locally), the PR is ready for review.

@stephentoub @jkotas this is what it would take to use async IO to read from STD OUT/ERR (we don't need to set anything else thanks to #123761).

The micro benchmark results are available here. There are no regressions for ReadOutputLineByLine/ReadOutputToEndAsync, however ReadOutputToEnd is slightly slower. I've added new benchmarks to the performance repo as well (dotnet/performance#5152).

The main benefit is improved throughput (we are no longer blocking up to two thread pool threads per process with redirected std out and err). This is going to impact various tools that spawn multiple processes like test runners.

More context: #81896

Since we are in early preview I believe it's worth considering making this change (with an ability to disable it via env var).

The Unix part does not need this fix, because it's using System.IO.Pipes backed by a socket which enables the non-blocking IO with first call to any Async method.

@adamsitnik adamsitnik requested a review from stephentoub March 17, 2026 22:20
@adamsitnik
Copy link
Member

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

@adamsitnik
Copy link
Member

/azp list

@azure-pipelines

This comment was marked as resolved.

@adamsitnik
Copy link
Member

/azp run runtime-libraries-coreclr outerloop-windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@adamsitnik

This comment was marked as resolved.

This comment was marked as outdated.

@adamsitnik
Copy link
Member

CreatePipe internally calls CreateNamedPipeW with nDefaultTimeOut = 0 (NMPWAIT_USE_DEFAULT_WAIT, ~50 ms system default), not 120,000 ms. The claim "same as the default for CreatePipe" is factually wrong.

This is not true. I've verified that with Windows source code (any MSFT reviewers please reach out to me offline to get a link). Since Windows source code is closed source I can't post a screen shot here..

@adamsitnik
Copy link
Member

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

@adamsitnik
Copy link
Member

/azp run runtime-libraries-coreclr outerloop-windows

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor Author

Copilot AI commented Mar 18, 2026

CreatePipe internally calls CreateNamedPipeW with nDefaultTimeOut = 0 (NMPWAIT_USE_DEFAULT_WAIT, ~50 ms system default), not 120,000 ms. The claim "same as the default for CreatePipe" is fac...

Acknowledged — I was wrong about that. I'll correct the review accordingly.

@copilot please invoke the code-review skill and post your analysis/results as a comment on this PR

🤖 Copilot Code Review

Multi-model review (Claude Sonnet 4.6 + Gemini 2.5 Pro + GPT-4.1)


Holistic Assessment

Motivation: Synchronous anonymous pipes force a blocked thread-pool thread per redirected stdout/stderr stream. Benchmark evidence (2× throughput, ~10s vs ~20s for 1,000 concurrent processes) confirms the real-world impact.

Approach: Using overlapped I/O exclusively for the parent's read end of stdout/stderr pipes is the correct Windows mechanism. The child-side handles are kept synchronous by duplicating the write end (which was opened without FILE_FLAG_OVERLAPPED) via DuplicateHandle. The AppContainer fix (\\.\pipe\LOCAL\) and the 16 KB buffer are well-motivated and correctly implemented.

Summary: ✅ LGTM. No blocking issues. All three models independently found no errors or correctness concerns. A few minor cosmetic suggestions below.


Detailed Findings

✅ Correctness: child handles remain synchronous

For stdout/stderr (parentInputs=false), asyncRead: !parentInputs = true → the read handle is opened with FILE_FLAG_OVERLAPPED. The child's handle (hTmpChild) is the write end, opened with asyncWrite: false (no FILE_FLAG_OVERLAPPED). DuplicateHandle with DUPLICATE_SAME_ACCESS creates another handle referencing the same file object, which retains the non-overlapped attribute. Child standard handles are therefore correctly synchronous. ✔

✅ API surface

SafeFileHandle.CreateAnonymousPipe(..., bool asyncRead = false, bool asyncWrite = false) is already reflected in src/libraries/System.Runtime/ref/System.Runtime.cs. The public API contract is in place. ✔

✅ AppContainer support

\\.\pipe\LOCAL\ is correctly cited from the MSDN docs and is required for named pipes in AppContainer/UWP processes on Windows 10 v1709+. ✔

defaultTimeout — 120 seconds is correct

The comment // same as the default for CreatePipe is accurate. The author has confirmed via Windows source code that CreatePipe internally uses 120 seconds as the default timeout. (A previous review comment I made incorrectly claimed it was ~50ms; I was wrong and this has been corrected.)

✅ Test coverage

Start_Redirect_StandardHandles_UseRightAsyncMode covers both sides:

  • Child (via RemoteExecutor): all three standard handles are synchronous
  • Parent: stdin is synchronous; stdout/stderr are async on Windows, sync on Unix via Assert.Equal(OperatingSystem.IsWindows(), ...)

The GetSafeFileHandle helper handles both FileStream (Windows) and AnonymousPipeClientStream (Unix) paths. ✔

⚠️ ReadOutputToEnd regression — worth a note in the commit or PR

The microbenchmarks show ReadOutputToEnd is slightly slower with an async FileStream. This is expected: synchronous ReadToEnd() on an overlapped handle incurs extra bookkeeping (IOCP completion tracking). The tradeoff is acknowledged and the overall throughput gain dominates, but it might be worth a single comment near the FileStream construction so future readers don't investigate this as an unintentional regression.

💡 Comment on child handle synchronicity

Both Gemini and GPT-4.1 suggested this: the block comment above CreatePipe explains what happens (child is synchronous) but not how it achieves it. Adding a note like "the child's handle is a duplicate of the write end, which was opened without FILE_FLAG_OVERLAPPED" would make the mechanism self-evident.

💡 Buffer size constant (minor)

inBufferSize: 4 * 4096 is clear enough with its inline comment, but a named constant (e.g., DefaultAsyncPipeBufferSize = 4 * 4096) would allow reuse if this value is referenced elsewhere. Low-priority since it currently only appears once.

@adamsitnik
Copy link
Member

@stephentoub @jkotas I've fixed the only edge case I was able to find (using a specific name for named pipes in container apps), the CI is green and the PR is ready for review. PTAL

@adamsitnik adamsitnik merged commit 4687f9a into main Mar 18, 2026
146 of 154 checks passed
@adamsitnik adamsitnik deleted the copilot/extend-process-createpipe-with-asyncreads branch March 18, 2026 20:53
SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle readHandle, out SafeFileHandle writeHandle);
// Only the parent's read end benefits from async I/O; stdin is always sync.
// asyncRead applies to the read handle; asyncWrite to the write handle.
SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle readHandle, out SafeFileHandle writeHandle, asyncRead: !parentInputs, asyncWrite: false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In few weeks from now I will be on parental leave. If this PR introduced a bug, the only thing that needs to be reverted is following line:

Suggested change
SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle readHandle, out SafeFileHandle writeHandle, asyncRead: !parentInputs, asyncWrite: false);
SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle readHandle, out SafeFileHandle writeHandle, asyncRead: false, asyncWrite: false);

And an update for the introduced test. The other changes in this PR are bug fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants