Skip to content

Commit 74ad56b

Browse files
tobias-tenglermichaelstaib
tobias-tengler
authored and
michaelstaib
committed
Fix race condition on MethodInfo.GetParameters() (#8778)
1 parent 6413a98 commit 74ad56b

File tree

12 files changed

+260
-116
lines changed

12 files changed

+260
-116
lines changed

‎src/HotChocolate/Core/src/Types/Internal/ExtendedType.Members.cs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Collections.Immutable;
12
using System.Reflection;
23
using HotChocolate.Utilities;
34

@@ -29,7 +30,10 @@ private static ExtendedType FromMember(MemberInfo member)
2930
};
3031
}
3132

32-
public static ExtendedMethodInfo FromMethod(MethodInfo method, TypeCache cache)
33+
public static ExtendedMethodInfo FromMethod(
34+
MethodInfo method,
35+
ParameterInfo[] parameters,
36+
TypeCache cache)
3337
{
3438
var helper = new NullableHelper(method.DeclaringType!);
3539
var context = helper.GetContext(method);
@@ -41,8 +45,8 @@ public static ExtendedMethodInfo FromMethod(MethodInfo method, TypeCache cache)
4145
method,
4246
cache));
4347

44-
var parameters = method.GetParameters();
45-
var parameterTypes = new Dictionary<ParameterInfo, IExtendedType>();
48+
var parameterTypes = ImmutableDictionary.CreateBuilder<ParameterInfo, IExtendedType>(
49+
ParameterInfoComparer.Instance);
4650

4751
foreach (var parameter in parameters)
4852
{

‎src/HotChocolate/Core/src/Types/Internal/ExtendedType.cs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,19 +221,27 @@ internal static ExtendedType FromMember(MemberInfo member, TypeCache cache)
221221
return Members.FromMember(member, cache);
222222
}
223223

224-
internal static ExtendedMethodInfo FromMethod(MethodInfo method, TypeCache cache)
224+
internal static ExtendedMethodInfo FromMethod(
225+
MethodInfo method,
226+
ParameterInfo[] parameters,
227+
TypeCache cache)
225228
{
226229
if (method is null)
227230
{
228231
throw new ArgumentNullException(nameof(method));
229232
}
230233

234+
if (parameters is null)
235+
{
236+
throw new ArgumentNullException(nameof(parameters));
237+
}
238+
231239
if (cache is null)
232240
{
233241
throw new ArgumentNullException(nameof(cache));
234242
}
235243

236-
return Members.FromMethod(method, cache);
244+
return Members.FromMethod(method, parameters, cache);
237245
}
238246

239247
/// <summary>

‎src/HotChocolate/Core/src/Types/Internal/ParameterInfoComparer.cs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#nullable enable
2+
3+
using System.Reflection;
4+
5+
namespace HotChocolate.Internal;
6+
7+
internal sealed class ParameterInfoComparer : IEqualityComparer<ParameterInfo>
8+
{
9+
public bool Equals(ParameterInfo? x, ParameterInfo? y)
10+
{
11+
if (ReferenceEquals(x, y))
12+
{
13+
return true;
14+
}
15+
16+
if (x is null || y is null)
17+
{
18+
return false;
19+
}
20+
21+
return x.MetadataToken == y.MetadataToken
22+
&& x.Member.Module.MetadataToken == y.Member.Module.MetadataToken;
23+
}
24+
25+
public int GetHashCode(ParameterInfo obj)
26+
{
27+
return HashCode.Combine(obj.MetadataToken, obj.Member.Module.MetadataToken);
28+
}
29+
30+
public static readonly ParameterInfoComparer Instance = new();
31+
}

‎src/HotChocolate/Core/src/Types/Resolvers/DefaultResolverCompiler.cs‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ internal sealed class DefaultResolverCompiler : IResolverCompiler
3232
private static readonly MethodInfo _resolver =
3333
typeof(IResolverContext).GetMethod(nameof(IResolverContext.Resolver))!;
3434

