Skip to content

Fix UnixConsoleStream when stdin/stdout is redirected to a seekable file#126844

Draft
Copilot wants to merge 6 commits intomainfrom
copilot/fix-unix-console-stream-bug
Draft

Fix UnixConsoleStream when stdin/stdout is redirected to a seekable file#126844
Copilot wants to merge 6 commits intomainfrom
copilot/fix-unix-console-stream-bug

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 13, 2026

Description

UnixConsoleStream used RandomAccess.Read/Write with fileOffset: 0, which internally calls pread/pwrite — syscalls that operate at a fixed offset and never advance the file position. For seekable handles (regular files), this means every read restarts at byte 0, causing infinite loops on CopyTo, and every write overwrites from the beginning.

Root Cause

RandomAccess.ReadAtOffset uses pread when handle.SupportsRandomAccess is true (i.e., the handle is seekable). Passing fileOffset: 0 always reads the same bytes. Same issue applies to writes.

Fix

  • In UnixConsoleStream, wrap the handle in a FileStream(handle, access, bufferSize: 0) and use it unconditionally for all reads and writes. FileStream correctly handles both seekable files (tracking and advancing _filePosition) and non-seekable files (pipes, terminals), so no conditional branching is needed. FileStream also owns the handle lifetime, so no separate _handle field is required.
  • Write() in UnixConsoleStream delegates to ConsolePal.WriteFromConsoleStream(_fileStream, buffer).
  • WriteFromConsoleStream in ConsolePal.Unix.cs and ConsolePal.Wasi.cs updated to accept FileStream instead of SafeFileHandle, calling fs.Write(buffer) which handles both seekable and non-seekable cases correctly, while preserving EPIPE handling and terminal cursor-position tracking.
  • The Flush() override has been removed from UnixConsoleStream — the base implementation is a no-op and bufferSize: 0 means no buffering.
  • The Write(SafeFileHandle fd, ...) overload in ConsolePal.Unix.cs (which used RandomAccess.Write(fd, buffer, fileOffset: 0)) has been removed entirely. WriteToTerminal now uses a cached s_terminalFileStream (wrapping s_terminalHandle) for the default terminal, and a temporary non-owning FileStream for explicit handle calls, both going through Write(FileStream, ...).
// Before: always pread/pwrite at offset 0 for seekable files → infinite loop / corrupt output
RandomAccess.Read(_handle, buffer, fileOffset: 0);

// After: FileStream handles both seekable and non-seekable files correctly
_fileStream.Read(buffer);

Tests

Added two regression tests to ConsoleHandles.cs (run on all platforms supported by RemoteExecutor, including Windows):

  • CanCopyStandardInputToStandardOutput — verifies stdin redirected to a seekable file is fully consumed (process exits, no infinite loop).
  • UnixConsoleStream_SeekableStdoutRedirection_WritesAllContent — verifies stdout redirected to a seekable file contains the complete expected output. The parent's copy of stdoutHandle is disposed before reading the file to ensure File.ReadAllText can open it on all platforms.

ConsoleHandles.cs was not previously included in System.Console.Tests.csproj — this is now fixed.

Changes

  • src/libraries/System.Console/src/System/ConsolePal.Unix.ConsoleStream.cs — core fix: always use FileStream for all I/O; removed _handle field and Flush() override
  • src/libraries/System.Console/src/System/ConsolePal.Unix.csWriteFromConsoleStream updated to accept FileStream; Write(SafeFileHandle, ...) removed; WriteToTerminal updated to use cached s_terminalFileStream; new s_terminalFileStream field added
  • src/libraries/System.Console/src/System/ConsolePal.Wasi.csWriteFromConsoleStream updated to accept FileStream; fixed XML doc comment
  • src/libraries/System.Console/tests/ConsoleHandles.cs — regression tests
  • src/libraries/System.Console/tests/System.Console.Tests.csproj — include ConsoleHandles.cs

