Skip to content

Static AttributesToAvoidReplicating is not Thread Safe #334

@BrunoJuchli

Description

@BrunoJuchli

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 AttributesToAvoidReplicating part of a specific ProxyGenerator Instance. Means attributes need to be supplied when creating it. Also means;
    • breaking change
    • AttributesToAvoidReplicating configuration 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 AttributesToAvoidReplicating thread safe
    • suggestion: use Interlocked.CompareExchange in Add to replace the backing collection.
    • performance overhead should be negliglible as Add is only called a few times...

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions