Multithreaded resolution throws ArgumentException or NullReferenceException

[alexmg]

From [email protected] on June 08, 2012 19:06:50

What steps will reproduce the problem? 1. Create two containers in the same process with dependencies that use constructor injection.
2. Resolve a dependency with constructor injection concurrently (multiple threads).
3. Boom! Either NullReferenceException or ArgumentException. If it doesn't fail at first, keep trying (a bigger loop ;-)) What is the expected output? What do you see instead? Duh. No exception, of course. What version of Autofac are you using? On what version of .NET/Silverlight? Autofac 2.6.2.859, .NET 4.0 Please provide any additional information below. Granted, multiple container instances inside the same process space might be somewhat of an anti-pattern. I just happen to be co-hosting two otherwise isolated services inside the same process space. I would expect two containers inside the same process space (appdomain to be more correct) to be isolated. Statics are evil. The exception callstack origin is observed in Autofac.Core.Activators.Reflection.ConstructorParameterBinding.Instantiate. The culprit is the static _contructorInvokers dictionary that walks into the multi-threaded minefield. I'd suspect concurrent resolutions on a single container to produce the same behavior.

I'd provide you with a patch, but it's unclear to me if the source up here is the latest nor what branch/version I should be working on. The quick'n'dirty solution is turning it into a ConcurrentDictionary for .NET 4.0. The long term solution seems a more explicit form of container local caching. Also, could the caching not be done any earlier (with associated trade-offs)?

Original issue: http://code.google.com/p/autofac/issues/detail?id=373

[alexmg]

From [email protected] on June 09, 2012 03:15:52

I'd like to contribute towards fixing this, but then I'd need the code that corresponds to 2.6.2.859. Can someone point me in the right direction?

[alexmg]

From alex.meyergleaves on June 09, 2012 03:36:07

Thanks but this has already been fixed. The first cut of the code actually used a ConcurrentDictionary but was changed during performance profiling and after reviewing locking at the container level. It did not occur to me that two containers would be used within the same AppDomain.

Status: Fixed
Owner: alex.meyergleaves

[alexmg]

From [email protected] on June 09, 2012 05:31:25

Upon reflection keeping it as static seems wrong to me. It should be scoped to the container it belongs to. Especially since it's part of a "product" that is so concerned with lifecycle management, an example should be set. It may sound like nitpicking, and yes the issue is resolved, but I think this is an important design issue that deserves consideration. Feel free to ignore this comment. Just an observation that does not diminish your hard work which is very much appreciated.

[alexmg]

From alex.meyergleaves on June 09, 2012 05:43:31

Your observation is certainly valid and I agree that this would be better. I intend to do more work in this area but will probably leave that for the 3.0 branch. It is simply a case of trying to keep the changes in this branch to a minimum.