35-
private readonly Dictionary<ParameterInfo, IParameterExpressionBuilder> _cache = new();
35+
private readonly ITypeInspector _typeInspector;
36+
private readonly Dictionary<ParameterInfo, IParameterExpressionBuilder> _cache = [];
3637
private readonly List<IParameterExpressionBuilder> _parameterExpressionBuilders;
3738
private readonly List<IParameterExpressionBuilder> _defaultParameterExpressionBuilders;
3839
private readonly List<IParameterFieldConfiguration> _parameterFieldConfigurations;
@@ -42,9 +43,12 @@ internal sealed class DefaultResolverCompiler : IResolverCompiler
4243
new Dictionary<ParameterInfo, string>();
4344

4445
public DefaultResolverCompiler(
46+
ITypeInspector typeInspector,
4547
IServiceProvider schemaServiceProvider,
4648
IEnumerable<IParameterExpressionBuilder>? customParameterExpressionBuilders)
4749
{
50+
_typeInspector = typeInspector;
51+
4852
var appServiceProvider = schemaServiceProvider.GetService<IApplicationServiceProvider>();
4953
var serviceInspector = appServiceProvider?.GetService<IServiceProviderIsService>();
5054

@@ -265,9 +269,10 @@ public SubscribeResolverDelegate CompileSubscribe(
265269
{
266270
if (method.IsStatic)
267271
{
272+
var parameters = _typeInspector.GetParameters(method);
268273
var parameterExpr = CreateParameters(
269274
_context,
270-
method.GetParameters(),
275+
parameters,
271276
argumentNames,
272277
parameterExpressionBuilders);
273278
Expression subscribeResolver = Call(method, parameterExpr);
@@ -276,7 +281,7 @@ public SubscribeResolverDelegate CompileSubscribe(
276281
}
277282
else
278283
{
279-
var parameters = method.GetParameters();
284+
var parameters = _typeInspector.GetParameters(method);
280285
var owner = CreateResolverOwner(_context, sourceType, resolverType);
281286
var parameterExpr = CreateParameters(
282287
_context,
@@ -346,12 +351,13 @@ private FieldResolverDelegate CompileStaticResolver(
346351
IReadOnlyDictionary<ParameterInfo, string> argumentNames,
347352
IReadOnlyList<IParameterExpressionBuilder> fieldParameterExpressionBuilders)
348353
{
349-
var parameters = CreateParameters(
354+
var parameters = _typeInspector.GetParameters(method);
355+
var parameterExpr = CreateParameters(
350356
_context,
351-
method.GetParameters(),
357+
parameters,
352358
argumentNames,
353359
fieldParameterExpressionBuilders);
354-
Expression resolver = Call(method, parameters);
360+
Expression resolver = Call(method, parameterExpr);
355361
resolver = EnsureResolveResult(resolver, method.ReturnType);
356362
return Lambda<FieldResolverDelegate>(resolver, _context).Compile();
357363
}
@@ -373,7 +379,7 @@ private FieldResolverDelegate CreateResolver(
373379

374380
if (member is MethodInfo method)
375381
{
376-
var parameters = method.GetParameters();
382+
var parameters = _typeInspector.GetParameters(method);
377383
var owner = CreateResolverOwner(_context, source, resolverType);
378384
var parameterExpr = CreateParameters(
379385
_context,
@@ -411,7 +417,7 @@ private FieldResolverDelegate CreateResolver(
411417

412418
if (member is MethodInfo method)
413419
{
414-
var parameters = method.GetParameters();
420+
var parameters = _typeInspector.GetParameters(method);
415421

416422
if (IsPureResolver(method, parameters, fieldParameterExpressionBuilders))
417423
{

‎src/HotChocolate/Core/src/Types/Types/Attributes/SubscribeAttribute.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ protected override void OnConfigure(
3737
if (MessageType is null)
3838
{
3939
var messageParameter =
40-
method.GetParameters()
40+
context.TypeInspector.GetParameters(method)
4141
.FirstOrDefault(t => t.IsDefined(typeof(EventMessageAttribute)));
4242

4343
if (messageParameter is null)

‎src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/DefaultTypeInspector.cs‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,15 @@ public class DefaultTypeInspector(bool ignoreRequiredAttribute = false) : Conven
2929
private const string _equals = "Equals";
3030
private const string _clone = "<Clone>$";
3131

32+
#if NET9_0_OR_GREATER
33+
private readonly Lock _parametersLock = new();
34+
#else
35+
private readonly object _parametersLock = new();
36+
#endif
3237
private readonly TypeCache _typeCache = new();
33-
private readonly ConcurrentDictionary<MethodInfo, ExtendedMethodInfo> _methods = new();
34-
private readonly ConcurrentDictionary<(Type, bool, bool), MemberInfo[]> _memberCache = new();
38+
private readonly ConcurrentDictionary<MethodInfo, ExtendedMethodInfo> _methods = [];
39+
private readonly ConcurrentDictionary<(Type, bool, bool), MemberInfo[]> _membersCache = new();
40+
private readonly ConcurrentDictionary<MethodInfo, ParameterInfo[]> _parametersCache = new();
3541

3642
/// <summary>
3743
/// Infer type to be non-null if <see cref="RequiredAttribute"/> is found.
@@ -52,7 +58,7 @@ public ReadOnlySpan<MemberInfo> GetMembers(
5258

5359
var cacheKey = (type, includeIgnored, includeStatic);
5460

55-
if (_memberCache.TryGetValue(cacheKey, out var cached))
61+
if (_membersCache.TryGetValue(cacheKey, out var cached))
5662
{
5763
return cached;
5864
}
@@ -76,12 +82,35 @@ public ReadOnlySpan<MemberInfo> GetMembers(
7682
var selectedMembers = new MemberInfo[next];
7783
span.CopyTo(selectedMembers);
7884
span.Clear();
79-
_memberCache.TryAdd(cacheKey, selectedMembers);
85+
_membersCache.TryAdd(cacheKey, selectedMembers);
8086

8187
ArrayPool<MemberInfo>.Shared.Return(temp);
8288
return selectedMembers;
8389
}
8490

91+
/// <inheritdoc />
92+
public ParameterInfo[] GetParameters(MethodInfo method)
93+
{
94+
// ReSharper disable once InconsistentlySynchronizedField
95+
if (_parametersCache.TryGetValue(method, out var parameters))
96+
{
97+
return parameters;
98+
}
99+
100+
lock (_parametersLock)
101+
{
102+
if (_parametersCache.TryGetValue(method, out parameters))
103+
{
104+
return parameters;
105+
}
106+
107+
parameters = method.GetParameters();
108+
_parametersCache.TryAdd(method, parameters);
109+
110+
return parameters;
111+
}
112+
}
113+
85114
/// <inheritdoc />
86115
public virtual bool IsMemberIgnored(MemberInfo member)
87116
{
@@ -181,7 +210,16 @@ public virtual IExtendedType GetArgumentType(
181210
private IExtendedType GetArgumentTypeInternal(ParameterInfo parameter)
182211
{
183212
var method = (MethodInfo)parameter.Member;
184-
var info = _methods.GetOrAdd(method, static (m, c) => ExtendedType.FromMethod(m, c), _typeCache);
213+
214+
var info = _methods.GetOrAdd(
215+
method,
216+
static (methodInfo, typeInspector) =>
217+
{
218+
var parameters = typeInspector.GetParameters(methodInfo);
219+
return ExtendedType.FromMethod(methodInfo, parameters, typeInspector._typeCache);
220+
},
221+
this);
222+
185223
return info.ParameterTypes[parameter];
186224
}
187225

@@ -737,7 +775,8 @@ private bool CanBeHandled(
737775
if (member is MethodInfo { IsGenericMethodDefinition: false, } method &&
738776
CanHandleReturnType(member, method.ReturnType, allowObjectType))
739777
{
740-
foreach (var parameter in method.GetParameters())
778+
var parameters = GetParameters(method);
779+
foreach (var parameter in parameters)
741780
{
742781
if (!CanHandleParameter(parameter, allowObjectType))
743782
{

‎src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/DescriptorContext.cs‎

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,9 @@ public sealed partial class DescriptorContext : IDescriptorContext
2727
private readonly IServiceProvider _schemaServices;
2828
private readonly ServiceHelper _serviceHelper;
2929
private readonly Func<IReadOnlySchemaOptions> _options;
30+
private INamingConventions? _naming;
3031

3132
private TypeDiscoveryHandler[]? _typeDiscoveryHandlers;
32-
private INamingConventions? _naming;
33-
private ITypeInspector? _inspector;
3433

3534
private DescriptorContext(
3635
Func<IReadOnlySchemaOptions> options,
@@ -47,13 +46,16 @@ private DescriptorContext(
4746
_serviceHelper = new ServiceHelper(_schemaServices);
4847
ContextData = contextData;
4948
TypeInterceptor = typeInterceptor;
50-
ResolverCompiler = new DefaultResolverCompiler(
51-
schemaServices,
52-
_serviceHelper.GetParameterExpressionBuilders());
5349

5450
TypeConverter = _serviceHelper.GetTypeConverter();
5551
InputFormatter = _serviceHelper.GetInputFormatter(TypeConverter);
5652
InputParser = _serviceHelper.GetInputParser(TypeConverter);
53+
54+
TypeInspector = this.GetConventionOrDefault<ITypeInspector>(new DefaultTypeInspector());
55+
ResolverCompiler = new DefaultResolverCompiler(
56+
TypeInspector,
57+
schemaServices,
58+
_serviceHelper.GetParameterExpressionBuilders());
5759
}
5860

5961
internal SchemaBuilder.LazySchema Schema { get; }
@@ -69,37 +71,21 @@ public INamingConventions Naming
6971
{
7072
get
7173
{
72-
if (_naming is null)
73-
{
74-
_naming = GetConventionOrDefault<INamingConventions>(
75-
() => Options.UseXmlDocumentation
76-
? new DefaultNamingConventions(
77-
new XmlDocumentationProvider(
78-
new XmlDocumentationFileResolver(
79-
Options.ResolveXmlDocumentationFileName),
80-
_serviceHelper.GetStringBuilderPool()))
81-
: new DefaultNamingConventions(
82-
new NoopDocumentationProvider()));
83-
}
74+
_naming ??= GetConventionOrDefault<INamingConventions>(() => Options.UseXmlDocumentation
75+
? new DefaultNamingConventions(
76+
new XmlDocumentationProvider(
77+
new XmlDocumentationFileResolver(
78+
Options.ResolveXmlDocumentationFileName),
79+
_serviceHelper.GetStringBuilderPool()))
80+
: new DefaultNamingConventions(
81+
new NoopDocumentationProvider()));
8482

8583
return _naming;
8684
}
8785
}
8886

8987
/// <inheritdoc />
90-
public ITypeInspector TypeInspector
91-
{
92-
get
93-
{
94-
if (_inspector is null)
95-
{
96-
_inspector = this.GetConventionOrDefault<ITypeInspector>(
97-
new DefaultTypeInspector());
98-
}
99-
100-
return _inspector;
101-
}
102-
}
88+
public ITypeInspector TypeInspector { get; }
10389

10490
/// <inheritdoc />
10591
public TypeInterceptor TypeInterceptor { get; }

‎src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/ITypeInspector.cs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,17 @@ ReadOnlySpan<MemberInfo> GetMembers(
3636
bool includeStatic = false,
3737
bool allowObject = false);
3838

39+
/// <summary>
40+
/// Gets the parameters of <paramref name="method"/> in a thread-safe manner.
41+
/// </summary>
42+
/// <param name="method">
43+
/// The method to get the parameters from.
44+
/// </param>
45+
/// <returns>
46+
/// The parameters of the <paramref name="method"/>.
47+
/// </returns>
48+
ParameterInfo[] GetParameters(MethodInfo method);
49+
3950
/// <summary>
4051
/// Defines if a member shall be ignored. This method interprets ignore attributes.
4152
/// </summary>

‎src/HotChocolate/Core/src/Types/Types/Descriptors/InterfaceFieldDescriptor.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ protected internal InterfaceFieldDescriptor(
5454

5555
if (member is MethodInfo m)
5656
{
57-
_parameterInfos = m.GetParameters();
57+
_parameterInfos = context.TypeInspector.GetParameters(m);
5858
Parameters = _parameterInfos.ToDictionary(t => t.Name!, StringComparer.Ordinal);
5959
}
6060
}
@@ -255,7 +255,7 @@ private IInterfaceFieldDescriptor ResolveWithInternal(
255255

256256
if (propertyOrMethod is MethodInfo m)
257257
{
258-
_parameterInfos = m.GetParameters();
258+
_parameterInfos = Context.TypeInspector.GetParameters(m);
259259
Parameters = _parameterInfos.ToDictionary(t => t.Name!, StringComparer.Ordinal);
260260
}
261261

0 commit comments

Comments
 (0)