Skip to content

Fix retry loop around Process.Kill#102962

Merged
danmoseley merged 1 commit into
dotnet:mainfrom
jkotas:issue-93321-2
Jun 2, 2024
Merged

Fix retry loop around Process.Kill#102962
danmoseley merged 1 commit into
dotnet:mainfrom
jkotas:issue-93321-2

Conversation

@jkotas
Copy link
Copy Markdown
Member

@jkotas jkotas commented Jun 2, 2024

We do not need to be retrying after Process.Kill succeeds

We do not need to be retrying after Process.Kill succeeds
@ghost ghost added the area-Infrastructure-coreclr Only use for closed issues label Jun 2, 2024
@jkotas jkotas requested a review from danmoseley June 2, 2024 02:49
@jkotas
Copy link
Copy Markdown
Member Author

jkotas commented Jun 2, 2024

Missed a change in #102958

@skyoxZ
Copy link
Copy Markdown
Contributor

skyoxZ commented Jun 2, 2024

Should it throw if all tries fail?

Copy link
Copy Markdown
Member

@danmoseley danmoseley left a comment

Choose a reason for hiding this comment

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

Oh wow, who code reviewed that!

@danmoseley danmoseley merged commit 7cc9156 into dotnet:main Jun 2, 2024
@jkotas jkotas deleted the issue-93321-2 branch June 2, 2024 14:29
@jkotas
Copy link
Copy Markdown
Member Author

jkotas commented Jun 2, 2024

Should it throw if all tries fail?

The tests should still work fine if the process that we tried to kill does not get actually killed in rare situations. It will increase memory load on the test machine.

This workaround should not be needed in the first place. It is working around bugs in Process.Kill implementation.

@skyoxZ
Copy link
Copy Markdown
Contributor

skyoxZ commented Jun 2, 2024

The tests should still work fine if the process that we tried to kill does not get actually killed in rare situations.

But how could we know it's a rare situation if it doesn't throw? Maybe all these 4 retries will 100% fail.

@jkotas
Copy link
Copy Markdown
Member Author

jkotas commented Jun 3, 2024

Ok, I have made the change to propagate the exception from the last retry: #102974 . It is going to tell us whether the failure is persistent once it occurs that is a useful data point.

@github-actions github-actions Bot locked and limited conversation to collaborators Jul 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Infrastructure-coreclr Only use for closed issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants