Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/8.0] Fix Options Source Gen RangeAttribute Thread Safety #97110

Merged
merged 2 commits into from Jan 17, 2024

Conversation

tarekgh
Copy link
Member

@tarekgh tarekgh commented Jan 17, 2024

Fixes #97037

Backport of #97045 to release/8.0

/cc @tarekgh

Customer Impact

Multi-threaded applications or libraries utilizing the Options validation source generator and employing the RangeAttribute validation attribute multiple times with identical parameters may encounter race conditions, leading to exceptions being thrown. This issue holds significance as certain extension libraries, such as the Http Resilient library, exhibit this problematic pattern, and we already have customers reporting issues with such libraries.

Testing

I have included additional test cases to cover the specific scenario of concern, ensuring that the code passes all regression tests. This change has been manually tested too.

Risk

Low, the actual change here is ensuring the initialization code of the RangeAttribute execute inside a lock to ensure thread safety.

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jan 17, 2024
@ghost ghost assigned tarekgh Jan 17, 2024
@tarekgh tarekgh added Servicing-consider Issue for next servicing release review area-Extensions-Options source-generator Indicates an issue with a source generator feature and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jan 17, 2024
@tarekgh tarekgh added this to the 8.0.x milestone Jan 17, 2024
@ghost
Copy link

ghost commented Jan 17, 2024

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

Issue Details

Fixes #97037

Backport of #97045 to release/8.0

/cc @tarekgh

Customer Impact

Applications or libraries utilizing the Options validation source generator and employing the RangeAttribute validation attribute multiple times with identical parameters may encounter race conditions, leading to exceptions being thrown. This issue holds significance as certain extension libraries, such as the Http Resilient library, exhibit this problematic pattern, and we already have customers reporting issues with such libraries.

Testing

I have included additional test cases to cover the specific scenario of concern, ensuring that the code passes all regression tests. This change has been manually tested too.

Risk

Low, the actual change here is ensuring the initialization code of the RangeAttribute execute inside a lock to ensure thread safety.

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: tarekgh
Assignees: tarekgh
Labels:

Servicing-consider, area-Extensions-Options, source-generator

Milestone: -

@tarekgh
Copy link
Member Author

tarekgh commented Jan 17, 2024

CC @artl93 @ericstj

@carlossanlop carlossanlop added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 17, 2024
<GeneratePackageOnBuild>false</GeneratePackageOnBuild>
<ServicingVersion>1</ServicingVersion>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<ServicingVersion>2</ServicingVersion>
Copy link
Member

Choose a reason for hiding this comment

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

LGTM 👌

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

@tarekgh let me know when the relevant CI legs have finished executing and they look good. This branch is restricted so I need to merge it.

@carlossanlop carlossanlop modified the milestones: 8.0.x, 8.0.2 Jan 17, 2024
@carlossanlop carlossanlop merged commit c987e4f into dotnet:release/8.0 Jan 17, 2024
105 of 110 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Options Servicing-approved Approved for servicing release source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants