Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is ConstructorInfo.GetParameters Thread-Safe?

This is a very strange problem that I have spent the day trying to track down. I am not sure if this is a bug, but it would be great to get some perspective and thoughts on why this is happening.

I am using xUnit (2.0) to run my unit tests. The beauty of xUnit is that it automatically runs tests in parallel for you. The problem that I have found, however, is that Constructor.GetParameters appears to not be thread-safe when the ConstructorInfo is marked as being a thread-safe type. That is, if two threads reach Constructor.GetParameters at the same time, two results are produced, and subsequent calls to this method returns the second result that was created (regardless of the thread that calls it).

I have created some code to demonstrate this unexpected behavior (I also have it hosted on GitHub if you would like to download and try the project locally).

Here is the code:

public class OneClass
{
    readonly ITestOutputHelper output;

    public OneClass( ITestOutputHelper output )
    {
        this.output = output;
    }

    [Fact]
    public void OutputHashCode()
    {
        Support.Add( typeof(SampleObject).GetTypeInfo() );
        output.WriteLine( "Initialized:" );
        Support.Output( output );

        Support.Add( typeof(SampleObject).GetTypeInfo() );
        output.WriteLine( "After Initialized:" );
        Support.Output( output );
    }
}

public class AnotherClass
{
    readonly ITestOutputHelper output;

    public AnotherClass( ITestOutputHelper output )
    {
        this.output = output;
    }

    [Fact]
    public void OutputHashCode()
    {
        Support.Add( typeof(SampleObject).GetTypeInfo() );
        output.WriteLine( "Initialized:" );
        Support.Output( output );

        Support.Add( typeof(SampleObject).GetTypeInfo() );
        output.WriteLine( "After Initialized:" );
        Support.Output( output );
    }
}

public static class Support
{
    readonly static ICollection<int> Numbers = new List<int>();

    public static void Add( TypeInfo info )
    {
        var code = info.DeclaredConstructors.Single().GetParameters().Single().GetHashCode();
        Numbers.Add( code );
    }

    public static void Output( ITestOutputHelper output )
    {
        foreach ( var number in Numbers.ToArray() )
        {
            output.WriteLine( number.ToString() );
        }
    }
}

public class SampleObject
{
    public SampleObject( object parameter ) {}
}

The two test classes ensure that two threads are created and run in parallel. Upon running these tests, you should get results that look like the following:

Initialized:
39053774 <---- Different!
45653674
After Initialized:
39053774 <---- Different!
45653674
45653674
45653674

(NOTE: I've added "<---- Different!" to denote the unexpected value. You will not see this in the test results.)

As you can see, the result from the very first call to the GetParameters returns a different value than all subsequent calls.

I have had my nose in .NET for quite a while now, but have never seen anything quite like this. Is this expected behavior? Is there a preferred/known way of initializing the .NET type system so that this does not happen?

Finally, if anyone is interested, I ran into this problem while using xUnit with MEF 2, where a ParameterInfo being used as a key in a dictionary is not returning as equal to the ParameterInfo being passed in from a previously saved value. This, of course, results in unexpected behavior and resulting in failed tests when run concurrently.

EDIT: After some good feedback from the answers, I have (hopefully) clarified this question and scenario. The core of the issue is "Thread-Safety" of a "Thead-Safe" type, and acquiring better knowledge of what exactly this means.

ANSWER: This issue ended up being being due to several factors, one of which is due to me never-ending ignorance to multi-threaded scenarios, which it seems I am forever learning with no end for the foreseeable future. I am again appreciative of xUnit for being designed in such a way to learn this territory in such an effective manner.

The other issue does appear to be inconsistencies with how the .NET type system initializes. With the TypeInfo/Type, you get the same type/reference/hashcode no matter which thread accesses it however many times. For MemberInfo/MethodInfo/ParameterInfo, this is not the case. Thread access beware.

Finally, it seems I am not the only person with this confusion and this has indeed been recognized as an invalid assumption on a submitted issue to .NET Core's GitHub repository.

So, problem solved, mostly. I would like to send my thanks and appreciation to all involved in dealing with my ignorance in this matter, and helping me learn (what I am finding is) this very complex problem space.

like image 806
Mike-E Avatar asked Mar 13 '16 22:03

Mike-E


1 Answers

It is one instance on the first call, and then another instance on every subsequent call.

OK, that's fine. A bit weird, but the method is not documented as always returning the same instance every time.

So, one thread will get one version on the first call, and then each thread will get another (unchanging instance on each subsequent call.

Again, weird, but perfectly legal.

Is this expected behavior?

Well, I would not have expected it before your experiment. But after your experiment, yes, I expect that behaviour to continue.

Is there a preferred/known way of initializing the .NET type system so that this does not happen?

Not to my knowledge.

If I am using that first call to store a key then yes, that is a problem.

Then you have evidence that you should stop doing that. If it hurts when you do that, don't do it.

A ParameterInfo reference should always represent the same ParameterInfo reference regardless of the thread it is on or how many times accessed.

That's a moral statement about how the feature should have been designed. It's not how it was designed, and its clearly not how it was implemented. You can certainly make the argument that the design is bad.

Mr. Lippert is also right in that the documentation does not guarantee/specify this, but this has always been my expectation of and experience with this behavior until this point.

Past performance is no guarantee of future results; your experience was not sufficiently varied until now. Multithreading has a way of confounding people's expectations! A world where memory is constantly changing unless something is keeping it still is contrary to our normal mode of things being the same until something changes them.

like image 141
Eric Lippert Avatar answered Sep 27 '22 20:09

Eric Lippert