[release/8.0-staging] Fix #91958: use mach_timebase_info to determine process time coefficient on macOS#100122
Merged
jeffhandley merged 8 commits intodotnet:release/8.0-stagingfrom Apr 30, 2024
Conversation
…efficient on macOS
Contributor
|
Tagging subscribers to this area: @dotnet/area-system-diagnostics-process |
adamsitnik
approved these changes
Mar 22, 2024
Member
adamsitnik
left a comment
There was a problem hiding this comment.
LGTM, thank you for backporting the fix @jeffhandley !
jkotas
reviewed
Mar 24, 2024
src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.OSX.cs
Outdated
Show resolved
Hide resolved
Without this fix, it was possible for another thread to see an incorrect (zero) value of s_timeBase_numer because of a race condition.
Member
|
@jeffhandley should this be marked as servicing consider now that #100122 (comment) got resolved? If so, do we need to email tactics? |
Contributor
|
@jozkee I didn't find a Tactics email, so assuming this is ready (pending @jeffhandley 's or @adamsitnik 's confirmation), then yes, please send an email to Tactics and add the |
Member
Author
|
This was approved in email and needs an approval for merge. @jkotas -- you can ignore the re-review request as this can now be merged. |
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.
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.
Backport of #92185 to release/8.0-staging
Customer Impact
This has been reported by 2 customers. The first report was in September 2023 by @ForNeVeR in #91958, and they also provided the fix and tests. The issue was reported again by @huntharo in February with #98121. In that issue, @huntharo requested that this fix be backported to 8.0. As @huntharo stated:
Regression
While this isn't a product regression, it's behavior that became incorrect with Apple Silicon.
Testing
@ForNeVeR included a new test with their fix that is part of this backport. The test performs a native call and compares the API's results against the native results. As stated in the original PR:
Risk
Low. This is a targeted fix that only affects
PrivilegedProcessorTime,TotalProcessorTime, andUserProcessorTimeon macOS. While the fix had been in main since September, a race condition bug in the original fix was found during the review of this backport. That bug was fixed in main on March 25 and cherry-picked to this backport.The automatic backport of this fix failed due to a merge conflict in src/libraries/Common/src/Interop/OSX/Interop.Libraries.cs. That conflict arose because the preceding line to the change here was removed in 20a3350. Otherwise, the rest of the merge was clean.