GetMock<T> is not thread-safe

[ian-g-holm-intel]

https://github.com/moq/Moq.AutoMocker/blob/master/Moq.AutoMock/AutoMocker.cs

GetMockImplementation is referencing the dictionary TypeMap, which is not a ThreadSafe collection (should be ConcurrentDictionary). This means if you call GetMock in parallel on multiple threads, you can end up with an exception similar to this.

System.InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. The collection's state is no longer correct.
at System.Collections.Generic.Dictionary2.TryInsert(TKey key, TValue value, InsertionBehavior behavior) at System.Collections.Generic.Dictionary2.set_Item(TKey key, TValue value)
at Moq.AutoMock.AutoMocker.GetMockImplementation(Type serviceType, Boolean enablePrivate)
at Moq.AutoMock.AutoMocker.GetMock(Type serviceType, Boolean enablePrivate)
at Moq.AutoMock.AutoMocker.GetMock(Type serviceType)
at Moq.AutoMock.AutoMocker.GetMockTService

[Keboo]

HI @retik
Thanks for the bug report. I would like to understand your use-case where you are accessing the AutoMocker from multiple threads. So far, the AutoMocker class has never included thread safety (the internal Resolvers collection is another example where multiple threads mutating it can be problematic). Can you provide more information on how you are using it?

[ian-g-holm-intel]

Ah I see, didn't realize it wasn't thread-safe. We use it for some simulation implementations of classes, not specifically for unit testing. We bootstrap an instance of AutoMocker in our primary container (Microsoft DI) and then register mocks for various low-level interfaces. Then we can access the mocker in our regression test code and setup mocks to simulate the low-level APIs. I think the problem we ran into was due to us registering using something like this:

serviceCollection.AddTransient(sp => mocker.GetMock<TService>().Object);

The problem this caused was that the GetMock<> isn't called until it is first resolved. So if multiple threads resolve at the same time, GetMock<> gets called in parallel. We seem to be able to work around this by changing it to a singleton that instantiates the mock first.

serviceCollection.AddSingleton(mocker.GetMock<TService>().Object);

So I think we're good for now. I was just assuming that AutoMocker was intended to be threadsafe and would benefit from this. If it's a bigger task to make the whole library thread-safe then it's probably not worth fixing just this one thing.

Thanks.

[Keboo]

Thanks for the explanation, that is certainly helpful. It is certainly not documented, and I think it is worth considering if we should make it thread-safe or at least document it.

[ian-g-holm-intel]

I would highly recommend it since AutoMocker is, at least in my mind, treated much in the same way as an IOC container. And being thread-safe is certainly a requirement of other IOC containers, which is why I think I assumed AutoMocker was as well.