search

luboshl / MiniValidationPlus

Validation library based on System.ComponentModel.DataAnnotations with non-nullable reference types support.
MIT License
0 stars 0 forks source link

Fix accessing NonNullablePropertyHelper from multiple threads #4

Closed luboshl closed 2 months ago

luboshl commented 2 months ago

NullabilityInfoContext is not thread-safe (see i.e. https://github.com/dotnet/runtime/issues/100254). So NonNullablePropertyHelper was changed from static to instance class. For performance reasons instance is not created for every call to method IsNonNullableReferenceType(property) but it is created once in TypeDetailsCache class.

Previous solution with static class sometimes caused tests to fail because they run in parallel (see https://github.com/luboshl/MiniValidationPlus/actions/runs/9961358339/job/27522843648#step:7:42). The same could happen in application code.

Error Message:
   System.ArgumentException : An item with the same key has already been added. Key: MiniValidationPlus.UnitTests.TestChildType
  Stack Trace:
     at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.Reflection.NullabilityInfoContext.GetNullableContext(MemberInfo memberInfo)
   at System.Reflection.NullabilityInfoContext.GetNullabilityInfo(MemberInfo memberInfo, Type type, NullableAttributeStateParser parser, Int32& index)
   at System.Reflection.NullabilityInfoContext.Create(PropertyInfo propertyInfo)
   at MiniValidationPlus.NonNullablePropertyHelper.IsNonNullableReferenceType(PropertyInfo propertyInfo) in /_/src/MiniValidationPlus/NonNullablePropertyHelper.cs:line 26
   at MiniValidationPlus.TypeDetailsCache.Visit(Type type, HashSet`1 visited, Boolean& requiresAsync) in /_/src/MiniValidationPlus/TypeDetailsCache.cs:line 90
   at MiniValidationPlus.TypeDetailsCache.Visit(Type type, HashSet`1 visited, Boolean& requiresAsync) in /_/src/MiniValidationPlus/TypeDetailsCache.cs:line 126
   at MiniValidationPlus.TypeDetailsCache.Visit(Type type) in /_/src/MiniValidationPlus/TypeDetailsCache.cs:line 38
   at MiniValidationPlus.TypeDetailsCache.Get(Type type) in /_/src/MiniValidationPlus/TypeDetailsCache.cs:line 28
   at MiniValidationPlus.MiniValidator.RequiresValidation(Type targetType, Boolean recurse) in /_/src/MiniValidationPlus/MiniValidator.cs:line [44](https://github.com/luboshl/MiniValidationPlus/actions/runs/9961358339/job/27522843648#step:7:45)
   at MiniValidationPlus.MiniValidator.TryValidateImpl[TTarget](TTarget target, IServiceProvider serviceProvider, Boolean recurse, Boolean allowAsync, IDictionary`2& errors) in /_/src/MiniValidationPlus/MiniValidator.cs:line 167
   at MiniValidationPlus.MiniValidator.TryValidate[TTarget](TTarget target, IDictionary`2& errors) in /_/src/MiniValidationPlus/MiniValidator.cs:line 60
   at MiniValidationPlus.UnitTests.TryValidate.NonRequiredValidator_Invalid_When_Invalid() in /_/tests/MiniValidationPlus.UnitTests/TryValidate.cs:line 58
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
luboshl commented 2 months ago

Solution was not good. TypeDetailsCache is instance class but is used as static field in MiniValidator. Thus there was still only one instance and the problem with parallel access from multiple threads sometimes happened. Creation of instance of NonNullablePropertyHelper was moved to the Visit method before properties iteration in 07f4d60cb361c08d2ddfec7e88d9c26dfd5c500d.