Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Handle EAGAIN in Console.Write on Unix #23539

Merged
merged 1 commit into from
Aug 30, 2017

Conversation

stephentoub
Copy link
Member

@stephentoub stephentoub commented Aug 25, 2017

If stdout/stderr is configured as a non-blocking file descriptor, Console.Write{Line} may fail if the descriptor is full and would block. With this commit, we instead poll in that case waiting for the ability to write and then retry.

(This should workaround the issue seen in dotnet/msbuild#2153, though it'd be good to understand why stdout is being set as O_NONBLOCK in that scenario, who's doing it, etc.)

cc: @joperezr, @wfurt, @CESARDELATORRE

If stdout/stderr is configured as a non-blocking file descriptor, Console.Write{Line} may fail if the descriptor is full and would block.  With this commit, we instead poll in that case waiting for the ability to write and then retry.
@stephentoub stephentoub changed the title Handle EAGAIN in Console.Write Handle EAGAIN in Console.Write on Unix Aug 25, 2017
@stephentoub
Copy link
Member Author

@ianhays, @Priya91, @joperezr, @wfurt, could one or more of you help out with a review of this?

@wfurt
Copy link
Member

wfurt commented Aug 29, 2017

It looks good to me @stephentoub . Out of curiosity: what would be behavior of the new test if you comment out your fix.

@stephentoub
Copy link
Member Author

It looks good to me

Thanks

what would be behavior of the new test if you comment out your fix.

There's a race condition involved, so it won't fail 100% of the time, but in most executions it'll fail with an exception that stems from this line translating the received EAGAIN/EWOULDBLOCK into an exception:

throw Interop.GetExceptionForIoErrno(errorInfo);

@wfurt
Copy link
Member

wfurt commented Aug 29, 2017

I think that is good enough. I was wondering is stdout is hijacked by xunits in this case.
The non-blocking flag would be set for all remaining tests in given assembly, right?

@stephentoub
Copy link
Member Author

The non-blocking flag would be set for all remaining tests in given assembly, right?

No, the test is using our RemoteInvoke helper to run the relevant code in a separate process:
https://github.com/dotnet/corefx/pull/23539/files#diff-3f6d4aa2105614ae7f5328fbfa355be6R17
so the setting on the file descriptor is only for that process' stdout.

@wfurt
Copy link
Member

wfurt commented Aug 29, 2017

great. thanks for clarification @stephentoub That is good trick to know.

@stephentoub stephentoub merged commit e84604e into dotnet:master Aug 30, 2017
@stephentoub stephentoub deleted the stdout_eagain branch August 30, 2017 13:31
stephentoub added a commit to stephentoub/corefx that referenced this pull request Aug 31, 2017
If stdout/stderr is configured as a non-blocking file descriptor, Console.Write{Line} may fail if the descriptor is full and would block.  With this commit, we instead poll in that case waiting for the ability to write and then retry.
Copy link
Member

@joperezr joperezr left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for fixing this Steve.

@karelz karelz modified the milestone: 2.1.0 Sep 6, 2017
weshaggard added a commit that referenced this pull request Sep 8, 2017
release/2.0: Handle EAGAIN in Console.Write (#23539)
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
If stdout/stderr is configured as a non-blocking file descriptor, Console.Write{Line} may fail if the descriptor is full and would block.  With this commit, we instead poll in that case waiting for the ability to write and then retry.

Commit migrated from dotnet/corefx@e84604e
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants