Skip to content

Conversation

@tarekgh
Copy link
Member

@tarekgh tarekgh commented Feb 18, 2022

Fixes #65502

Backport of #65414 to release/6.0

/cc @tarekgh

Customer Impact

This is a migration to .NET 6.0 blocker for users who enable Globalization Invariant Mode on their app and try to use the performance counter library.
Performance counter library has old code which creating CultureInfo object using LCID. LCID is an obsolete concept, and it is recommended to always create a culture with the language tag names instead. When enabling the Globalization Invariant Mode, we don't support creating cultures using LCID at all as we don't depend on the underlying OS at all. This will cause the code in the performance counter library to throw an exception. That means there is no way to use this library when enabling the invariant mode.

Testing

This change is already merged in .NET 7.0 and passing all regression and CI tests. Also, I added a new test for testing the library when enabling the Globalization Invariant Mode.

Risk

Low as the change scoped to the performance counter library which is supported on Windows only and all the changes is changing the creation of the culture using the LCID to use the culture name instead which should be exact result anyway but will avoid the exception.

@ghost
Copy link

ghost commented Feb 18, 2022

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti, @safern
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: tarekgh
Assignees: tarekgh
Labels:

area-System.Diagnostics.Tracing

Milestone: -

@tarekgh tarekgh changed the title [dotnet/runtime] Backport Fix PerformanceCounter's when running with Globalization Invariant Mode (PR #65414) [release/6.0] Backport Fix PerformanceCounter's when running with Globalization Invariant Mode Feb 18, 2022
@tarekgh tarekgh requested a review from carlossanlop February 18, 2022 01:29
@ghost
Copy link

ghost commented Feb 18, 2022

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

Issue Details

Backport of #65414 to release/6.0

/cc @tarekgh

Customer Impact

This is a migration to .NET 6.0 blocker for users who enable Globalization Invariant Mode on their app and try to use the performance counter library.
Performance counter library has old code which creating CultureInfo object using LCID. LCID is an obsolete concept, and it is recommended to always create a culture with the language tag names instead. When enabling the Globalization Invariant Mode, we don't support creating cultures using LCID at all as we don't depend on the underlying OS at all. This will cause the code in the performance counter library to throw an exception. That means there is no way to use this library when enabling the invariant mode.

Testing

This change is already merged in .NET 7.0 and passing all regression and CI tests. Also, I added a new test for testing the library when enabling the Globalization Invariant Mode.

Risk

The low as the change scoped to the performance counter library which is supported on Windows only and all the changes is changing the creation of the culture using the LCID to use the culture name instead which should be exact result anyway but will avoid the exception.

Author: tarekgh
Assignees: tarekgh
Labels:

area-System.Diagnostics.PerformanceCounter

Milestone: -

@tarekgh tarekgh added this to the 6.0.x milestone Feb 18, 2022
@tarekgh tarekgh added the Servicing-consider Issue for next servicing release review label Feb 18, 2022
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Feb 22, 2022
@rbhanda rbhanda modified the milestones: 6.0.x, 6.0.4 Feb 22, 2022
@carlossanlop
Copy link
Contributor

carlossanlop commented Mar 8, 2022

@tarekgh just noticed this change affects an OOB package. Per the servicing instructions, if the csproj has the <IsPackable>true</IsPackable> property (which it does), then you also need to set the <GeneratePackageOnBuild>true</GeneratePackageOnBuild> property.

You also need to add the <ServicingVersion>1</ServicingVersion> property, since it's the first time this package gets serviced. (If it had already existed, you would just have to bump the version number to the next integer).

Here are a couple of examples (we had to add the property on the next release because we forgot): #65523 #65733

Can you please add the change?

@tarekgh
Copy link
Member Author

tarekgh commented Mar 9, 2022

@carlossanlop @ericstj could you please have a look at the last commit in this PR? Thanks!

Copy link
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

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

LGTM

@ericstj
Copy link
Member

ericstj commented Mar 10, 2022

Failure is fixed by #66366

@ericstj ericstj merged commit d4ba2cc into dotnet:release/6.0 Mar 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants