fix: shared mutable state in test infrastructure causes race conditions - #4928
Conversation
549226f to
7dff290CompareCode ReviewThe thread-safety fixes in this PR are correct and well-targeted. The logic for each change is sound:
Architectural Observation: |
7dff290 to
b49e5aaCompareSeveral infrastructure classes used non-thread-safe collections (HashSet, Dictionary) for static shared state that can be accessed from multiple threads during parallel test execution: - GenericTestRegistry.AotCompatibleMethods: HashSet<MethodInfo> -> ConcurrentDictionary - GenericTestRegistry._registeredCombinations: add lock around inner HashSet mutations and return snapshots from GetRegisteredCombinations - DataSourceHelpers.TypeCreators: Dictionary -> ConcurrentDictionary - TUnitInitializer.ParseParameters: use GetOrAdd to eliminate TOCTOU race
9447a8f to
6ec07c4CompareAdd Defaults class, SetRetryBackoff method, BackoffMs/BackoffMultiplier properties on RetryAttribute, RetryBackoffMs/RetryBackoffMultiplier on TestDetails and ITestConfiguration - matching the DotNet8_0 snapshot but without DynamicallyAccessedMembers annotations.
Uh oh!
There was an error while loading. Please reload this page.
Summary
HashSet<MethodInfo>withConcurrentDictionary<MethodInfo, byte>inGenericTestRegistry.AotCompatibleMethodsto prevent concurrentAdd/Containsrace conditionsHashSet<Type[]>mutations insideGenericTestRegistry._registeredCombinationsand return snapshots fromGetRegisteredCombinationsto prevent concurrent modification during enumerationDictionary<Type, Func<...>>withConcurrentDictionaryinDataSourceHelpers.TypeCreatorsto prevent concurrent read/write race conditions from generated module initializersConcurrentDictionary.GetOrAddinTUnitInitializer.ParseParametersto eliminate TOCTOU (time-of-check-time-of-use) race on parameter list creationTest plan
dotnet build TUnit.Core/TUnit.Core.csprojpasses (all 4 TFMs: netstandard2.0, net8.0, net9.0, net10.0)dotnet build TUnit.Engine/TUnit.Engine.csprojpasses (all 4 TFMs)Fixes#4875