Uh oh!
There was an error while loading. Please reload this page.
Add OpenStandardInputHandle, OpenStandardOutputHandle, and OpenStandardErrorHandle APIs - #123478
Conversation
…rdErrorHandle APIs Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
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.
…ify tests 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.
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.
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
adamsitnik
left a comment
There was a problem hiding this comment.
I wrapped my head around error handling. Long story short, we need to keep throwing the same exception in all the places that were using Interop.Sys.FileDescriptors so far.
Before the introduction of the new public API, we were using Interop.CheckIo(Interop.Sys.Dup(FD)).
Dup would fail with EBADF if FD was invalid, so is fcntl. We need to return -1 in SystemNative_FcntlCheckAccess when fcntl fails.
When the access rights are not as requested, the SystemNative_FcntlCheckAccess should also set errno to EBADF and return -1. To mimic read/write sys-calls.
So SystemNative_FcntlCheckAccess should not return bool, but failure (invalid fd, wrong access) or success.
The OpenStandardInputHandle, OpenStandardOutputHandle and OpenStandardErrorHandle form ConsolePal.Unix.cs should accept an optional boolean parameter bool verifyFd: true. And When the call to SystemNative_FcntlCheckAcces fails, they should just call Interop.CheckIo when verifyFd is true.
When verifyFd is false, the method should just return raw handle value. Something like this:
publicstaticSafeFileHandleOpenStandardInputHandle(boolverifyFd=true)=>OpenStandardHandle(0,FileAccess.Read,verifyFd);publicstaticSafeFileHandleOpenStandardOutputHandle(boolverifyFd=true)=>OpenStandardHandle(1,FileAccess.Write,verifyFd);publicstaticSafeFileHandleOpenStandardErrorHandle(boolverifyFd=true)=>OpenStandardHandle(2,FileAccess.Write,verifyFd);privatestaticSafeFileHandleOpenStandardHandle(IntPtrfd,FileAccessaccess,boolverifyFd){if(verifyFd){Interop.CheckIo(Interop.Sys.Fcntl.CheckAccess(fd,(int)access));}returnnewSafeFileHandle(fd,ownsHandle:false);}Now, all the places that were using Interop.Sys.FileDescriptors.STDIN_FILENO and similar directly like here, should pass verifyFd: false to ensure they don't suddenly start throwing an exception:
public static bool IsInputRedirectedCore()
{
- return IsHandleRedirected(Interop.Sys.FileDescriptors.STDIN_FILENO);+ return IsHandleRedirected(OpenStandardInputHandle(verifyFd: false));
}This should allow to not introduce any breaking changes into existing public API
Uh oh!
There was an error while loading. Please reload this page.
adamsitnik
commented
Jan 22, 2026
@copilot please address my feedback from #123478 (review) (I forgot to tag you) |
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
…orms Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
jkotas
commented
Feb 3, 2026
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jkotas
left a comment
There was a problem hiding this comment.
LGTM!
Please check that the new tests pass in runtime-extra-platforms pipeline before merging.
jkotas
commented
Feb 4, 2026
/ba-g none of the failures in extra platforms look related to this change. Mostly infrastructure issues. |
Uh oh!
There was an error while loading. Please reload this page.
…rdErrorHandle APIs (dotnet#123478) Fixesdotnet#122802 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com> Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
…rdErrorHandle APIs (dotnet#123478) Fixesdotnet#122802 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com> Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.