Skip to content

Commit 4398123

Browse files
tobias-tenglertobias-tengler
authored
Fix race condition on MethodInfo.GetParameters() (#8778)
1 parent a805ec0 commit 4398123

16 files changed

+244
-104
lines changed

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ private static ExtendedType FromMember(MemberInfo member)
2828
};
2929
}
3030

31-
public static ExtendedMethodInfo FromMethod(MethodInfo method, TypeCache cache)
31+
public static ExtendedMethodInfo FromMethod(
32+
MethodInfo method,
33+
ParameterInfo[] parameters,
34+
TypeCache cache)
3235
{
3336
var helper = new NullableHelper(method.DeclaringType!);
3437
var context = helper.GetContext(method);
@@ -40,8 +43,8 @@ public static ExtendedMethodInfo FromMethod(MethodInfo method, TypeCache cache)
4043
method,
4144
cache));
4245

43-
var parameters = method.GetParameters();
44-
var parameterTypes = ImmutableDictionary.CreateBuilder<ParameterInfo, IExtendedType>();
46+
var parameterTypes = ImmutableDictionary.CreateBuilder<ParameterInfo, IExtendedType>(
47+
ParameterInfoComparer.Instance);
4548

4649
foreach (var parameter in parameters)
4750
{

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,12 +205,16 @@ internal static ExtendedType FromMember(MemberInfo member, TypeCache cache)
205205
return Members.FromMember(member, cache);
206206
}
207207

208-
internal static ExtendedMethodInfo FromMethod(MethodInfo method, TypeCache cache)
208+
internal static ExtendedMethodInfo FromMethod(
209+
MethodInfo method,
210+
ParameterInfo[] parameters,
211+
TypeCache cache)
209212
{
210213
ArgumentNullException.ThrowIfNull(method);
214+
ArgumentNullException.ThrowIfNull(parameters);
211215
ArgumentNullException.ThrowIfNull(cache);
212216

213-
return Members.FromMethod(method, cache);
217+
return Members.FromMethod(method, parameters, cache);
214218
}
215219

216220
/// <summary>

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

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

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ internal sealed class DefaultResolverCompiler : IResolverCompiler
3030
private static readonly MethodInfo s_resolver =
3131
typeof(IResolverContext).GetMethod(nameof(IResolverContext.Resolver))!;
3232

33+
private readonly ITypeInspector _typeInspector;
3334
private readonly Dictionary<ParameterInfo, IParameterExpressionBuilder> _cache = [];
3435
private readonly List<IParameterExpressionBuilder> _parameterExpressionBuilders;
3536
private readonly List<IParameterExpressionBuilder> _defaultParameterExpressionBuilders;
@@ -40,9 +41,12 @@ internal sealed class DefaultResolverCompiler : IResolverCompiler
4041
new Dictionary<ParameterInfo, string>();
4142

