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/7.0] Fix non-determinism in Regex source generator #78149

Merged
merged 2 commits into from
Nov 11, 2022

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 10, 2022

Backport of #78103 to release/7.0

/cc @stephentoub

Customer Impact

Non-deterministic build output, such that compiling the same inputs multiple times may result in different bits in the resulting binaries. The source generator has a Hashtable built up by the Regex implementation, and that Hashtable is then enumerated to write its contents into the C# source being generated. That Hashtable has string keys. In .NET Core, string hash codes are randomized, such that a string's hash code potentially changes from process to process. And Hashtable's enumeration order is influenced by the hash codes of its keys, as it enumerates its internal buckets and the hash codes impact which bucket a string is slotted into. The fix is to sort as part of enumerating the Hashtable so we always get a consistent, reproducible order.

Testing

New tests + existing tests run locally and in CI.

Risk

Low. It's just using OrderBy to sort the data being foreach'd.

The source generator enumerates a Hashtable to write out its contents.  When the keys of the Hashtable are strings, string hash code randomization may result in the order of that enumeration being different in different processes, leading to non-deterministic ordering of values written out and thus non-deterministic source generator output.
@ghost
Copy link

ghost commented Nov 10, 2022

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

Issue Details

Backport of #78103 to release/7.0

/cc @stephentoub

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Text.RegularExpressions

Milestone: -

@joperezr joperezr added the Servicing-consider Issue for next servicing release review label Nov 10, 2022
@carlossanlop
Copy link
Member

Signed off by area owners and approved by Tactics.
No OOB package authoring changes needed for the S.T.RegularExpressions csproj.
CI failure is known and unrelated: #77988
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit abecfbe into release/7.0 Nov 11, 2022
@carlossanlop carlossanlop deleted the backport/pr-78103-to-release/7.0 branch November 11, 2022 02:57
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 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