Open Generics Registration Bug

Oct 31, 2013 at 3:29 PM
Yesterday, we just found what seems to be a problem with the implementation the container uses to traverse the registrations.

Let me try to explain our situation here before I delve into the problem. We had these so called CommandHandlers in a few places of our code, which basically follow the command pattern.
    public interface ICommand
    {
        Guid Id { get; }
    }

    public interface ICommandHandler<in TCommand>
        where TCommand : ICommand
    {
        void Handle(TCommand command);
    }
We have a bunch of implementations for both commands and handlers, and register them explicitly in the container.

A few days ago, we started experimenting with a ESB library, Mass Transit. They have the concept of a 'Consumer<T>', which needs to be implemented to support auto wiring of the pub/sub model.

To make our code independent of the MassTransit API, I then created a separate project, containing this adapter class:
    public sealed class CommandHandlerAdapter<TCommand> : Consumes<TCommand>.All
        where TCommand : class, ICommand
    {
        private readonly ICommandHandler<TCommand> _handler;

        public CommandHandlerAdapter(ICommandHandler<TCommand> handler)
        {
            _handler = handler;
        }

        void Consumes<TCommand>.All.Consume(TCommand message)
        {
            _handler.Handle(message);
        }
    }
The purpose here is to wrap one of our command handlers in a way that is compatible with the type required by MassTransit. It would be a matter of registering this new type in the container and all would work.

At that point, I remembered about registering open generic types, and that this would be a perfect fit for that. This single line would make all of our previous registrations automatically available to the ESB:
.RegisterType(typeof (Consumes<>.All), typeof (CommandHandlerAdapter<>))
To our surprise though, this throws an exception the accessing the 'Registrations' property on the container class. The method the ESB uses to populate the consumers, tries to iterate over the registrations on the container, causing the issue. This is the test result from one of the tests I created to showcase the problem:
Test Name:  UnityFailsOnGetRegistrationsForOpenGenerics
Test FullName:  UnityBug.ApplicationTest.UnitTest1.UnityFailsOnGetRegistrationsForOpenGenerics
Test Source:    c:\Users\julianogoncalves\Documents\Visual Studio 2013\Projects\UnityBug\UnityBug.ApplicationTest\UnitTest1.cs : line 26
Test Outcome:   Failed
Test Duration:  0:00:00.0011958

