SafeProcessHandle.Unix: fix missing DangerousRelease (port of #37412)#37446
Merged
danmoseley merged 1 commit intodotnet:release/5.0-preview6from Jun 5, 2020
Merged
SafeProcessHandle.Unix: fix missing DangerousRelease (port of #37412)#37446danmoseley merged 1 commit intodotnet:release/5.0-preview6from
danmoseley merged 1 commit intodotnet:release/5.0-preview6from
Conversation
Because the SafeProcessHandle was not owned, ReleaseHandle was not called, causing the wrapped SafeWaitHandle to never release its resources.
|
Tagging subscribers to this area: @eiriktsarpalis |
stephentoub
approved these changes
Jun 4, 2020
mmitche
added
Servicing-approved
Member
Author
|
@mmitche can I merge, or will you? |
Member
Either way works for me. Whoever gets to it first. |
Member
Author
|
I don't think we need to wait on iOS tests here, as we're not shipping any iOS targeting inaries in this preview. Plus I don't think Process.Start will work on iOS anyway. |
Member
Author
|
@mmitche merged |
ghost
locked as resolved and limited conversation to collaborators
freddyrios
added a commit
to copenhagenatomics/CA_DataUploader
that referenced
this pull request
Feb 17, 2021
reason: the root cause of the last rpi temp slow downs / crashes, is a leak on dotnet core. i.e. specifically we want this fix dotnet/runtime#37446 we could have gone with .net core 3.1.7, but we were already planning to move to .net 5 anyway dotnet/corefx#42941
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Ports #37412 to preview 6. Goal is to get extra coverage in advance of potentially servicing 3.1. Original issue #36661
Fixes regression introduced in this PR between 2.2 and 3.0
Description from original PR
Because the SafeProcessHandle was not owned, ReleaseHandle was not called, causing the wrapped SafeWaitHandle to never release its resources.
Customer Impact
From customer:
Our goal is to get coverage in 5.0 so we can confidently port to 3.1 and customer can move back to 3.1
Risk
Low. The change is limited to Unix; also, the lifetime was now too short, it is likely that existing tests would be crashing. Also the change is very localized and relatively straightforward. This change has not been in master long. The impact if this change was somehow in error would likely be crashes launching processes in some cases on Unix. In such a hypothetical case we would advise impacted customers to stay on preview 5 pending preview 7, but it would have helped us avoid putting a bad change in 3.1.