4243
public DefaultResolverCompiler(
44+
ITypeInspector typeInspector,
4345
IServiceProvider schemaServiceProvider,
4446
IEnumerable<IParameterExpressionBuilder>? customParameterExpressionBuilders)
4547
{
48+
_typeInspector = typeInspector;
49+
4650
var appServiceProvider = schemaServiceProvider.GetService<IRootServiceProviderAccessor>()?.ServiceProvider;
4751
var serviceInspector = appServiceProvider?.GetService<IServiceProviderIsService>();
4852

@@ -251,9 +255,10 @@ public SubscribeResolverDelegate CompileSubscribe(
251255
{
252256
if (method.IsStatic)
253257
{
258+
var parameters = _typeInspector.GetParameters(method);
254259
var parameterExpr = CreateParameters(
255260
s_context,
256-
method.GetParameters(),
261+
parameters,
257262
argumentNames,
258263
parameterExpressionBuilders);
259264
Expression subscribeResolver = Call(method, parameterExpr);
@@ -262,7 +267,7 @@ public SubscribeResolverDelegate CompileSubscribe(
262267
}
263268
else
264269
{
265-
var parameters = method.GetParameters();
270+
var parameters = _typeInspector.GetParameters(method);
266271
var owner = CreateResolverOwner(s_context, sourceType, resolverType);
267272
var parameterExpr = CreateParameters(
268273
s_context,
@@ -326,12 +331,13 @@ private FieldResolverDelegate CompileStaticResolver(
326331
IReadOnlyDictionary<ParameterInfo, string> argumentNames,
327332
IReadOnlyList<IParameterExpressionBuilder> fieldParameterExpressionBuilders)
328333
{
329-
var parameters = CreateParameters(
334+
var parameters = _typeInspector.GetParameters(method);
335+
var parameterExpr = CreateParameters(
330336
s_context,
331-
method.GetParameters(),
337+
parameters,
332338
argumentNames,
333339
fieldParameterExpressionBuilders);
334-
Expression resolver = Call(method, parameters);
340+
Expression resolver = Call(method, parameterExpr);
335341
resolver = EnsureResolveResult(resolver, method.ReturnType);
336342
return Lambda<FieldResolverDelegate>(resolver, s_context).Compile();
337343
}
@@ -353,7 +359,7 @@ private FieldResolverDelegate CreateResolver(
353359

354360
if (member is MethodInfo method)
355361
{
356-
var parameters = method.GetParameters();
362+
var parameters = _typeInspector.GetParameters(method);
357363
var owner = CreateResolverOwner(s_context, source, resolverType);
358364
var parameterExpr = CreateParameters(
359365
s_context,
@@ -391,7 +397,7 @@ private FieldResolverDelegate CreateResolver(
391397

392398
if (member is MethodInfo method)
393399
{
394-
var parameters = method.GetParameters();
400+
var parameters = _typeInspector.GetParameters(method);
395401

396402
if (IsPureResolver(method, parameters, fieldParameterExpressionBuilders))
397403
{

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

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

4141
if (messageParameter is null)

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

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,15 @@ public class DefaultTypeInspector(bool ignoreRequiredAttribute = false) : Conven
2727
private const string EqualsMethodName = "Equals";
2828
private const string CloneMethodName = "<Clone>$";
2929

30+
#if NET9_0_OR_GREATER
31+
private readonly Lock _parametersLock = new();
32+
#else
33+
private readonly object _parametersLock = new();
34+
#endif
3035
private readonly TypeCache _typeCache = new();
3136
private readonly ConcurrentDictionary<MethodInfo, ExtendedMethodInfo> _methods = [];
32-
private readonly ConcurrentDictionary<(Type, bool, bool), MemberInfo[]> _memberCache = new();
37+
private readonly ConcurrentDictionary<(Type, bool, bool), MemberInfo[]> _membersCache = new();
38+
private readonly ConcurrentDictionary<MethodInfo, ParameterInfo[]> _parametersCache = new();
3339

3440
/// <summary>
3541
/// Infer type to be non-null if <see cref="RequiredAttribute"/> is found.
@@ -47,7 +53,7 @@ public ReadOnlySpan<MemberInfo> GetMembers(
4753

4854
var cacheKey = (type, includeIgnored, includeStatic);
4955

50-
if (_memberCache.TryGetValue(cacheKey, out var cached))
56+
if (_membersCache.TryGetValue(cacheKey, out var cached))
5157
{
5258
return cached;
5359
}
@@ -71,12 +77,35 @@ public ReadOnlySpan<MemberInfo> GetMembers(
7177
var selectedMembers = new MemberInfo[next];
7278
span.CopyTo(selectedMembers);
7379
span.Clear();
74-
_memberCache.TryAdd(cacheKey, selectedMembers);
80+
_membersCache.TryAdd(cacheKey, selectedMembers);
7581

7682
ArrayPool<MemberInfo>.Shared.Return(temp);
7783
return selectedMembers;
7884
}
7985

86+
/// <inheritdoc />
87+
public ParameterInfo[] GetParameters(MethodInfo method)
88+
{
89+
// ReSharper disable once InconsistentlySynchronizedField
90+
if (_parametersCache.TryGetValue(method, out var parameters))
91+
{
92+
return parameters;
93+
}
94+
95+
lock (_parametersLock)
96+
{
97+
if (_parametersCache.TryGetValue(method, out parameters))
98+
{
99+
return parameters;
100+
}
101+
102+
parameters = method.GetParameters();
103+
_parametersCache.TryAdd(method, parameters);
104+
105+
return parameters;
106+
}
107+
}
108+
80109
/// <inheritdoc />
81110
public virtual bool IsMemberIgnored(MemberInfo member)
82111
{
@@ -164,7 +193,11 @@ private IExtendedType GetArgumentTypeInternal(ParameterInfo parameter)
164193

165194
var info = _methods.GetOrAdd(
166195
method,
167-
static (m, c) => ExtendedType.FromMethod(m, c), _typeCache);
196+
static (methodInfo, typeInspector) =>
197+
{
198+
var parameters = typeInspector.GetParameters(methodInfo);
199+
return ExtendedType.FromMethod(methodInfo, parameters, typeInspector._typeCache);
200+
}, this);
168201

169202
return info.ParameterTypes[parameter];
170203
}
@@ -667,7 +700,8 @@ private bool CanBeHandled(
667700
if (member is MethodInfo { IsGenericMethodDefinition: false } method
668701
&& CanHandleReturnType(member, method.ReturnType, allowObjectType))
669702
{
670-
foreach (var parameter in method.GetParameters())
703+
var parameters = GetParameters(method);
704+
foreach (var parameter in parameters)
671705
{
672706
if (!CanHandleParameter(parameter, allowObjectType))
673707
{

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

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ public sealed partial class DescriptorContext : IDescriptorContext
2626
private readonly Func<IReadOnlySchemaOptions> _options;
2727
private FeatureReference<TypeSystemFeature> _typeSystemFeature = FeatureReference<TypeSystemFeature>.Default;
2828
private TypeDiscoveryHandler[]? _typeDiscoveryHandlers;
29-
private INamingConventions? _naming;
30-
private ITypeInspector? _inspector;
3129

3230
private DescriptorContext(
3331
Func<IReadOnlySchemaOptions> options,
@@ -42,13 +40,16 @@ private DescriptorContext(
4240
_serviceHelper = new ServiceHelper(Services);
4341
Features = features;
4442
TypeInterceptor = typeInterceptor;
45-
ResolverCompiler = new DefaultResolverCompiler(
46-
schemaServices,
47-
_serviceHelper.GetParameterExpressionBuilders());
4843

4944
TypeConverter = _serviceHelper.GetTypeConverter();
5045
InputFormatter = _serviceHelper.GetInputFormatter(TypeConverter);
5146
InputParser = _serviceHelper.GetInputParser(TypeConverter);
47+
48+
TypeInspector = this.GetConventionOrDefault<ITypeInspector>(new DefaultTypeInspector());
49+
ResolverCompiler = new DefaultResolverCompiler(
50+
TypeInspector,
51+
schemaServices,
52+
_serviceHelper.GetParameterExpressionBuilders());
5253
}
5354

5455
internal SchemaBuilder.LazySchema Schema { get; }
@@ -60,11 +61,12 @@ private DescriptorContext(
6061
public IReadOnlySchemaOptions Options => _options();
6162

6263
/// <inheritdoc />
64+
[field: AllowNull, MaybeNull]
6365
public INamingConventions Naming
6466
{
6567
get
6668
{
67-
_naming ??= GetConventionOrDefault<INamingConventions>(() => Options.UseXmlDocumentation
69+
field ??= GetConventionOrDefault<INamingConventions>(() => Options.UseXmlDocumentation
6870
? new DefaultNamingConventions(
6971
new XmlDocumentationProvider(
7072
new XmlDocumentationFileResolver(
@@ -73,21 +75,12 @@ public INamingConventions Naming
7375
: new DefaultNamingConventions(
7476
new NoopDocumentationProvider()));
7577

76-
return _naming;
78+
return field;
7779
}
7880
}
7981

8082
/// <inheritdoc />
81-
public ITypeInspector TypeInspector
82-
{
83-
get
84-
{
85-
_inspector ??= this.GetConventionOrDefault<ITypeInspector>(
86-
new DefaultTypeInspector());
87-
88-
return _inspector;
89-
}
90-
}
83+
public ITypeInspector TypeInspector { get; }
9184

9285
/// <inheritdoc />
9386
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
@@ -34,6 +34,17 @@ ReadOnlySpan<MemberInfo> GetMembers(
3434
bool includeStatic = false,
3535
bool allowObject = false);
3636

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

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

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

5353
if (member is MethodInfo m)
5454
{
55-
_parameterInfos = m.GetParameters();
55+
_parameterInfos = context.TypeInspector.GetParameters(m);
5656
Parameters = _parameterInfos.ToDictionary(t => t.Name!, StringComparer.Ordinal);
5757
}
5858
}
@@ -241,7 +241,7 @@ private IInterfaceFieldDescriptor ResolveWithInternal(
241241

242242
if (propertyOrMethod is MethodInfo m)
243243
{
244-
_parameterInfos = m.GetParameters();
244+
_parameterInfos = Context.TypeInspector.GetParameters(m);
245245
Parameters = _parameterInfos.ToDictionary(t => t.Name!, StringComparer.Ordinal);
246246
}
247247

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ protected ObjectFieldDescriptor(
6262

6363
if (member is MethodInfo m)
6464
{
65-
_parameterInfos = m.GetParameters();
65+
_parameterInfos = context.TypeInspector.GetParameters(m);
6666
Parameters = _parameterInfos.ToDictionary(t => t.Name!, StringComparer.Ordinal);
6767
Configuration.ResultType = m.ReturnType;
6868
}
@@ -172,7 +172,7 @@ private void CompleteArguments(ObjectFieldConfiguration definition)
172172

173173
if (subscribeMember is MethodInfo subscribeMethod)
174174
{
175-
var subscribeParameters = subscribeMethod.GetParameters();
175+
var subscribeParameters = Context.TypeInspector.GetParameters(subscribeMethod);
176176
var parameterLength = _parameterInfos.Length + subscribeParameters.Length;
177177
var parameters = new ParameterInfo[parameterLength];
178178

@@ -397,7 +397,7 @@ private IObjectFieldDescriptor ResolveWithInternal(
397397

398398
if (propertyOrMethod is MethodInfo m)
399399
{
400-
_parameterInfos = m.GetParameters();
400+
_parameterInfos = Context.TypeInspector.GetParameters(m);
401401
Parameters = _parameterInfos.ToDictionary(t => t.Name!, StringComparer.Ordinal);
402402
}
403403

0 commit comments

Comments
 (0)