Copilot AI requested review from Copilot and removed request for Copilot April 13, 2026 17:27
Copilot AI linked an issue Apr 13, 2026 that may be closed by this pull request
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

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>
Copilot AI requested review from Copilot and removed request for Copilot April 13, 2026 18:15
…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>
Copilot AI requested review from Copilot and removed request for Copilot April 13, 2026 18:17
Copilot AI changed the title [WIP] Fix bug in UnixConsoleStream when working with seekable files Fix UnixConsoleStream when stdin/stdout is redirected to a seekable file Apr 13, 2026
Copilot AI requested a review from adamsitnik April 13, 2026 18:20
Copy link
Copy Markdown
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.

@copilot please address my feedback

Comment on lines +97 to +102
const string inputContent = "Hello from seekable stdin!";
string testFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
try
{
File.WriteAllText(testFilePath, inputContent, Encoding.UTF8);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
const string inputContent = "Hello from seekable stdin!";
string testFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
try
{
File.WriteAllText(testFilePath, inputContent, Encoding.UTF8);
const string inputContent = "Hello from seekable stdin!";
string testFilePath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
File.WriteAllText(testFilePath, inputContent, Encoding.UTF8);
try
{

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in adb2b69.

…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>
Copilot AI requested review from Copilot and removed request for Copilot April 14, 2026 06:53
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 14, 2026

@copilot please address my feedback

Done in adb2b69. All review feedback addressed:

  • Removed _useFileStreamForIo and extra comments; FileStream is always used for all I/O (handles both seekable and non-seekable correctly)
  • Write() in UnixConsoleStream now calls WriteFromConsoleStream(_fileStream, buffer)
  • WriteFromConsoleStream in both ConsolePal.Unix.cs and ConsolePal.Wasi.cs updated to accept FileStream and call fs.Write(buffer)
  • Tests renamed/simplified and now run on all non-mobile platforms

Copilot AI requested a review from adamsitnik April 14, 2026 06:56
Copy link
Copy Markdown
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.

@copilot address my feedback

- 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>
Copilot AI requested review from Copilot and removed request for Copilot April 14, 2026 08:27
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 14, 2026

@copilot address my feedback

Done in e5c1365. All feedback addressed:

  • Fixed XML comment fdfs in ConsolePal.Wasi.cs (resolves build failure)
  • Removed _handle field; FileStream now fully owns the handle lifetime
  • Removed Flush() override (base is a nop; bufferSize: 0 means no buffering)
  • Removed redundant [PlatformSpecific] from both new tests
  • Added stdoutHandle.Dispose() before WaitForExit to allow File.ReadAllText to open the file

Copilot AI requested a review from adamsitnik April 14, 2026 08:30
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot this method needs to be removed, as calling RandomAccess.Write(fd, buffer, fileOffset: 0); for seekable files will introduce a bug we are trying to fix

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 49df75d. Write(SafeFileHandle, ...) is removed. WriteToTerminal now uses Write(FileStream, ...) — via a cached s_terminalFileStream for the default terminal handle, and a temporary FileStream (wrapping a freshly-obtained non-owning handle) for the WriteTerminalAnsiColorString path.

…l writes

- Add s_terminalFileStream cached field alongside s_terminalHandle
- Initialize s_terminalFileStream in EnsureInitializedCore when s_terminalHandle is set
- Update WriteToTerminal to use Write(FileStream,...) — cached s_terminalFileStream
  for the default terminal, temporary FileStream for explicit non-owning handles
- Remove Write(SafeFileHandle, ...) which used RandomAccess.Write(fd, offset:0),
  a buggy pattern for seekable files (now fully eliminated from the codebase)

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/6de74f54-eefb-49ea-88ce-3ffa1c3628e8

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot April 14, 2026 09:40
Copilot AI requested a review from adamsitnik April 14, 2026 09:43
else
{
// handle is always a freshly-obtained non-owning handle; safe to wrap in a FileStream and dispose.
using FileStream fs = new FileStream(handle, FileAccess.Write, bufferSize: 0);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is going to close the handle..

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.

Bug in UnixConsoleStream when working with seekable files

2 participants