Skip to content

MethodBase.GetParameters() is not thread-safe #99147

Open
@koenigst

Description

@koenigst

Description

If multiple threads call System.Reflection.MethodBase.GetParameters() at the same time, the resulting ParameterInfos are not always Equals even though they represent the same parameter.

Reproduction Steps

using System;
using System.Linq;
using System.Reflection;
using Xunit;

public sealed class ReflectionThreadSafety
{
    [Fact]
    public void SequentialBehavior()
    {
        var method = typeof(ReflectionThreadSafety).GetMethod("SequentialMethod", BindingFlags.Static | BindingFlags.NonPublic)!;
        var parameters = new[]
        {
            method.GetParameters()[0],
            method.GetParameters()[0],
        };
        Assert.All(parameters, p => Assert.Equal(parameters[0], p));
    }

    [Fact]
    public void ConcurrentBehavior()
    {
        var method = typeof(ReflectionThreadSafety).GetMethod("ConcurrentMethod", BindingFlags.Static | BindingFlags.NonPublic)!;
        var parameters = Enumerable.Range(0, 2 * Environment.ProcessorCount)
            .AsParallel()
            .WithExecutionMode(ParallelExecutionMode.ForceParallelism)
            .Select(_ => method.GetParameters()[0])
            .ToList();
        Assert.All(parameters, p => Assert.Equal(parameters[0], p));
    }

    private static void SequentialMethod(int p, DateTimeOffset o, TimeSpan t, AppDomain a) { }
    private static void ConcurrentMethod(int p, DateTimeOffset o, TimeSpan t, AppDomain a) { }
}

Expected behavior

Sequential and concurrent access should behave the same.

Actual behavior

The concurrent test sometimes fails (repeat test run a few times as the concurrency issue can not be reproduced reliably). The likely cause is that multiple threads fill the RuntimeMethodInfo.m_parameters cache resulting in multiple instances of ParameterInfo for the same method parameter. Most information on the instances are identical but because ParameterInfo.Equals is based on reference equality the test fails.

Regression?

The code is essentially the same as in .NET Framework 4.8.1 and the relevant .NET Core code has not changed in the last five years.

Known Workarounds

Call MethodBase.GetParameters from a single thread before accessing it in parallel. Once the cache is initialised all further concurrent calls will return the correct parameter instances.

Configuration

  • TargetFramework: net8.0
  • .NET SDK: 8.0.101
  • Architecture: x64
  • OS: Windows 11 Enterprise 22H2 (OS build 22621.3155)

Other information

Calling reflection code concurrently seems to be a rather unusual case because most of the reflection code uses the same not completely thread-safe caching mechanism and I have not run into this issue in the last ten years of .NET use. I am also aware that making the caching thread-safe has some overhead that is not desired for dealing with a rare occurence. The documentation declares the class as thread-safe. Would it make sense to change the documentation instead of the implementation?

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions