Skip to content

Conversation

@epeshk
Copy link
Contributor

@epeshk epeshk commented Feb 3, 2022

@ghost ghost added area-System.Diagnostics.Process community-contribution Indicates that the PR has been added by a community member labels Feb 3, 2022
@ghost
Copy link

ghost commented Feb 3, 2022

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

Issue Details

#63937

Author: epeshk
Assignees: -
Labels:

area-System.Diagnostics.Process

Milestone: -

Copy link
Member

Choose a reason for hiding this comment

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

@adamsitnik, do you know why we fall back to the IndexOf here for the !IsRemoteMachine case? I'm wondering why the using block below can't just contain return !processHandle.IsInvalid && !HasExited(processHandle, ref signaled, out _);.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to continue working on this code to get rid of this fallback in most of cases. This fallback needed because it's not always possible to open process handle, e.g. corner cases is Idle process (pid = 0) and csrss processes. We probably should fall back only if processId == 0 or if error code of OpenProcess is ERROR_ACCESS_DENIED but it may be a controversial topic, so I decide to implement it in another PR, separated from bugfix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stephentoub
Copy link
Member

Thanks for working on this. Can you please add a test that would have failed before and will now succeed?

@epeshk
Copy link
Contributor Author

epeshk commented Feb 15, 2022

I've added test for this

@jozkee jozkee merged commit f97919d into dotnet:main Feb 24, 2022
@danmoseley
Copy link
Member

Thanks for the contribution @epeshk

@ghost ghost locked as resolved and limited conversation to collaborators Mar 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Diagnostics.Process 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.

4 participants