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

Thread safety issue: race condition in MockDefaultValueProvider #205

Closed
mizbrodin opened this issue Sep 28, 2015 · 0 comments
Closed

Thread safety issue: race condition in MockDefaultValueProvider #205

mizbrodin opened this issue Sep 28, 2015 · 0 comments

Comments

@mizbrodin
Copy link
Contributor

There is a thread safety issue when using Mock instances in a multi-threaded environment.

Here is the exception that I get occasionally when using an instance of a generic Mock class for my IProcessingKey interface (created using the default Mock constructor) and trying to get a value of Id property which is defined in my IProcessingKey interface, which is of a reference type and which has not been explicitly setup (default value is expected to be used):

System.ArgumentException: An item with the same key has already been added.: at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
at Moq.MockDefaultValueProvider.ProvideDefault(MethodInfo member)
at Moq.HandleMockRecursion.HandleIntercept(ICallContext invocation, InterceptorContext ctx, CurrentInterceptContext localctx)
at Moq.Interceptor.Intercept(ICallContext invocation)
at Castle.DynamicProxy.AbstractInvocation.Proceed()
at Castle.Proxies.IProcessingKeyProxy.get_Id()
at [ … skipping my own production code … ]

Moq version used: 4.2.1507.118 (obtained from NuGet)

The problem seems to be caused by a race condition in ProvideDefault method of MockDefaultValueProvider class where Mock.InnerMocks dictionary is used in a not thread-safe manner. When two different threads call that method concurrently for the same instance of MockDefaultValueProvider, there is a good chance that Mock.InnerMocks.TryGetValue would return false for both of them and they both would try to add the same entry twice to the dictionary (which would cause an exception like the one shown above).

A solution for this problem could be using a lock covering the operations performed on the dictionary (TryGetValue and Add) or using System.Collections.Concurrent.ConcurrentDictionary class as the type for Mock.InnerMocks propety (instead of the common Dictionary class) and use it’s GetOrAdd method for thread-safe checking and updating of the dictionary (instead of calling separate methods TryGetValue and Add).

mizbrodin added a commit to mizbrodin/moq4 that referenced this issue Oct 11, 2015
…ockDefaultValueProvider)

Fixed issue devlooped#205 with the replacement of Dictionary type with
ConcurrentDictionary for InnerMocks property and using its GetOrAdd
method in MockDefaultValueProvider for making it thread-safe.
kzu added a commit that referenced this issue Feb 11, 2016
Fix for issue #205 (Thread safety issue: race condition in MockDefaultValueProvider)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant