Skip to content

Transfer ownership in ReflectionMetadataHook.Register instead of cloning by @Evangelink in #9586 (backport to rel/4.3)#9589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Amaury Levé (Evangelink) merged 2 commits into from
Jul 3, 2026
Merged

Transfer ownership in ReflectionMetadataHook.Register instead of cloning by @Evangelink in #9586 (backport to rel/4.3) #9589

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ecf779d
Transfer ownership in ReflectionMetadataHook.Register instead of cloning
Evangelink Jul 3, 2026
5435548
Reword ownership-transfer remarks to avoid implying a trust boundary
Evangelink Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 59 additions & 41 deletions src/Adapter/MSTestAdapter.PlatformServices/SourceGeneration/ReflectionMetadataHook.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -52,8 +52,8 @@ public static class ReflectionMetadataHook
/// <param name="types">All types directly annotated with <c>[TestClass]</c> in the assembly.</param>
/// <param name="testMethods">
/// A map from each test class to its <c>[TestMethod]</c>-annotated <see cref="MethodInfo"/>
/// set. The dictionary and arrays are copied defensively; the caller may mutate the inputs
/// after the call.
/// set. Ownership transfers to the adapter: the source generator hands over freshly-built
/// collections and must not mutate them after the call (see the remarks on the full overload).
/// </param>
/// <remarks>
/// Do not call this method from hand-written code; it is meant to be invoked exclusively from
Expand DownExpand Up@@ -120,8 +120,20 @@ public static void Register(
/// assigns it directly.
/// </param>
/// <remarks>
/// <para>
/// Do not call this method from hand-written code; it is meant to be invoked exclusively from
/// the <c>[ModuleInitializer]</c> emitted by the MSTest source generator.
/// </para>
/// <para>
/// <b>Ownership transfer.</b> The adapter takes ownership of every collection passed in
/// (the <paramref name="types"/> array, the <paramref name="assemblyAttributes"/> array, and
/// each dictionary with its value arrays) and stores them without cloning. Callers MUST hand
/// over freshly-built collections and MUST NOT mutate them after the call returns; the source
/// generator (the only intended caller) already emits fresh, throwaway collections that satisfy
/// this. This is a contract about ownership and mutation, not caller identity: it trades the
/// previous defensive copies for zero-copy startup on the understanding that the inputs are the
/// adapter's to keep.
/// </para>
/// </remarks>
[EditorBrowsable(EditorBrowsableState.Never)]
public static void Register(
Expand DownExpand Up@@ -174,29 +186,20 @@ public static void Register(
throw new ArgumentNullException(nameof(propertySetters));
}

var typesCopy = (Type[])types.Clone();
// Ownership transfer (see the remarks on this method): the source generator hands over
// freshly-built, throwaway collections and never mutates them after the call, so we store
// them directly instead of cloning. Arrays are kept by reference; dictionaries are reused
// as-is when already concrete and only materialized when the caller passed some other
// IReadOnlyDictionary implementation.
Dictionary<Type, MethodInfo[]> testMethodsMap = AsOwnedDictionary(testMethods);
Dictionary<Type, Attribute[]> typeAttributesMap = AsOwnedDictionary(typeAttributes);
Dictionary<MethodInfo, Func<object?, object?[]?, object?>> methodInvokersMap = AsOwnedDictionary(methodInvokers);
Dictionary<PropertyInfo, Action<object?, object?>> propertySettersMap = AsOwnedDictionary(propertySetters);

var testMethodsCopy = new Dictionary<Type, MethodInfo[]>(testMethods.Count);
foreach (KeyValuePair<Type, MethodInfo[]> kvp in testMethods)
{
testMethodsCopy[kvp.Key] = (MethodInfo[])kvp.Value.Clone();
}

var typeAttributesCopy = new Dictionary<Type, Attribute[]>(typeAttributes.Count);
foreach (KeyValuePair<Type, Attribute[]> kvp in typeAttributes)
{
typeAttributesCopy[kvp.Key] = (Attribute[])kvp.Value.Clone();
}

object[] assemblyAttributesCopy = (object[])assemblyAttributes.Clone();

var methodInvokersCopy = new Dictionary<MethodInfo, Func<object?, object?[]?, object?>>(methodInvokers.Count);
foreach (KeyValuePair<MethodInfo, Func<object?, object?[]?, object?>> kvp in methodInvokers)
{
methodInvokersCopy[kvp.Key] = kvp.Value;
}

var constructorInvokersCopy = new Dictionary<Type, SourceGeneratedReflectionDataProvider.ConstructorInvoker[]>(constructorInvokers.Count);
// ConstructorInvokerInfo (public struct) has to be projected onto the adapter's internal
// ConstructorInvoker type; this is a representation change, not a defensive copy, and the
// parameter-type arrays are taken by reference.
var constructorInvokersMap = new Dictionary<Type, SourceGeneratedReflectionDataProvider.ConstructorInvoker[]>(constructorInvokers.Count);
foreach (KeyValuePair<Type, ConstructorInvokerInfo[]> kvp in constructorInvokers)
{
var invokers = new SourceGeneratedReflectionDataProvider.ConstructorInvoker[kvp.Value.Length];
Expand All@@ -205,26 +208,20 @@ public static void Register(
ConstructorInvokerInfo info = kvp.Value[i];
invokers[i] = new SourceGeneratedReflectionDataProvider.ConstructorInvoker
{
Parameters = (Type[])info.ParameterTypes.Clone(),
Parameters = info.ParameterTypes,
Invoker = info.Invoker,
};
}

constructorInvokersCopy[kvp.Key] = invokers;
}

var propertySettersCopy = new Dictionary<PropertyInfo, Action<object?, object?>>(propertySetters.Count);
foreach (KeyValuePair<PropertyInfo, Action<object?, object?>> kvp in propertySetters)
{
propertySettersCopy[kvp.Key] = kvp.Value;
constructorInvokersMap[kvp.Key] = invokers;
}

// TypesByName must always match Type.FullName at runtime (see comment in the source
// generator emitter): compute it on the runtime side from typeof(T).FullName so the
// generator emits less code and the same FullName conventions are honored for nested
// and generic types.
var typesByName = new Dictionary<string, Type>(typesCopy.Length, StringComparer.Ordinal);
foreach (Type type in typesCopy)
var typesByName = new Dictionary<string, Type>(types.Length, StringComparer.Ordinal);
foreach (Type type in types)
{
if (type.FullName is { } fullName)
{
Expand All@@ -236,14 +233,14 @@ public static void Register(
{
Assembly = assembly,
AssemblyName = assembly.GetName().Name ?? string.Empty,
Types = typesCopy,
Types = types,
TypesByName = typesByName,
TypeMethods = testMethodsCopy,
TypeAttributes = typeAttributesCopy,
AssemblyAttributes = assemblyAttributesCopy,
TypeMethodInvokers = methodInvokersCopy,
TypeConstructorsInvoker = constructorInvokersCopy,
TypePropertySetters = propertySettersCopy,
TypeMethods = testMethodsMap,
TypeAttributes = typeAttributesMap,
AssemblyAttributes = assemblyAttributes,
TypeMethodInvokers = methodInvokersMap,
TypeConstructorsInvoker = constructorInvokersMap,
TypePropertySetters = propertySettersMap,
};

lock (Lock)
Expand All@@ -260,6 +257,27 @@ public static void Register(
}
}

// Reuses the caller-provided dictionary when it is already a concrete Dictionary<,> (the shape
// the source generator always emits), honoring the ownership-transfer contract with zero
// copying. Any other IReadOnlyDictionary implementation is materialized once so the provider
// still owns a concrete instance. Values are always taken by reference.
private static Dictionary<TKey, TValue> AsOwnedDictionary<TKey, TValue>(IReadOnlyDictionary<TKey, TValue> source)
where TKey : notnull
{
if (source is Dictionary<TKey, TValue> concrete)
{
return concrete;
}

var copy = new Dictionary<TKey, TValue>(source.Count);
foreach (KeyValuePair<TKey, TValue> kvp in source)
{
copy[kvp.Key] = kvp.Value;
}

return copy;
}

private static readonly Dictionary<Type, Attribute[]> EmptyTypeAttributes = [];

private static readonly Dictionary<MethodInfo, Func<object?, object?[]?, object?>> EmptyMethodInvokers = [];
Expand Down