Result Message: 
Test method UnityBug.ApplicationTest.UnitTest1.UnityFailsOnGetRegistrationsForOpenGenerics threw exception: 
System.ArgumentException: GenericArguments[0], 'TMessage', on 'UnityBug.MassTransit.CommandHandlerAdapter`1[TCommand]' violates the constraint of type 'TCommand'. ---> System.TypeLoadException: GenericArguments[0], 'TMessage', on 'UnityBug.MassTransit.CommandHandlerAdapter`1[TCommand]' violates the constraint of type parameter 'TCommand'.
Result StackTrace:  
at System.RuntimeTypeHandle.Instantiate(RuntimeTypeHandle handle, IntPtr* pInst, Int32 numGenericArgs, ObjectHandleOnStack type)
   at System.RuntimeTypeHandle.Instantiate(Type[] inst)
   at System.RuntimeType.MakeGenericType(Type[] instantiation)
 --- End of inner exception stack trace ---
    at System.RuntimeType.ValidateGenericArguments(MemberInfo definition, RuntimeType[] genericArguments, Exception e)
   at System.RuntimeType.MakeGenericType(Type[] instantiation)
   at Microsoft.Practices.ObjectBuilder2.GenericTypeBuildKeyMappingPolicy.Map(NamedTypeBuildKey buildKey, IBuilderContext context)
   at Microsoft.Practices.Unity.ContainerRegistration.GetMappedType(IPolicyList policies)
   at Microsoft.Practices.Unity.ContainerRegistration..ctor(Type registeredType, String name, IPolicyList policies)
   at Microsoft.Practices.Unity.UnityContainer.<get_Registrations>b__16(Type type, String name)
   at System.Linq.Enumerable.<SelectManyIterator>d__31`3.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at UnityBug.ApplicationTest.UnitTest1.UnityFailsOnGetRegistrationsForOpenGenerics() in c:\Users\julianogoncalves\Documents\Visual Studio 2013\Projects\UnityBug\UnityBug.ApplicationTest\UnitTest1.cs:line 31

The test itself is quite short too:
        [TestMethod]
        public void UnityFailsOnGetRegistrationsForOpenGenerics()
        {
            var container = new UnityContainer()
                .RegisterType<ICommandHandler<TestCommand>, TestCommandHandler>()
                .RegisterType(typeof(Consumes<>.All), typeof(CommandHandlerAdapter<>));

            container.Registrations.ToList();
        }
After finding out that the problem was apparently in the container itself, I updated the unity nuget package to the latest version and, to my surprise, the problem was gone oO. Since the test project I created was .Net 4.5, I could use the latest version, but I can't do the same on our production code, since it is limited to .Net 4, which Unity 3 does not support.

I wonder if it would be somehow possible to correct this behavior on v2? By the stack trace, it seems to be a problem in the .Net framework itself, but perhaps it is avoidable via code with some preemptive tests. It is obviously related to the constraints on the types too, which we would also like to keep this way, since we don't want to modify our code (to remove the ICommand restriction on T) and need to conform to the MassTransit restriction of 'class'. I wonder if this is related to the changes in the reflection stuff from .Net4 to .Net4.5, since from what I've read unity 3 uses the new model to do it's magic.

I've created a small solution to demonstrate the problem, but I'm not sure how to share it in this post. If you need it for diagnostics purposes just send me an email or something so that I can send it.

Regards,
Juliano
Nov 1, 2013 at 6:10 AM
My first thought was also that the difference might be in the .NET Framework. But it also looks like the logic of GenericTypeBuildKeyMappingPolicy.Map was modified to use the new reflection API but the behavior is now slightly different. Here is the Unity 2 version that throws:
        public NamedTypeBuildKey Map(NamedTypeBuildKey buildKey, IBuilderContext context)
        {
            Guard.ArgumentNotNull(buildKey, "buildKey");
            Type originalType = buildKey.Type;
            GuardSameNumberOfGenericArguments(originalType);

            Type[] genericArguments = originalType.GetGenericArguments();
            Type resultType = destinationKey.Type.MakeGenericType(genericArguments);
            return new NamedTypeBuildKey(resultType, destinationKey.Name);
        }

Here is the new Unity 3 version that works:
        public NamedTypeBuildKey Map(NamedTypeBuildKey buildKey, IBuilderContext context)
        {
            Guard.ArgumentNotNull(buildKey, "buildKey");

            var originalTypeInfo = buildKey.Type.GetTypeInfo();
            if (originalTypeInfo.IsGenericTypeDefinition)
            {
                // No need to perform a mapping - the source type is an open generic
                return this.destinationKey;
            }

            this.GuardSameNumberOfGenericArguments(originalTypeInfo);
            Type[] genericArguments = originalTypeInfo.GenericTypeArguments;
            Type resultType = this.destinationKey.Type.MakeGenericType(genericArguments);
            return new NamedTypeBuildKey(resultType, this.destinationKey.Name);
        }

Besides the TypeInfo changes the new code has a short circuit for open generic types which seems to align with the behavior you are seeing. I would try implementing a similar change in the Unity 2 source to see if it helps. That said, I would worry a bit that the change might cause another issue in Unity 2 (although after a quick check I don't see anything).

In terms of formally getting a fix into Unity 2, I would say that it would be unlikely at this time -- you would probably have to modify the Unity source and go with a custom build.

~~
Randy Levy
entlib.support@live.com
Enterprise Library support engineer
Support How-to
Nov 5, 2013 at 6:24 PM
Edited Nov 5, 2013 at 6:24 PM
Oh... so it really is a bug in the policy causing this issue...

Isn't there a way to override just this policy, using a container extension? If I remember correctly, I could just remove the default one and add the custom, fixed one, through code.

I'm not willing to maintain the whole source code just for this, since it is very localized. Unfortunately, I also cannot upgrade to Unity 3 because of the .Net framework 4.5 restrictions (many of our clients still use Windows Xp and Windows Server 2003).

About the update, why do you think it is unlikely that a fix will go live? Is it that complicated to correct this problem? I suppose it's just a matter of adding the open generics check there right?

By the way, thanks a lot again as always Randy, you are the man ;)

Regards,
Juliano
Nov 7, 2013 at 5:11 AM
Thanks for the kind words, Juliano.
Isn't there a way to override just this policy, using a container extension? If I remember correctly, I could just remove the default one and add the custom, fixed one, through code.

You're right. Unity is very flexible and you can do most anything in a container extension. So you can put the fix in and then create a container extension to apply it.
public class CustomPolicyOverrideBehaviorExtension : UnityContainerExtension
{
    /// <summary>
    /// Install the default container behavior into the container.
    /// </summary>
    protected override void Initialize()
    {
        Context.Registering += OnRegister;
    }

    public override void Remove()
    {
        Context.Registering -= OnRegister;
    }

    private void OnRegister(object sender, RegisterEventArgs e)
    {
        if (e.TypeFrom != null)
        {
            if (e.TypeFrom.IsGenericTypeDefinition && e.TypeTo.IsGenericTypeDefinition)
            {
                // Override the existing policy with custom implementation
                Context.Policies.Set<IBuildKeyMappingPolicy>(
                    new CustomGenericTypeBuildKeyMappingPolicy(new NamedTypeBuildKey(e.TypeTo, e.Name)),
                    new NamedTypeBuildKey(e.TypeFrom, e.Name));
            }
        }
    }
}

public class CustomGenericTypeBuildKeyMappingPolicy : IBuildKeyMappingPolicy
{
    private readonly NamedTypeBuildKey destinationKey;

    /// <summary>
    /// Create a new <see cref="CustomGenericTypeBuildKeyMappingPolicy"/> instance
    /// that will map generic types.
    /// </summary>
    /// <param name="destinationKey">Build key to map to. This must be or contain an open generic type.</param>
    // FxCop suppression: Validation is done by Guard class
    [SuppressMessage("Microsoft.Design", "CA1062:ValidateArgumentsOfPublicMethods")]
    public CustomGenericTypeBuildKeyMappingPolicy(NamedTypeBuildKey destinationKey)
    {
        Guard.ArgumentNotNull(destinationKey, "destinationKey");
        if (!destinationKey.Type.IsGenericTypeDefinition)
        {
            throw new ArgumentException(
                string.Format("Resources.MustHaveOpenGenericType: {0}",
                                destinationKey.Type.Name));
        }
        this.destinationKey = destinationKey;
    }

    /// <summary>
    /// Maps the build key.
    /// </summary>
    /// <param name="buildKey">The build key to map.</param>
    /// <param name="context">Current build context. Used for contextual information
    /// if writing a more sophisticated mapping.</param>
    /// <returns>The new build key.</returns>
    public NamedTypeBuildKey Map(NamedTypeBuildKey buildKey, IBuilderContext context)
    {
        Guard.ArgumentNotNull(buildKey, "buildKey");

        var originalType = buildKey.Type;
        if (originalType.IsGenericTypeDefinition)
        {
            // No need to perform a mapping - the source type is an open generic
            return this.destinationKey;
        }

        this.GuardSameNumberOfGenericArguments(originalType);
        Type[] genericArguments = originalType.GetGenericArguments();
        Type resultType = this.destinationKey.Type.MakeGenericType(genericArguments);
        return new NamedTypeBuildKey(resultType, this.destinationKey.Name);
    }

    private void GuardSameNumberOfGenericArguments(Type sourceType)
    {
        if (sourceType.GetGenericArguments().Length != this.DestinationType.GetGenericArguments().Length)
        {
            throw new ArgumentException(
                string.Format("Resources.MustHaveSameNumberOfGenericArguments.  Source Type: {0}, Destination Type: {1}",
                                sourceType.Name, this.DestinationType.Name),
                "sourceTypeInfo");
        }
    }

    private Type DestinationType
    {
        get { return destinationKey.Type; }
    }
}

And the code which should pass your test:
IUnityContainer container = new UnityContainer();
container.AddExtension(new CustomPolicyOverrideBehaviorExtension());

container.RegisterType<ICommandHandler<TestCommand>, TestCommandHandler>();
container.RegisterType(typeof(Consumes<>), typeof(CommandHandlerAdapter<>));

container.Registrations.ToList();

~~
Randy Levy
entlib.support@live.com
Enterprise Library support engineer
Support How-to