-
Notifications
You must be signed in to change notification settings - Fork 485
Static AttributesToAvoidReplicating is not Thread Safe #334
Description
We rarely get the following exception (Castle.Core 4.2.1):
System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
at System.Collections.Generic.List1.Enumerator.MoveNextRare() at System.Collections.Generic.List1.Enumerator.MoveNext()
at System.Linq.Enumerable.Any[TSource](IEnumerable1 source, Func2 predicate)
at Castle.DynamicProxy.Generators.AttributesToAvoidReplicating.ShouldAvoid(Type attribute)
at Castle.DynamicProxy.Internal.AttributeUtil.ShouldSkipAttributeReplication(Type attribute)
at Castle.DynamicProxy.Internal.AttributeUtil.d__5.MoveNext()
at Castle.DynamicProxy.Contributors.CompositeTypeContributor.ImplementMethod(MetaMethod method, ClassEmitter class, ProxyGenerationOptions options, OverrideMethodDelegate overrideMethod)
at Castle.DynamicProxy.Contributors.CompositeTypeContributor.Generate(ClassEmitter class, ProxyGenerationOptions options)
at Castle.DynamicProxy.Generators.InterfaceProxyWithoutTargetGenerator.GenerateType(String typeName, Type proxyTargetType, Type[] interfaces, INamingScope namingScope)
at Castle.DynamicProxy.Generators.InterfaceProxyWithTargetGenerator.<>c__DisplayClass6_0.b__0(String n, INamingScope s)
at Castle.DynamicProxy.Generators.BaseProxyGenerator.ObtainProxyType(CacheKey cacheKey, Func`3 factory)
at Castle.DynamicProxy.Generators.InterfaceProxyWithTargetGenerator.GenerateCode(Type proxyTargetType, Type[] interfaces, ProxyGenerationOptions options)
at Castle.DynamicProxy.DefaultProxyBuilder.CreateInterfaceProxyTypeWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options)
at Castle.DynamicProxy.ProxyGenerator.CreateInterfaceProxyWithoutTarget(Type interfaceToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options, IInterceptor[] interceptors)
Now, the obvious error case would be a concurrent call to AttributesToAvoidReplicating.Add<T>() or AttributesToAvoidReplicating.Add(Type attribute).
As it happens we're also using MOQ which is in fact modifying this collection.
Given that our use of MOQ happens concurrent to our own code creating proxies it's, IMHO, quite clear that this can occasionaly cause the above exception.
Even though MOQ and our own code don't share the same ProxyGenerator instance, as AttributesToAvoidReplicating is static it's being shared.
The only way (workaround) to synchronize access in this case is to cause MOQ to initialize before use Castle.Core in our own code. Of course, that's quite the design smell.
Two alternative suggestions for fixing the issue:
- make
AttributesToAvoidReplicatingpart of a specificProxyGeneratorInstance. Means attributes need to be supplied when creating it. Also means;- breaking change
AttributesToAvoidReplicatingconfiguration not leaking to other users of DynamicProxy- don't know how this will affect the cleanliness of the implementation itself, as the information would have to be passed around since it can't be static anymore...
- make
AttributesToAvoidReplicatingthread safe- suggestion: use
Interlocked.CompareExchangeinAddto replace the backing collection. - performance overhead should be negliglible as
Addis only called a few times...
- suggestion: use