Uh oh!
There was an error while loading. Please reload this page.
Fix UnixConsoleStream regression when stdin/stdout is redirected to a seekable file - #126844
Conversation
Tagging subscribers to this area: @dotnet/area-system-io |
RandomAccess.Read/Write use pread/pwrite syscalls which always operate at a specified fileOffset. Passing fileOffset:0 for seekable files (e.g., regular files redirected as stdin/stdout) caused reads/writes to always operate at position 0, resulting in infinite loops or incorrect output. The fix creates a FileStream (with bufferSize:0) wrapping the handle when the handle is seekable. FileStream uses OSFileStreamStrategy which tracks the file position and advances it after each read/write, ensuring correct sequential behavior for seekable files. For non-seekable files (pipes, terminals), the existing RandomAccess-based path is used, preserving cursor position tracking for terminals. Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/0df60225-7783-4496-abcf-9917387f56e5 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
…g duplication - Replace 'float for GC' FileStream approach with explicit _useFileStreamForIo bool field; FileStream is always held and disposed deterministically - Fix string duplication in stdout test by passing outputContent as RemoteExecutor arg - Add try/catch around File.Delete in finally blocks Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/0df60225-7783-4496-abcf-9917387f56e5 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
adamsitnik
left a comment
There was a problem hiding this comment.
@copilot please address my feedback
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…onsoleStream, simplify tests - Remove _useFileStreamForIo field and extra comments; always use FileStream for I/O - Change Write() in UnixConsoleStream to call WriteFromConsoleStream(_fileStream, buffer) - Update WriteFromConsoleStream in ConsolePal.Unix.cs and ConsolePal.Wasi.cs to accept FileStream instead of SafeFileHandle; use fs.Write(buffer) which handles both seekable and non-seekable files correctly - Rename SeekableStdinRedirection test to CanCopyStandardInputToStandardOutput and run on all non-mobile platforms (not just AnyUnix); simplify via RemoteInvokeOptions.StartInfo - Simplify SeekableStdoutRedirection test similarly; remove boilerplate try/catch Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/67a83dee-25ae-47d1-ae0f-b4aa1f18265f Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Done in adb2b69. All review feedback addressed:
|
adamsitnik
left a comment
There was a problem hiding this comment.
@copilot address my feedback
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Fix XML comment in ConsolePal.Wasi.cs Write(FileStream): fd → fs - Remove _handle field from UnixConsoleStream; FileStream owns handle lifetime - Remove Flush() override from UnixConsoleStream (base is a nop; bufferSize:0) - Remove redundant [PlatformSpecific] from two new tests (RemoteExecutor handles it) - Dispose stdoutHandle before WaitForExit so File.ReadAllText can open the file Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/ba3bd32e-023a-4160-bf27-f0ae05027d8d Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
adamsitnik
commented
Apr 19, 2026
@jozkee@jkotas@stephentoub I would appreciate your review, as I would like to start a longer leave knowing that I leave no known bugs opened. For context please refer to #126843 This PR is more or less a revert + tests that are going to ensure we are not going to introduce this bug again. |
There was a problem hiding this comment.
Pull request overview
This PR reverts recent System.Console Unix/WASI I/O changes that used RandomAccess.Read/Write back to the prior Interop.Sys.Read/Write implementation, and adds regression tests for redirected stdin/stdout to seekable files (to prevent hangs/incorrect output).
Changes:
- Restore
Interop.Sys.Read/Interop.Sys.Write-based console stream I/O on Unix and WASI (including the write loop with EPIPE/EAGAIN handling). - Add/enable regression tests covering seekable stdin and seekable stdout redirection scenarios.
- Restore needed interop source inclusions in
System.Console.csprojand includeConsoleHandles.csin the test project.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Console/tests/System.Console.Tests.csproj | Includes ConsoleHandles.cs in the test build. |
| src/libraries/System.Console/tests/ConsoleHandles.cs | Adds regression tests for seekable stdin/stdout redirection. |
| src/libraries/System.Console/src/System/ConsolePal.Wasi.cs | Restores Interop.Sys.Read/Write + write-loop behavior for WASI. |
| src/libraries/System.Console/src/System/ConsolePal.Unix.cs | Restores Interop.Sys.Read/Write + write-loop behavior and cursor-cache updates. |
| src/libraries/System.Console/src/System/ConsolePal.Unix.ConsoleStream.cs | Switches console stream reads back to ConsolePal.Read (Interop-based). |
| src/libraries/System.Console/src/System.Console.csproj | Re-adds interop source files (Poll/Read/Write) for Unix/WASI builds. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…ression Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/8c4bae7d-c5a8-4e7d-9567-c22ad8c8ff76 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Head branch was pushed to by a user without write access
Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/ee38cb6a-7c51-4290-b71e-c815838eb480 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
adamsitnik
commented
Apr 30, 2026
/backport to release/11.0-preview4 |
Started backporting to |
@adamsitnik backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patchCreating an empty commit: Initial planApplying: Fix UnixConsoleStream to handle seekable files correctlyUsing index info to reconstruct a base tree...M src/libraries/System.Console/src/System/ConsolePal.Unix.ConsoleStream.csM src/libraries/System.Console/tests/ConsoleHandles.csM src/libraries/System.Console/tests/System.Console.Tests.csprojFalling back to patching base and 3-way merge...Auto-merging src/libraries/System.Console/src/System/ConsolePal.Unix.ConsoleStream.csCONFLICT (content): Merge conflict in src/libraries/System.Console/src/System/ConsolePal.Unix.ConsoleStream.csAuto-merging src/libraries/System.Console/tests/ConsoleHandles.csCONFLICT (content): Merge conflict in src/libraries/System.Console/tests/ConsoleHandles.cserror: Failed to merge in the changes.hint: Use 'git am --show-current-patch=diff' to see the failed patchhint: When you have resolved this problem, run "git am --continue".hint: If you prefer to skip this patch, run "git am --skip" instead.hint: To restore the original branch and stop patching, run "git am --abort".hint: Disable this message with "git config set advice.mergeConflict false"Patch failed at 0002 Fix UnixConsoleStream to handle seekable files correctlyError: The process '/usr/bin/git' failed with exit code 128 |
adamsitnik
commented
Apr 30, 2026
I am stupid, it was merged before P4 snap so it's already there... |
Reverts all System.Console source changes from PR #125512 (which introduced
RandomAccess.Read/Writebased I/O) back to the originalInterop.Sys.Read/Writeimplementation. TheFileStream-based approach will be revisited separately.Adds regression tests that verify
Console.OpenStandardInput().CopyTo()andConsole.OpenStandardOutput().Write()work correctly when stdin/stdout is redirected to a seekable file.Description
ConsolePal.Unix.ConsoleStream.csto useConsolePal.Read()withInterop.Sys.ReadConsolePal.Unix.csto useInterop.Sys.Writewith the full write loop (EPIPE, EAGAIN handling) and pointer-basedUpdatedCachedCursorPositionConsolePal.Wasi.csto useInterop.Sys.Read/WritedirectlyInterop.Poll.cs,Interop.Read.cs,Interop.Write.csreferences inSystem.Console.csprojfor both WASI and Unix targetsFORCE_COLORcomment inConsolePal.Unix.csthat was accidentally reverted along with PR Relax RandomAccess type requirements: make Read/Write methods work with non-seekable files #125512 changesConsoleHandles.cstoSystem.Console.Tests.csproj(was previously missing)CanCopyStandardInputToStandardOutputtest — verifies reading from seekable stdin viaCopyToUnixConsoleStream_SeekableStdoutRedirection_WritesAllContenttest — verifies writing to seekable stdout using two separateWritecalls to ensure content is concatenated (not overwritten from offset 0), which properly covers the offset-based write regression