Skip to content

Commit f12b8ee

Browse files
authored
Fix thread-safety bug in System.ClientModel's ReflectionContext (Azure#51253)
ReflectionContext holds onto a Dictionary and doesn't use any synchronization to access it, but ModelReaderWriter stores a singleton into a static field, which is also used unprotected. This leads to corruption of the dictionary as the ReflectionContext gets hammered on by concurrent usage.
1 parent 0bf548e commit f12b8ee

File tree

1 file changed

+6
-10
lines changed

1 file changed

+6
-10
lines changed
Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,19 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
using System.Collections.Concurrent;
5+
46
namespace System.ClientModel.Primitives;
57

68
internal class ReflectionContext : ModelReaderWriterContext
79
{
8-
private Dictionary<Type, ModelReaderWriterTypeBuilder>? _typeBuilders;
9-
private Dictionary<Type, ModelReaderWriterTypeBuilder> TypeBuilders => _typeBuilders ??= [];
10+
private ConcurrentDictionary<Type, ModelReaderWriterTypeBuilder>? _typeBuilders;
11+
private ConcurrentDictionary<Type, ModelReaderWriterTypeBuilder> TypeBuilders =>
12+
LazyInitializer.EnsureInitialized(ref _typeBuilders, static () => [])!;
1013

1114
protected override bool TryGetTypeBuilderCore(Type type, out ModelReaderWriterTypeBuilder? builder)
1215
{
13-
if (TypeBuilders.TryGetValue(type, out builder))
14-
{
15-
return true;
16-
}
17-
18-
builder = new ReflectionModelBuilder(type);
19-
TypeBuilders[type] = builder;
20-
16+
builder = TypeBuilders.GetOrAdd(type, static type => new ReflectionModelBuilder(type));
2117
return true;
2218
}
2319
}

0 commit comments

Comments
 (0)