Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix setting pipe used in PAL_ProbeMemory as nonblocking #61126

Merged
merged 1 commit into from
Nov 26, 2021

Conversation

vtikoo
Copy link
Contributor

@vtikoo vtikoo commented Nov 2, 2021

Signed-off-by: Vikas Tikoo vikasamar@gmail.com

Signed-off-by: Vikas Tikoo <vikasamar@gmail.com>
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Nov 2, 2021
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.

@vtikoo have you hit any particular problem or just found a possible issue when analyzing the source code?

LGTM, thank you!

Comment on lines +753 to +754
flags = fcntl(fds[0], F_GETFL, 0);
fcntl(fds[0], F_SETFL, flags | O_NONBLOCK);
Copy link
Member

Choose a reason for hiding this comment

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

an alternative would be using pipe2 but this would require an #ifdef

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do some targets not support pipe2?

Copy link
Member

Choose a reason for hiding this comment

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

pipe2 is Linux-specific and we support macOS and few other Unix-like OSese. The current solution is good, I am going to merge the PR.

@vtikoo
Copy link
Contributor Author

vtikoo commented Nov 25, 2021

@vtikoo have you hit any particular problem or just found a possible issue when analyzing the source code?

I hit this when running against a library OS our team is working on - Mystikos.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-PAL-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants