From 826bff9edbec76d7f78cbfc1aad68588fb84c953 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 16 Jul 2026 11:01:10 +0200 Subject: [PATCH 1/8] [Mono.Android] Rename SafeJavaCollectionFactory to SafeContainerConverterFactory Pure rename in preparation for the container converter redesign: the factory will grow per-container factories and own non-generic collection targets, so "Collection" no longer describes its scope. Also renames the entry point TryGetFromJniHandleConverter -> TryCreateConverter and updates the JavaConvert and ValueTypeFactory references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f9ec4f09-ffc5-4f66-99bf-ec40ea2728e8 --- src/Mono.Android/Java.Interop/JavaConvert.cs | 2 +- ...Factory.cs => SafeContainerConverterFactory.cs} | 6 +++--- src/Mono.Android/Java.Interop/ValueTypeFactory.cs | 14 +++++++------- 3 files changed, 11 insertions(+), 11 deletions(-) rename src/Mono.Android/Java.Interop/{SafeJavaCollectionFactory.cs => SafeContainerConverterFactory.cs} (98%) diff --git a/src/Mono.Android/Java.Interop/JavaConvert.cs b/src/Mono.Android/Java.Interop/JavaConvert.cs index c02a02e00fe..51bd11adb76 100644 --- a/src/Mono.Android/Java.Interop/JavaConvert.cs +++ b/src/Mono.Android/Java.Interop/JavaConvert.cs @@ -76,7 +76,7 @@ static class JavaConvert { if (target.IsGenericType && !target.IsGenericTypeDefinition) { if (RuntimeFeature.TrimmableTypeMap) { - if (SafeJavaCollectionFactory.TryGetFromJniHandleConverter (target, out var collectionConverter)) + if (SafeContainerConverterFactory.TryCreateConverter (target, out var collectionConverter)) return collectionConverter; } else if (System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported) { var factoryConverter = TryMakeGenericCollectionTypeFactory (target); diff --git a/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs b/src/Mono.Android/Java.Interop/SafeContainerConverterFactory.cs similarity index 98% rename from src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs rename to src/Mono.Android/Java.Interop/SafeContainerConverterFactory.cs index b26bbc2b5f1..305646ccc25 100644 --- a/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs +++ b/src/Mono.Android/Java.Interop/SafeContainerConverterFactory.cs @@ -9,7 +9,7 @@ namespace Java.Interop; -static class SafeJavaCollectionFactory +static class SafeContainerConverterFactory { internal const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; @@ -24,7 +24,7 @@ static class SafeJavaCollectionFactory // direct generic type references and constructors instead of asking MakeGenericType() to invent them. // The shared ValueTypeFactory map also roots the exact array vectors for these same value types. - internal static bool TryGetFromJniHandleConverter ( + internal static bool TryCreateConverter ( Type targetType, [NotNullWhen (true)] out Func? converter) { @@ -188,7 +188,7 @@ internal static Type MakeGenericType ( } [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", - Justification = "Activator.CreateInstance() targets only collection wrapper types produced by SafeJavaCollectionFactory. " + + Justification = "Activator.CreateInstance() targets only collection wrapper types produced by SafeContainerConverterFactory. " + "Reference-only wrappers use NativeAOT's canonical generic construction, exact value-type wrappers are rooted by ValueTypeFactory, " + "and mixed dictionaries root their reference/value canonical shapes with dedicated type tokens.")] [UnconditionalSuppressMessage ("Trimming", "IL2072:UnrecognizedReflectionPattern", diff --git a/src/Mono.Android/Java.Interop/ValueTypeFactory.cs b/src/Mono.Android/Java.Interop/ValueTypeFactory.cs index a2543d301d1..f8b0e2da7ad 100644 --- a/src/Mono.Android/Java.Interop/ValueTypeFactory.cs +++ b/src/Mono.Android/Java.Interop/ValueTypeFactory.cs @@ -53,13 +53,13 @@ abstract class ValueTypeFactory internal abstract IDictionary CreateDictionaryWithReferenceValue (Type valueType, IntPtr handle, JniHandleOwnership transfer); - internal abstract IDictionary CreateDictionaryWithKey<[DynamicallyAccessedMembers (SafeJavaCollectionFactory.Constructors)] TKey> ( + internal abstract IDictionary CreateDictionaryWithKey<[DynamicallyAccessedMembers (SafeContainerConverterFactory.Constructors)] TKey> ( ValueTypeFactory keyFactory, IntPtr handle, JniHandleOwnership transfer); } -sealed class ValueTypeFactory<[DynamicallyAccessedMembers (SafeJavaCollectionFactory.Constructors)] T> : ValueTypeFactory +sealed class ValueTypeFactory<[DynamicallyAccessedMembers (SafeContainerConverterFactory.Constructors)] T> : ValueTypeFactory { // These tokens root the mixed reference/value dictionary canonical shapes. For example, // JavaDictionary canonicalizes like JavaDictionary<__Canon,int>, so @@ -98,18 +98,18 @@ internal override IDictionary CreateDictionary (ValueTypeFactory valueFactory, I internal override IDictionary CreateDictionaryWithReferenceKey (Type keyType, IntPtr handle, JniHandleOwnership transfer) { _ = ReferenceKeyDictionaryType; - var dictionaryType = SafeJavaCollectionFactory.MakeGenericType (typeof (JavaDictionary<,>), [keyType, typeof (T)]); - return (IDictionary) SafeJavaCollectionFactory.CreateInstance (dictionaryType, handle, transfer); + var dictionaryType = SafeContainerConverterFactory.MakeGenericType (typeof (JavaDictionary<,>), [keyType, typeof (T)]); + return (IDictionary) SafeContainerConverterFactory.CreateInstance (dictionaryType, handle, transfer); } internal override IDictionary CreateDictionaryWithReferenceValue (Type valueType, IntPtr handle, JniHandleOwnership transfer) { _ = ReferenceValueDictionaryType; - var dictionaryType = SafeJavaCollectionFactory.MakeGenericType (typeof (JavaDictionary<,>), [typeof (T), valueType]); - return (IDictionary) SafeJavaCollectionFactory.CreateInstance (dictionaryType, handle, transfer); + var dictionaryType = SafeContainerConverterFactory.MakeGenericType (typeof (JavaDictionary<,>), [typeof (T), valueType]); + return (IDictionary) SafeContainerConverterFactory.CreateInstance (dictionaryType, handle, transfer); } - internal override IDictionary CreateDictionaryWithKey<[DynamicallyAccessedMembers (SafeJavaCollectionFactory.Constructors)] TKey> ( + internal override IDictionary CreateDictionaryWithKey<[DynamicallyAccessedMembers (SafeContainerConverterFactory.Constructors)] TKey> ( ValueTypeFactory keyFactory, IntPtr handle, JniHandleOwnership transfer) From 3db5f2288245e1703fb8fb1dfa94ebf15f055612 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 16 Jul 2026 11:11:05 +0200 Subject: [PATCH 2/8] [Mono.Android] Redesign SafeContainerConverterFactory with per-container factories Split the container converter into explicit, per-container factories (JavaListTypeFactory, JavaCollectionTypeFactory, JavaDictionaryTypeFactory) chained via TryCreateFromJniHandle(...) || ..., following the AOT prototype: - reference element/key/value arguments -> typeof(<>).MakeGenericType + Activator, riding NativeAOT's __Canon canonical templates; - mapped primitive/nullable arguments -> ValueTypeFactory, rooting the exact instantiation with a direct `new` (no reflection); - any other value type (custom struct) -> NotSupportedException, instead of the previous silent fall-through to a non-generic wrapper. TryCreateConverter now also owns non-generic JavaList/JavaCollection/JavaDictionary targets (and the raw IList/ICollection/IDictionary interfaces), delegating to the existing FromJniHandle helpers (direct construction, no reflection). JavaConvert's trimmable branch moves out of the IsGenericType guard so the factory is the single entry point for all Java collection construction on the trimmable path. NOTE: not yet built locally (requires the full framework); pending CI/local build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f9ec4f09-ffc5-4f66-99bf-ec40ea2728e8 --- src/Mono.Android/Java.Interop/JavaConvert.cs | 12 +- .../SafeContainerConverterFactory.cs | 298 ++++++++++-------- 2 files changed, 173 insertions(+), 137 deletions(-) diff --git a/src/Mono.Android/Java.Interop/JavaConvert.cs b/src/Mono.Android/Java.Interop/JavaConvert.cs index 51bd11adb76..c95a07b3146 100644 --- a/src/Mono.Android/Java.Interop/JavaConvert.cs +++ b/src/Mono.Android/Java.Interop/JavaConvert.cs @@ -74,11 +74,13 @@ static class JavaConvert { if (target.IsArray) return (h, t) => JNIEnv.GetArray (h, t, target.GetElementType ()); - if (target.IsGenericType && !target.IsGenericTypeDefinition) { - if (RuntimeFeature.TrimmableTypeMap) { - if (SafeContainerConverterFactory.TryCreateConverter (target, out var collectionConverter)) - return collectionConverter; - } else if (System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported) { + if (RuntimeFeature.TrimmableTypeMap) { + // The trimmable typemap owns all Java collection construction (generic and non-generic) + // without runtime code generation. + if (SafeContainerConverterFactory.TryCreateConverter (target, out var collectionConverter)) + return collectionConverter; + } else if (target.IsGenericType && !target.IsGenericTypeDefinition) { + if (System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported) { var factoryConverter = TryMakeGenericCollectionTypeFactory (target); if (factoryConverter != null) return factoryConverter; diff --git a/src/Mono.Android/Java.Interop/SafeContainerConverterFactory.cs b/src/Mono.Android/Java.Interop/SafeContainerConverterFactory.cs index 305646ccc25..1577782bd8e 100644 --- a/src/Mono.Android/Java.Interop/SafeContainerConverterFactory.cs +++ b/src/Mono.Android/Java.Interop/SafeContainerConverterFactory.cs @@ -1,5 +1,6 @@ #nullable enable using System; +using System.Collections; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Globalization; @@ -9,166 +10,99 @@ namespace Java.Interop; +// Produces the Java-handle-to-managed converters used by the trimmable typemap when the marshaling +// target is a Java collection wrapper (JavaList/JavaCollection/JavaDictionary or the IList/ICollection/ +// IDictionary interfaces they implement). +// +// NativeAOT's MakeGenericType() path eventually calls +// ExecutionEnvironment.TryGetConstructedGenericTypeForComponents(), then TypeBuilder.TryBuildGenericType(). +// The builder looks for a template by canonical form: reference type arguments canonicalize to __Canon, +// while value-type arguments stay value-specific. Consequently, JavaList can share the +// JavaList<__Canon> template, but JavaList and JavaList need exact rooted instantiations. +// +// The per-container factories below therefore split construction into explicit, non-overlapping paths: +// * reference element arguments -> typeof (JavaList<>).MakeGenericType (...) + Activator, riding the __Canon template; +// * mapped primitive/nullable -> ValueTypeFactory, which roots the exact instantiation with a direct `new`; +// * any other value type -> NotSupportedException (no reflection fallback is AOT-safe). static class SafeContainerConverterFactory { internal const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; - // NativeAOT's MakeGenericType() path eventually calls - // ExecutionEnvironment.TryGetConstructedGenericTypeForComponents(), then TypeBuilder.TryBuildGenericType(). - // The builder looks for a template by canonical form: reference type arguments canonicalize to __Canon, - // while value-type arguments stay value-specific. Consequently, JavaList can share the - // JavaList<__Canon> template, but JavaList and JavaList need exact rooted instantiations. - // - // These factories intentionally root the exact primitive/nullable Java collection instantiations through - // direct generic type references and constructors instead of asking MakeGenericType() to invent them. - // The shared ValueTypeFactory map also roots the exact array vectors for these same value types. + static readonly ISafeContainerTypeFactory ListFactory = new JavaListTypeFactory (); + static readonly ISafeContainerTypeFactory CollectionFactory = new JavaCollectionTypeFactory (); + static readonly ISafeContainerTypeFactory DictionaryFactory = new JavaDictionaryTypeFactory (); internal static bool TryCreateConverter ( Type targetType, [NotNullWhen (true)] out Func? converter) { - if (targetType == null) - throw new ArgumentNullException (nameof (targetType)); - - if (!TryGetCollectionShape (targetType, out var shape)) { - converter = null; - return false; - } + ArgumentNullException.ThrowIfNull (targetType); + + if (targetType.IsGenericType && !targetType.IsGenericTypeDefinition) { + var genericDefinition = targetType.GetGenericTypeDefinition (); + if (IsKnownContainerDefinition (genericDefinition)) { + // Capture the parsed arguments so GetGenericArguments () runs once when the converter is + // selected, rather than again for every conversion. + var arguments = targetType.GetGenericArguments (); + converter = (handle, transfer) => CreateFromJniHandle (genericDefinition, arguments, handle, transfer); + return true; + } - if (!IsSupportedCollectionShape (shape)) { converter = null; return false; } - // Capture the parsed shape so GetGenericArguments() runs once when the converter is - // selected, rather than once for the support gate and again for every conversion. - converter = (handle, transfer) => CreateFromJniHandle (shape, handle, transfer); - return true; + // Non-generic wrappers and raw collection interfaces construct their peer directly (no reflection) + // through the existing FromJniHandle helpers, which also reuse an already-registered managed peer. + return TryCreateNonGenericConverter (targetType, out converter); } - static bool IsSupportedCollectionShape (CollectionShape shape) - { - // Unsupported value-type arguments are rejected before any MakeGenericType() call: - // those would need an exact unrooted instantiation, whereas reference and mapped - // primitive/nullable arguments are all AOT-safe. - foreach (var argument in shape.Arguments) { - if (argument.IsValueType && !ValueTypeFactory.PrimitiveTypeFactories.ContainsKey (argument)) { - return false; - } - } - - return true; - } + static bool IsKnownContainerDefinition (Type genericDefinition) + => genericDefinition == typeof (IList<>) || genericDefinition == typeof (JavaList<>) + || genericDefinition == typeof (ICollection<>) || genericDefinition == typeof (JavaCollection<>) + || genericDefinition == typeof (IDictionary<,>) || genericDefinition == typeof (JavaDictionary<,>); - static object? CreateFromJniHandle ( - CollectionShape shape, - IntPtr handle, - JniHandleOwnership transfer) + static object? CreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer) { if (handle == IntPtr.Zero) { return null; } - if (TryCreateFromMappedValueTypeFactories (shape, handle, transfer, out var collection)) { - return collection; + object? result; + if (ListFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result) + || CollectionFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result) + || DictionaryFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result)) { + return result; } - return CreateInstance (GetClosedCollectionType (shape), handle, transfer); + throw new NotSupportedException ($"Unsupported Java container type with generic definition '{genericDefinition}'."); } - static bool TryGetCollectionShape (Type targetType, out CollectionShape shape) + static bool TryCreateNonGenericConverter ( + Type targetType, + [NotNullWhen (true)] out Func? converter) { - if (!targetType.IsGenericType || targetType.IsGenericTypeDefinition) { - shape = default; - return false; - } - - var genericDefinition = targetType.GetGenericTypeDefinition (); - if (genericDefinition == typeof (IList<>) || genericDefinition == typeof (JavaList<>)) { - shape = new CollectionShape (JavaCollectionKind.List, targetType.GetGenericArguments ()); + // Order mirrors the historical JavaConvert fallback: dictionary, then list, then collection. + if (typeof (IDictionary).IsAssignableFrom (targetType)) { + converter = static (handle, transfer) => JavaDictionary.FromJniHandle (handle, transfer); return true; } - if (genericDefinition == typeof (ICollection<>) || genericDefinition == typeof (JavaCollection<>)) { - shape = new CollectionShape (JavaCollectionKind.Collection, targetType.GetGenericArguments ()); + if (typeof (IList).IsAssignableFrom (targetType)) { + converter = static (handle, transfer) => JavaList.FromJniHandle (handle, transfer); return true; } - if (genericDefinition == typeof (IDictionary<,>) || genericDefinition == typeof (JavaDictionary<,>)) { - shape = new CollectionShape (JavaCollectionKind.Dictionary, targetType.GetGenericArguments ()); - return true; - } - - shape = default; - return false; - } - - static bool TryCreateFromMappedValueTypeFactories ( - CollectionShape shape, - IntPtr handle, - JniHandleOwnership transfer, - [NotNullWhen (true)] out object? collection) - { - if (shape.Kind == JavaCollectionKind.Dictionary) { - // Null when the argument is not a supported value type; the direct null checks below let the - // compiler track the non-null flow (an intermediate bool would not, producing CS8602/CS8604). - TryGetValueTypeFactory (shape.Arguments [0], out var keyFactory); - TryGetValueTypeFactory (shape.Arguments [1], out var valueFactory); - - if (keyFactory != null && valueFactory != null) { - collection = keyFactory.CreateDictionary (valueFactory, handle, transfer); - return true; - } - - // Mixed dictionaries are safe only when the other side is a reference type. If it is an - // unsupported value type, MakeGenericType() would need an exact unrooted instantiation. - if (keyFactory != null && !shape.Arguments [1].IsValueType) { - collection = keyFactory.CreateDictionaryWithReferenceValue (shape.Arguments [1], handle, transfer); - return true; - } - - if (valueFactory != null && !shape.Arguments [0].IsValueType) { - collection = valueFactory.CreateDictionaryWithReferenceKey (shape.Arguments [0], handle, transfer); - return true; - } - - collection = null; - return false; - } - - if (TryGetValueTypeFactory (shape.Arguments [0], out var factory)) { - collection = shape.Kind == JavaCollectionKind.List - ? factory.CreateList (handle, transfer) - : factory.CreateCollection (handle, transfer); + if (typeof (ICollection).IsAssignableFrom (targetType)) { + converter = static (handle, transfer) => JavaCollection.FromJniHandle (handle, transfer); return true; } - collection = null; - return false; - } - - static bool TryGetValueTypeFactory (Type type, [NotNullWhen (true)] out ValueTypeFactory? factory) - { - if (type.IsValueType) { - return ValueTypeFactory.PrimitiveTypeFactories.TryGetValue (type, out factory); - } - - factory = null; + converter = null; return false; } - [return: DynamicallyAccessedMembers (Constructors)] - static Type GetClosedCollectionType (CollectionShape shape) - { - return shape.Kind switch { - JavaCollectionKind.List => MakeGenericType (typeof (JavaList<>), shape.Arguments), - JavaCollectionKind.Collection => MakeGenericType (typeof (JavaCollection<>), shape.Arguments), - JavaCollectionKind.Dictionary => MakeGenericType (typeof (JavaDictionary<,>), shape.Arguments), - _ => throw new InvalidOperationException ($"Unsupported Java collection kind '{shape.Kind}'."), - }; - } - [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", Justification = "NativeAOT's Type.MakeGenericType() is annotated because arbitrary constructed generics can lack a runtime template. " + "Callers of this helper restrict the shape to Android.Runtime Java collection wrappers. Reference arguments use NativeAOT's __Canon generic templates. " + @@ -192,7 +126,7 @@ internal static Type MakeGenericType ( "Reference-only wrappers use NativeAOT's canonical generic construction, exact value-type wrappers are rooted by ValueTypeFactory, " + "and mixed dictionaries root their reference/value canonical shapes with dedicated type tokens.")] [UnconditionalSuppressMessage ("Trimming", "IL2072:UnrecognizedReflectionPattern", - Justification = "The collection type is annotated with DynamicallyAccessedMembers(Constructors) by GetClosedCollectionType/MakeGenericType. " + + Justification = "The collection type is annotated with DynamicallyAccessedMembers(Constructors) by the per-container factories and MakeGenericType. " + "Only the known JavaList, JavaCollection, and JavaDictionary constructors are invoked here.")] internal static object CreateInstance ([DynamicallyAccessedMembers (Constructors)] Type collectionType, params object?[] arguments) { @@ -207,24 +141,124 @@ internal static object CreateInstance ([DynamicallyAccessedMembers (Constructors } return instance; } +} - readonly struct CollectionShape +// One factory per Java collection container. Each factory owns a single container type so that the +// reference-vs-value construction paths stay explicit and statically analyzable (correctness over reuse). +interface ISafeContainerTypeFactory +{ + // Returns false when genericDefinition is not this factory's container, so callers can chain the + // factories with `||`. Returns true (and sets result) once the matching container is constructed; + // an unsupported (non-primitive) value-type argument throws instead of falling through. + bool TryCreateFromJniHandle ( + Type genericDefinition, + Type[] arguments, + IntPtr handle, + JniHandleOwnership transfer, + out object? result); +} + +sealed class JavaListTypeFactory : ISafeContainerTypeFactory +{ + public bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) { - public CollectionShape (JavaCollectionKind kind, Type[] arguments) - { - Kind = kind; - Arguments = arguments; + if (genericDefinition != typeof (IList<>) && genericDefinition != typeof (JavaList<>)) { + result = null; + return false; } - public JavaCollectionKind Kind { get; } + var elementType = arguments [0]; + if (elementType.IsValueType) { + if (!ValueTypeFactory.PrimitiveTypeFactories.TryGetValue (elementType, out var valueFactory)) { + throw new NotSupportedException ($"'JavaList<{elementType}>' is not available on the trimmable typemap: only reference and mapped primitive element types are supported."); + } + result = valueFactory.CreateList (handle, transfer); + return true; + } - public Type[] Arguments { get; } + result = SafeContainerConverterFactory.CreateInstance ( + SafeContainerConverterFactory.MakeGenericType (typeof (JavaList<>), [elementType]), + handle, transfer); + return true; + } +} + +sealed class JavaCollectionTypeFactory : ISafeContainerTypeFactory +{ + public bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) + { + if (genericDefinition != typeof (ICollection<>) && genericDefinition != typeof (JavaCollection<>)) { + result = null; + return false; + } + + var elementType = arguments [0]; + if (elementType.IsValueType) { + if (!ValueTypeFactory.PrimitiveTypeFactories.TryGetValue (elementType, out var valueFactory)) { + throw new NotSupportedException ($"'JavaCollection<{elementType}>' is not available on the trimmable typemap: only reference and mapped primitive element types are supported."); + } + result = valueFactory.CreateCollection (handle, transfer); + return true; + } + + result = SafeContainerConverterFactory.CreateInstance ( + SafeContainerConverterFactory.MakeGenericType (typeof (JavaCollection<>), [elementType]), + handle, transfer); + return true; } +} + +sealed class JavaDictionaryTypeFactory : ISafeContainerTypeFactory +{ + public bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) + { + if (genericDefinition != typeof (IDictionary<,>) && genericDefinition != typeof (JavaDictionary<,>)) { + result = null; + return false; + } + + var keyType = arguments [0]; + var valueType = arguments [1]; - enum JavaCollectionKind { - List, - Collection, - Dictionary, + // A value-type argument is only AOT-safe when it is a mapped primitive/nullable; anything else + // (a custom struct) has no rooted instantiation and must not fall back to reflection. + var keyFactory = GetValueTypeFactoryOrThrow (keyType); + var valueFactory = GetValueTypeFactoryOrThrow (valueType); + + if (keyFactory != null && valueFactory != null) { + result = keyFactory.CreateDictionary (valueFactory, handle, transfer); + return true; + } + + // Mixed value/reference dictionaries root JavaDictionary / JavaDictionary<__Canon,value> + // through the value factory's dedicated type tokens. + if (keyFactory != null) { + result = keyFactory.CreateDictionaryWithReferenceValue (valueType, handle, transfer); + return true; + } + + if (valueFactory != null) { + result = valueFactory.CreateDictionaryWithReferenceKey (keyType, handle, transfer); + return true; + } + + // Both arguments are reference types: JavaDictionary rides the __Canon template. + result = SafeContainerConverterFactory.CreateInstance ( + SafeContainerConverterFactory.MakeGenericType (typeof (JavaDictionary<,>), [keyType, valueType]), + handle, transfer); + return true; } + static ValueTypeFactory? GetValueTypeFactoryOrThrow (Type argument) + { + if (!argument.IsValueType) { + return null; + } + + if (!ValueTypeFactory.PrimitiveTypeFactories.TryGetValue (argument, out var factory)) { + throw new NotSupportedException ($"'JavaDictionary' with value-type argument '{argument}' is not available on the trimmable typemap: only reference and mapped primitive arguments are supported."); + } + + return factory; + } } From b5b95613cc4c25885d20191c17a4ca0898c07647 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 16 Jul 2026 12:22:16 +0200 Subject: [PATCH 3/8] [Mono.Android] Update csproj for SafeContainerConverterFactory rename The explicit item still referenced the old SafeJavaCollectionFactory.cs filename, breaking the build with CS2001. Point it at the renamed SafeContainerConverterFactory.cs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f9ec4f09-ffc5-4f66-99bf-ec40ea2728e8 --- src/Mono.Android/Mono.Android.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mono.Android/Mono.Android.csproj b/src/Mono.Android/Mono.Android.csproj index 25aee38cdf3..8d86df9f9e8 100644 --- a/src/Mono.Android/Mono.Android.csproj +++ b/src/Mono.Android/Mono.Android.csproj @@ -325,7 +325,7 @@ - + From 1737fdb49e94543b94a200a34ebb3f77230c0ae2 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 16 Jul 2026 13:07:59 +0200 Subject: [PATCH 4/8] [Mono.Android] Refactor container factory into abstract base class Convert ISafeContainerTypeFactory into an abstract SafeContainerTypeFactory base class and move the MakeGenericType and CreateInstance reflection helpers (with their trimming/AOT suppressions) onto it as protected members, so the per-container factories use them via inheritance instead of sharing them through the entry-point class. MakeGenericType now takes a concrete reference-argument wrapper exemplar (e.g. JavaList) whose DynamicallyAccessedMembers annotation roots the __Canon template, making the AOT safety obvious. The dishonest [return: DAM] is dropped in favor of scoping the constructor suppression to CreateInstance. ValueTypeFactory's mixed reference/value dictionaries adopt the same exemplar pattern, removing the dedicated rooting-token fields. Also rename the dictionary GetValueTypeFactoryOrThrow helper to a pure TryGetPrimitiveValueTypeFactory Try-pattern method, with an up-front EnsureReferenceOrPrimitive validation, and revert the entry-point class name back to SafeJavaCollectionFactory to reduce churn. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 50ab60e5-5b4d-4e5f-abd2-2878d87a10c9 --- src/Mono.Android/Java.Interop/JavaConvert.cs | 2 +- .../SafeContainerConverterFactory.cs | 264 --------------- .../Java.Interop/SafeJavaCollectionFactory.cs | 301 ++++++++++++++++++ .../Java.Interop/ValueTypeFactory.cs | 68 +++- src/Mono.Android/Mono.Android.csproj | 2 +- 5 files changed, 356 insertions(+), 281 deletions(-) delete mode 100644 src/Mono.Android/Java.Interop/SafeContainerConverterFactory.cs create mode 100644 src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs diff --git a/src/Mono.Android/Java.Interop/JavaConvert.cs b/src/Mono.Android/Java.Interop/JavaConvert.cs index c95a07b3146..79d4cb9efbf 100644 --- a/src/Mono.Android/Java.Interop/JavaConvert.cs +++ b/src/Mono.Android/Java.Interop/JavaConvert.cs @@ -77,7 +77,7 @@ static class JavaConvert { if (RuntimeFeature.TrimmableTypeMap) { // The trimmable typemap owns all Java collection construction (generic and non-generic) // without runtime code generation. - if (SafeContainerConverterFactory.TryCreateConverter (target, out var collectionConverter)) + if (SafeJavaCollectionFactory.TryCreateConverter (target, out var collectionConverter)) return collectionConverter; } else if (target.IsGenericType && !target.IsGenericTypeDefinition) { if (System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported) { diff --git a/src/Mono.Android/Java.Interop/SafeContainerConverterFactory.cs b/src/Mono.Android/Java.Interop/SafeContainerConverterFactory.cs deleted file mode 100644 index 1577782bd8e..00000000000 --- a/src/Mono.Android/Java.Interop/SafeContainerConverterFactory.cs +++ /dev/null @@ -1,264 +0,0 @@ -#nullable enable -using System; -using System.Collections; -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; -using System.Globalization; -using System.Reflection; - -using Android.Runtime; - -namespace Java.Interop; - -// Produces the Java-handle-to-managed converters used by the trimmable typemap when the marshaling -// target is a Java collection wrapper (JavaList/JavaCollection/JavaDictionary or the IList/ICollection/ -// IDictionary interfaces they implement). -// -// NativeAOT's MakeGenericType() path eventually calls -// ExecutionEnvironment.TryGetConstructedGenericTypeForComponents(), then TypeBuilder.TryBuildGenericType(). -// The builder looks for a template by canonical form: reference type arguments canonicalize to __Canon, -// while value-type arguments stay value-specific. Consequently, JavaList can share the -// JavaList<__Canon> template, but JavaList and JavaList need exact rooted instantiations. -// -// The per-container factories below therefore split construction into explicit, non-overlapping paths: -// * reference element arguments -> typeof (JavaList<>).MakeGenericType (...) + Activator, riding the __Canon template; -// * mapped primitive/nullable -> ValueTypeFactory, which roots the exact instantiation with a direct `new`; -// * any other value type -> NotSupportedException (no reflection fallback is AOT-safe). -static class SafeContainerConverterFactory -{ - internal const DynamicallyAccessedMemberTypes Constructors = - DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; - - static readonly ISafeContainerTypeFactory ListFactory = new JavaListTypeFactory (); - static readonly ISafeContainerTypeFactory CollectionFactory = new JavaCollectionTypeFactory (); - static readonly ISafeContainerTypeFactory DictionaryFactory = new JavaDictionaryTypeFactory (); - - internal static bool TryCreateConverter ( - Type targetType, - [NotNullWhen (true)] out Func? converter) - { - ArgumentNullException.ThrowIfNull (targetType); - - if (targetType.IsGenericType && !targetType.IsGenericTypeDefinition) { - var genericDefinition = targetType.GetGenericTypeDefinition (); - if (IsKnownContainerDefinition (genericDefinition)) { - // Capture the parsed arguments so GetGenericArguments () runs once when the converter is - // selected, rather than again for every conversion. - var arguments = targetType.GetGenericArguments (); - converter = (handle, transfer) => CreateFromJniHandle (genericDefinition, arguments, handle, transfer); - return true; - } - - converter = null; - return false; - } - - // Non-generic wrappers and raw collection interfaces construct their peer directly (no reflection) - // through the existing FromJniHandle helpers, which also reuse an already-registered managed peer. - return TryCreateNonGenericConverter (targetType, out converter); - } - - static bool IsKnownContainerDefinition (Type genericDefinition) - => genericDefinition == typeof (IList<>) || genericDefinition == typeof (JavaList<>) - || genericDefinition == typeof (ICollection<>) || genericDefinition == typeof (JavaCollection<>) - || genericDefinition == typeof (IDictionary<,>) || genericDefinition == typeof (JavaDictionary<,>); - - static object? CreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer) - { - if (handle == IntPtr.Zero) { - return null; - } - - object? result; - if (ListFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result) - || CollectionFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result) - || DictionaryFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result)) { - return result; - } - - throw new NotSupportedException ($"Unsupported Java container type with generic definition '{genericDefinition}'."); - } - - static bool TryCreateNonGenericConverter ( - Type targetType, - [NotNullWhen (true)] out Func? converter) - { - // Order mirrors the historical JavaConvert fallback: dictionary, then list, then collection. - if (typeof (IDictionary).IsAssignableFrom (targetType)) { - converter = static (handle, transfer) => JavaDictionary.FromJniHandle (handle, transfer); - return true; - } - - if (typeof (IList).IsAssignableFrom (targetType)) { - converter = static (handle, transfer) => JavaList.FromJniHandle (handle, transfer); - return true; - } - - if (typeof (ICollection).IsAssignableFrom (targetType)) { - converter = static (handle, transfer) => JavaCollection.FromJniHandle (handle, transfer); - return true; - } - - converter = null; - return false; - } - - [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", - Justification = "NativeAOT's Type.MakeGenericType() is annotated because arbitrary constructed generics can lack a runtime template. " + - "Callers of this helper restrict the shape to Android.Runtime Java collection wrappers. Reference arguments use NativeAOT's __Canon generic templates. " + - "Value-type arguments are either rejected or handled by explicit primitive/nullable factories that root the exact instantiation. " + - "Mixed reference/value dictionaries additionally root JavaDictionary<__Canon,T> or JavaDictionary through dedicated type tokens.")] - [UnconditionalSuppressMessage ("Trimming", "IL2055:MakeGenericType", - Justification = "The generic type definitions are known Java collection wrappers, not arbitrary user types. " + - "The constructed wrapper constructors are preserved by the return annotation and by the explicit value-type factory references. " + - "The generic element arguments are not activated by this helper; element peer creation still goes through the normal JavaConvert/trimmable typemap path.")] - [return: DynamicallyAccessedMembers (Constructors)] - internal static Type MakeGenericType ( - [DynamicallyAccessedMembers (Constructors)] - Type genericTypeDefinition, - Type[] arguments) - { - return genericTypeDefinition.MakeGenericType (arguments); - } - - [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", - Justification = "Activator.CreateInstance() targets only collection wrapper types produced by SafeContainerConverterFactory. " + - "Reference-only wrappers use NativeAOT's canonical generic construction, exact value-type wrappers are rooted by ValueTypeFactory, " + - "and mixed dictionaries root their reference/value canonical shapes with dedicated type tokens.")] - [UnconditionalSuppressMessage ("Trimming", "IL2072:UnrecognizedReflectionPattern", - Justification = "The collection type is annotated with DynamicallyAccessedMembers(Constructors) by the per-container factories and MakeGenericType. " + - "Only the known JavaList, JavaCollection, and JavaDictionary constructors are invoked here.")] - internal static object CreateInstance ([DynamicallyAccessedMembers (Constructors)] Type collectionType, params object?[] arguments) - { - var instance = Activator.CreateInstance ( - collectionType, - BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, - binder: null, - args: arguments, - culture: CultureInfo.InvariantCulture); - if (instance == null) { - throw new InvalidOperationException ($"Unable to create an instance of collection type '{collectionType}'."); - } - return instance; - } -} - -// One factory per Java collection container. Each factory owns a single container type so that the -// reference-vs-value construction paths stay explicit and statically analyzable (correctness over reuse). -interface ISafeContainerTypeFactory -{ - // Returns false when genericDefinition is not this factory's container, so callers can chain the - // factories with `||`. Returns true (and sets result) once the matching container is constructed; - // an unsupported (non-primitive) value-type argument throws instead of falling through. - bool TryCreateFromJniHandle ( - Type genericDefinition, - Type[] arguments, - IntPtr handle, - JniHandleOwnership transfer, - out object? result); -} - -sealed class JavaListTypeFactory : ISafeContainerTypeFactory -{ - public bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) - { - if (genericDefinition != typeof (IList<>) && genericDefinition != typeof (JavaList<>)) { - result = null; - return false; - } - - var elementType = arguments [0]; - if (elementType.IsValueType) { - if (!ValueTypeFactory.PrimitiveTypeFactories.TryGetValue (elementType, out var valueFactory)) { - throw new NotSupportedException ($"'JavaList<{elementType}>' is not available on the trimmable typemap: only reference and mapped primitive element types are supported."); - } - result = valueFactory.CreateList (handle, transfer); - return true; - } - - result = SafeContainerConverterFactory.CreateInstance ( - SafeContainerConverterFactory.MakeGenericType (typeof (JavaList<>), [elementType]), - handle, transfer); - return true; - } -} - -sealed class JavaCollectionTypeFactory : ISafeContainerTypeFactory -{ - public bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) - { - if (genericDefinition != typeof (ICollection<>) && genericDefinition != typeof (JavaCollection<>)) { - result = null; - return false; - } - - var elementType = arguments [0]; - if (elementType.IsValueType) { - if (!ValueTypeFactory.PrimitiveTypeFactories.TryGetValue (elementType, out var valueFactory)) { - throw new NotSupportedException ($"'JavaCollection<{elementType}>' is not available on the trimmable typemap: only reference and mapped primitive element types are supported."); - } - result = valueFactory.CreateCollection (handle, transfer); - return true; - } - - result = SafeContainerConverterFactory.CreateInstance ( - SafeContainerConverterFactory.MakeGenericType (typeof (JavaCollection<>), [elementType]), - handle, transfer); - return true; - } -} - -sealed class JavaDictionaryTypeFactory : ISafeContainerTypeFactory -{ - public bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) - { - if (genericDefinition != typeof (IDictionary<,>) && genericDefinition != typeof (JavaDictionary<,>)) { - result = null; - return false; - } - - var keyType = arguments [0]; - var valueType = arguments [1]; - - // A value-type argument is only AOT-safe when it is a mapped primitive/nullable; anything else - // (a custom struct) has no rooted instantiation and must not fall back to reflection. - var keyFactory = GetValueTypeFactoryOrThrow (keyType); - var valueFactory = GetValueTypeFactoryOrThrow (valueType); - - if (keyFactory != null && valueFactory != null) { - result = keyFactory.CreateDictionary (valueFactory, handle, transfer); - return true; - } - - // Mixed value/reference dictionaries root JavaDictionary / JavaDictionary<__Canon,value> - // through the value factory's dedicated type tokens. - if (keyFactory != null) { - result = keyFactory.CreateDictionaryWithReferenceValue (valueType, handle, transfer); - return true; - } - - if (valueFactory != null) { - result = valueFactory.CreateDictionaryWithReferenceKey (keyType, handle, transfer); - return true; - } - - // Both arguments are reference types: JavaDictionary rides the __Canon template. - result = SafeContainerConverterFactory.CreateInstance ( - SafeContainerConverterFactory.MakeGenericType (typeof (JavaDictionary<,>), [keyType, valueType]), - handle, transfer); - return true; - } - - static ValueTypeFactory? GetValueTypeFactoryOrThrow (Type argument) - { - if (!argument.IsValueType) { - return null; - } - - if (!ValueTypeFactory.PrimitiveTypeFactories.TryGetValue (argument, out var factory)) { - throw new NotSupportedException ($"'JavaDictionary' with value-type argument '{argument}' is not available on the trimmable typemap: only reference and mapped primitive arguments are supported."); - } - - return factory; - } -} diff --git a/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs b/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs new file mode 100644 index 00000000000..5e653b2c6a5 --- /dev/null +++ b/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs @@ -0,0 +1,301 @@ +#nullable enable +using System; +using System.Collections; +using System.Collections.Generic; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using System.Reflection; + +using Android.Runtime; + +namespace Java.Interop; + +/// +/// Produces the Java-handle-to-managed converters used by the trimmable typemap when the marshaling +/// target is a Java collection wrapper (, , +/// , or the // +/// interfaces they implement). +/// +/// +/// NativeAOT's path eventually calls +/// ExecutionEnvironment.TryGetConstructedGenericTypeForComponents(), then +/// TypeBuilder.TryBuildGenericType(). The builder looks for a template by canonical form: +/// reference type arguments canonicalize to __Canon, while value-type arguments stay +/// value-specific. Consequently, JavaList<string> can share the JavaList<__Canon> +/// template, but JavaList<int> and JavaList<int?> need exact rooted instantiations. +/// +/// The per-container factories therefore split construction into explicit, non-overlapping paths: +/// +/// +/// reference element arguments ride the __Canon template via +/// plus +/// ; +/// mapped primitive/nullable arguments go through , +/// which roots the exact instantiation with a direct new; +/// any other value type throws (no reflection +/// fallback is AOT-safe). +/// +/// +static class SafeJavaCollectionFactory +{ + internal const DynamicallyAccessedMemberTypes Constructors = + DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; + + static readonly SafeContainerTypeFactory ListFactory = new JavaListTypeFactory (); + static readonly SafeContainerTypeFactory CollectionFactory = new JavaCollectionTypeFactory (); + static readonly SafeContainerTypeFactory DictionaryFactory = new JavaDictionaryTypeFactory (); + + internal static bool TryCreateConverter ( + Type targetType, + [NotNullWhen (true)] out Func? converter) + { + ArgumentNullException.ThrowIfNull (targetType); + + if (targetType.IsGenericType && !targetType.IsGenericTypeDefinition) { + var genericDefinition = targetType.GetGenericTypeDefinition (); + if (IsKnownContainerDefinition (genericDefinition)) { + // Capture the parsed arguments so GetGenericArguments () runs once when the converter is + // selected, rather than again for every conversion. + var arguments = targetType.GetGenericArguments (); + converter = (handle, transfer) => CreateFromJniHandle (genericDefinition, arguments, handle, transfer); + return true; + } + + converter = null; + return false; + } + + // Non-generic wrappers and raw collection interfaces construct their peer directly (no reflection) + // through the existing FromJniHandle helpers, which also reuse an already-registered managed peer. + return TryCreateNonGenericConverter (targetType, out converter); + } + + static bool IsKnownContainerDefinition (Type genericDefinition) + => genericDefinition == typeof (IList<>) || genericDefinition == typeof (JavaList<>) + || genericDefinition == typeof (ICollection<>) || genericDefinition == typeof (JavaCollection<>) + || genericDefinition == typeof (IDictionary<,>) || genericDefinition == typeof (JavaDictionary<,>); + + static object? CreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer) + { + if (handle == IntPtr.Zero) { + return null; + } + + object? result; + if (ListFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result) + || CollectionFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result) + || DictionaryFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result)) { + return result; + } + + throw new NotSupportedException ($"Unsupported Java container type with generic definition '{genericDefinition}'."); + } + + static bool TryCreateNonGenericConverter ( + Type targetType, + [NotNullWhen (true)] out Func? converter) + { + // Order mirrors the historical JavaConvert fallback: dictionary, then list, then collection. + if (typeof (IDictionary).IsAssignableFrom (targetType)) { + converter = static (handle, transfer) => JavaDictionary.FromJniHandle (handle, transfer); + return true; + } + + if (typeof (IList).IsAssignableFrom (targetType)) { + converter = static (handle, transfer) => JavaList.FromJniHandle (handle, transfer); + return true; + } + + if (typeof (ICollection).IsAssignableFrom (targetType)) { + converter = static (handle, transfer) => JavaCollection.FromJniHandle (handle, transfer); + return true; + } + + converter = null; + return false; + } +} + +/// +/// Base class for the per-container factories. One factory owns a single Java collection container +/// so that the reference-vs-value construction paths stay explicit and statically analyzable +/// (correctness over reuse). The reflection-based construction helpers live here (rather than being +/// shared through ) so their trimming/AOT suppressions are +/// scoped to the factories that actually build the wrapper types. +/// +abstract class SafeContainerTypeFactory +{ + /// + /// Constructs the managed wrapper for a Java collection handle. + /// + /// + /// when is not this factory's container, + /// so callers can chain the factories with ||. (and + /// set) once the matching container is constructed; an unsupported (non-primitive) value-type argument + /// throws instead of falling through. + /// + public abstract bool TryCreateFromJniHandle ( + Type genericDefinition, + Type[] arguments, + IntPtr handle, + JniHandleOwnership transfer, + out object? result); + + /// + /// Closes the generic definition of the reference-type collection wrapper + /// over . + /// + /// + /// A concrete reference-argument instantiation of the wrapper (for example + /// JavaList<IJavaPeerable>). Its roots the + /// wrapper's constructors on the __Canon template, which every reference-argument instantiation + /// (such as JavaList<string>) shares. This makes the AOT safety of the construction obvious: + /// the exact template the runtime resolves for is the one already rooted here. + /// + /// The reference-type generic arguments to close the definition over. + [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", + Justification = "NativeAOT's Type.MakeGenericType() is annotated because arbitrary constructed generics can lack a runtime template. " + + "This helper only closes the generic definition of the reference-argument wrapper T (e.g. JavaList) over reference-type arguments, " + + "which all canonicalize to the same __Canon template that T already roots. Value-type arguments never reach this helper; " + + "they are rejected or built by ValueTypeFactory, which roots the exact instantiation.")] + [UnconditionalSuppressMessage ("Trimming", "IL2055:MakeGenericType", + Justification = "The generic type definition comes from a known Java collection wrapper T, not an arbitrary user type, and the arguments are reference types. " + + "The constructed wrapper shares the __Canon template of T, whose constructors are rooted by the DynamicallyAccessedMembers(Constructors) annotation on T. " + + "The generic element arguments are not activated by this helper; element peer creation still goes through the normal JavaConvert/trimmable typemap path.")] + protected static Type MakeGenericType<[DynamicallyAccessedMembers (SafeJavaCollectionFactory.Constructors)] T> (Type[] arguments) + { + Debug.Assert (typeof (T).IsGenericType && typeof (T) != typeof (T).GetGenericTypeDefinition ()); + foreach (var argument in arguments) { + Debug.Assert (!argument.IsValueType); + } + + return typeof (T).GetGenericTypeDefinition ().MakeGenericType (arguments); + } + + /// + /// Activates using its (IntPtr, JniHandleOwnership) constructor. + /// + /// + /// is intentionally not annotated with + /// : the only supported callers construct it with + /// , whose T already roots the wrapper constructors on the + /// __Canon template. The suppressions below capture that guarantee rather than a static annotation. + /// + [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", + Justification = "Activator.CreateInstance() targets only collection wrapper types produced by the per-container factories. " + + "Reference-argument wrappers use NativeAOT's canonical generic construction rooted by MakeGenericType, " + + "and exact value-type wrappers are rooted by ValueTypeFactory.")] + [UnconditionalSuppressMessage ("Trimming", "IL2067:UnrecognizedReflectionPattern", + Justification = "collectionType is a JavaList, JavaCollection, or JavaDictionary produced by MakeGenericType, " + + "whose T (a reference-argument wrapper instantiation) roots the wrapper constructors on the __Canon template. " + + "Only that activation constructor is invoked here.")] + protected static object CreateInstance (Type collectionType, params object?[] arguments) + { + var instance = Activator.CreateInstance ( + collectionType, + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, + binder: null, + args: arguments, + culture: CultureInfo.InvariantCulture); + if (instance == null) { + throw new InvalidOperationException ($"Unable to create an instance of collection type '{collectionType}'."); + } + return instance; + } +} + +sealed class JavaListTypeFactory : SafeContainerTypeFactory +{ + public override bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) + { + if (genericDefinition != typeof (IList<>) && genericDefinition != typeof (JavaList<>)) { + result = null; + return false; + } + + var elementType = arguments [0]; + if (elementType.IsValueType) { + if (!ValueTypeFactory.PrimitiveTypeFactories.TryGetValue (elementType, out var valueFactory)) { + throw new NotSupportedException ($"'JavaList<{elementType}>' is not available on the trimmable typemap: only reference and mapped primitive element types are supported."); + } + result = valueFactory.CreateList (handle, transfer); + return true; + } + + result = CreateInstance (MakeGenericType> ([elementType]), handle, transfer); + return true; + } +} + +sealed class JavaCollectionTypeFactory : SafeContainerTypeFactory +{ + public override bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) + { + if (genericDefinition != typeof (ICollection<>) && genericDefinition != typeof (JavaCollection<>)) { + result = null; + return false; + } + + var elementType = arguments [0]; + if (elementType.IsValueType) { + if (!ValueTypeFactory.PrimitiveTypeFactories.TryGetValue (elementType, out var valueFactory)) { + throw new NotSupportedException ($"'JavaCollection<{elementType}>' is not available on the trimmable typemap: only reference and mapped primitive element types are supported."); + } + result = valueFactory.CreateCollection (handle, transfer); + return true; + } + + result = CreateInstance (MakeGenericType> ([elementType]), handle, transfer); + return true; + } +} + +sealed class JavaDictionaryTypeFactory : SafeContainerTypeFactory +{ + public override bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) + { + if (genericDefinition != typeof (IDictionary<,>) && genericDefinition != typeof (JavaDictionary<,>)) { + result = null; + return false; + } + + var keyType = arguments [0]; + var valueType = arguments [1]; + + // A value-type argument is only AOT-safe when it is a mapped primitive/nullable; anything else + // (a custom struct) has no rooted instantiation and must not fall back to reflection. Validate both + // arguments up front so the construction paths below only deal with reference or mapped-primitive types. + EnsureReferenceOrPrimitive (keyType); + EnsureReferenceOrPrimitive (valueType); + + if (TryGetPrimitiveValueTypeFactory (keyType, out var keyFactory)) { + // The key is a mapped primitive. A value/value dictionary uses the full rooted cross-product; + // a value/reference dictionary roots JavaDictionary via the value factory's token. + result = TryGetPrimitiveValueTypeFactory (valueType, out var valueFactory) + ? keyFactory.CreateDictionary (valueFactory, handle, transfer) + : keyFactory.CreateDictionaryWithReferenceValue (valueType, handle, transfer); + return true; + } + + if (TryGetPrimitiveValueTypeFactory (valueType, out var referenceKeyValueFactory)) { + // The key is a reference type and the value is a mapped primitive: root JavaDictionary<__Canon,value>. + result = referenceKeyValueFactory.CreateDictionaryWithReferenceKey (keyType, handle, transfer); + return true; + } + + // Both arguments are reference types: JavaDictionary rides the __Canon template. + result = CreateInstance (MakeGenericType> ([keyType, valueType]), handle, transfer); + return true; + } + + static void EnsureReferenceOrPrimitive (Type argument) + { + if (argument.IsValueType && !ValueTypeFactory.PrimitiveTypeFactories.ContainsKey (argument)) { + throw new NotSupportedException ($"'JavaDictionary' with value-type argument '{argument}' is not available on the trimmable typemap: only reference and mapped primitive arguments are supported."); + } + } + + static bool TryGetPrimitiveValueTypeFactory (Type argument, [NotNullWhen (true)] out ValueTypeFactory? factory) + => ValueTypeFactory.PrimitiveTypeFactories.TryGetValue (argument, out factory); +} diff --git a/src/Mono.Android/Java.Interop/ValueTypeFactory.cs b/src/Mono.Android/Java.Interop/ValueTypeFactory.cs index f8b0e2da7ad..f08c2f4e27a 100644 --- a/src/Mono.Android/Java.Interop/ValueTypeFactory.cs +++ b/src/Mono.Android/Java.Interop/ValueTypeFactory.cs @@ -2,7 +2,10 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using System.Reflection; using Android.Runtime; @@ -53,20 +56,14 @@ abstract class ValueTypeFactory internal abstract IDictionary CreateDictionaryWithReferenceValue (Type valueType, IntPtr handle, JniHandleOwnership transfer); - internal abstract IDictionary CreateDictionaryWithKey<[DynamicallyAccessedMembers (SafeContainerConverterFactory.Constructors)] TKey> ( + internal abstract IDictionary CreateDictionaryWithKey<[DynamicallyAccessedMembers (SafeJavaCollectionFactory.Constructors)] TKey> ( ValueTypeFactory keyFactory, IntPtr handle, JniHandleOwnership transfer); } -sealed class ValueTypeFactory<[DynamicallyAccessedMembers (SafeContainerConverterFactory.Constructors)] T> : ValueTypeFactory +sealed class ValueTypeFactory<[DynamicallyAccessedMembers (SafeJavaCollectionFactory.Constructors)] T> : ValueTypeFactory { - // These tokens root the mixed reference/value dictionary canonical shapes. For example, - // JavaDictionary canonicalizes like JavaDictionary<__Canon,int>, so - // JavaDictionary supplies the NativeAOT template when T is a mapped value type. - static readonly Type ReferenceKeyDictionaryType = typeof (JavaDictionary); - static readonly Type ReferenceValueDictionaryType = typeof (JavaDictionary); - internal ValueTypeFactory () { } @@ -97,19 +94,60 @@ internal override IDictionary CreateDictionary (ValueTypeFactory valueFactory, I internal override IDictionary CreateDictionaryWithReferenceKey (Type keyType, IntPtr handle, JniHandleOwnership transfer) { - _ = ReferenceKeyDictionaryType; - var dictionaryType = SafeContainerConverterFactory.MakeGenericType (typeof (JavaDictionary<,>), [keyType, typeof (T)]); - return (IDictionary) SafeContainerConverterFactory.CreateInstance (dictionaryType, handle, transfer); + // JavaDictionary canonicalizes like JavaDictionary<__Canon, T>; the + // JavaDictionary exemplar roots that template with its constructors. + return CreateReferenceMixedDictionary> ([keyType, typeof (T)], handle, transfer); } internal override IDictionary CreateDictionaryWithReferenceValue (Type valueType, IntPtr handle, JniHandleOwnership transfer) { - _ = ReferenceValueDictionaryType; - var dictionaryType = SafeContainerConverterFactory.MakeGenericType (typeof (JavaDictionary<,>), [typeof (T), valueType]); - return (IDictionary) SafeContainerConverterFactory.CreateInstance (dictionaryType, handle, transfer); + // JavaDictionary canonicalizes like JavaDictionary; the + // JavaDictionary exemplar roots that template with its constructors. + return CreateReferenceMixedDictionary> ([typeof (T), valueType], handle, transfer); + } + + /// + /// Builds a mixed reference/value whose reference argument rides the + /// __Canon template rooted by the instantiation. + /// + /// + /// The JavaDictionary<IJavaPeerable, T> / JavaDictionary<T, IJavaPeerable> exemplar. + /// Its roots the constructors of the canonical + /// template that resolves to (the exact-value argument stays value-specific). + /// + /// The [key, value] generic arguments, exactly one of which is a reference type. + [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", + Justification = "NativeAOT's Type.MakeGenericType() and Activator.CreateInstance() are annotated because arbitrary constructed generics can lack a runtime template. " + + "This helper only closes JavaDictionary<,> over one reference argument and the mapped value type T. " + + "The reference argument canonicalizes to __Canon and T stays value-specific, so the result shares the JavaDictionary / JavaDictionary " + + "template that TExemplar already roots.")] + [UnconditionalSuppressMessage ("Trimming", "IL2055:MakeGenericType", + Justification = "The generic definition comes from the known JavaDictionary<,> wrapper via TExemplar, not an arbitrary user type. " + + "The wrapper constructors are rooted by the DynamicallyAccessedMembers(Constructors) annotation on TExemplar.")] + [UnconditionalSuppressMessage ("Trimming", "IL2072:UnrecognizedReflectionPattern", + Justification = "The constructed dictionary type rides the TExemplar canonical template whose constructors are rooted by DynamicallyAccessedMembers(Constructors). " + + "Only the known JavaDictionary constructor is invoked here.")] + static IDictionary CreateReferenceMixedDictionary<[DynamicallyAccessedMembers (SafeJavaCollectionFactory.Constructors)] TExemplar> ( + Type[] arguments, + IntPtr handle, + JniHandleOwnership transfer) + { + Debug.Assert (typeof (TExemplar).IsGenericType && typeof (TExemplar) != typeof (TExemplar).GetGenericTypeDefinition ()); + + var dictionaryType = typeof (TExemplar).GetGenericTypeDefinition ().MakeGenericType (arguments); + var instance = Activator.CreateInstance ( + dictionaryType, + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, + binder: null, + args: [handle, transfer], + culture: CultureInfo.InvariantCulture); + if (instance == null) { + throw new InvalidOperationException ($"Unable to create an instance of collection type '{dictionaryType}'."); + } + return (IDictionary) instance; } - internal override IDictionary CreateDictionaryWithKey<[DynamicallyAccessedMembers (SafeContainerConverterFactory.Constructors)] TKey> ( + internal override IDictionary CreateDictionaryWithKey<[DynamicallyAccessedMembers (SafeJavaCollectionFactory.Constructors)] TKey> ( ValueTypeFactory keyFactory, IntPtr handle, JniHandleOwnership transfer) diff --git a/src/Mono.Android/Mono.Android.csproj b/src/Mono.Android/Mono.Android.csproj index 8d86df9f9e8..25aee38cdf3 100644 --- a/src/Mono.Android/Mono.Android.csproj +++ b/src/Mono.Android/Mono.Android.csproj @@ -325,7 +325,7 @@ - + From 9b3d3f8df0f316f1ee1af3085203de0e9d2444b3 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 16 Jul 2026 13:27:55 +0200 Subject: [PATCH 5/8] [Mono.Android] Simplify JavaConvert collection path and fix IL2055 justification Revert the JavaConvert.cs restructuring that hoisted the trimmable-typemap branch out of the generic-target guard. Non-generic collection targets are already handled identically by the existing IDictionary/IList/ICollection fallback below, so routing them through SafeJavaCollectionFactory was redundant. JavaConvert.cs now matches main except for the entry-point method name. Drop the resulting dead TryCreateNonGenericConverter helper (and the now-unused System.Collections import) from SafeJavaCollectionFactory. Also correct the IL2055 suppression justifications on MakeGenericType and CreateReferenceMixedDictionary: IL2055 is raised because the runtime element arguments cannot be proven to satisfy the DynamicallyAccessedMembers (Constructors) requirement on the wrapper's element type parameters, not because of the wrapper's own constructor. On the trimmable typemap path the wrapper never activates its elements (peer creation goes through JavaConvert and the typemap), so the requirement is never exercised. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 50ab60e5-5b4d-4e5f-abd2-2878d87a10c9 --- src/Mono.Android/Java.Interop/JavaConvert.cs | 12 +++--- .../Java.Interop/SafeJavaCollectionFactory.cs | 41 ++++--------------- .../Java.Interop/ValueTypeFactory.cs | 7 +++- 3 files changed, 17 insertions(+), 43 deletions(-) diff --git a/src/Mono.Android/Java.Interop/JavaConvert.cs b/src/Mono.Android/Java.Interop/JavaConvert.cs index 79d4cb9efbf..56d4512f957 100644 --- a/src/Mono.Android/Java.Interop/JavaConvert.cs +++ b/src/Mono.Android/Java.Interop/JavaConvert.cs @@ -74,13 +74,11 @@ static class JavaConvert { if (target.IsArray) return (h, t) => JNIEnv.GetArray (h, t, target.GetElementType ()); - if (RuntimeFeature.TrimmableTypeMap) { - // The trimmable typemap owns all Java collection construction (generic and non-generic) - // without runtime code generation. - if (SafeJavaCollectionFactory.TryCreateConverter (target, out var collectionConverter)) - return collectionConverter; - } else if (target.IsGenericType && !target.IsGenericTypeDefinition) { - if (System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported) { + if (target.IsGenericType && !target.IsGenericTypeDefinition) { + if (RuntimeFeature.TrimmableTypeMap) { + if (SafeJavaCollectionFactory.TryCreateConverter (target, out var collectionConverter)) + return collectionConverter; + } else if (System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported) { var factoryConverter = TryMakeGenericCollectionTypeFactory (target); if (factoryConverter != null) return factoryConverter; diff --git a/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs b/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs index 5e653b2c6a5..520a4d1b765 100644 --- a/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs +++ b/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs @@ -1,6 +1,5 @@ #nullable enable using System; -using System.Collections; using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -61,14 +60,10 @@ internal static bool TryCreateConverter ( converter = (handle, transfer) => CreateFromJniHandle (genericDefinition, arguments, handle, transfer); return true; } - - converter = null; - return false; } - // Non-generic wrappers and raw collection interfaces construct their peer directly (no reflection) - // through the existing FromJniHandle helpers, which also reuse an already-registered managed peer. - return TryCreateNonGenericConverter (targetType, out converter); + converter = null; + return false; } static bool IsKnownContainerDefinition (Type genericDefinition) @@ -91,30 +86,6 @@ static bool IsKnownContainerDefinition (Type genericDefinition) throw new NotSupportedException ($"Unsupported Java container type with generic definition '{genericDefinition}'."); } - - static bool TryCreateNonGenericConverter ( - Type targetType, - [NotNullWhen (true)] out Func? converter) - { - // Order mirrors the historical JavaConvert fallback: dictionary, then list, then collection. - if (typeof (IDictionary).IsAssignableFrom (targetType)) { - converter = static (handle, transfer) => JavaDictionary.FromJniHandle (handle, transfer); - return true; - } - - if (typeof (IList).IsAssignableFrom (targetType)) { - converter = static (handle, transfer) => JavaList.FromJniHandle (handle, transfer); - return true; - } - - if (typeof (ICollection).IsAssignableFrom (targetType)) { - converter = static (handle, transfer) => JavaCollection.FromJniHandle (handle, transfer); - return true; - } - - converter = null; - return false; - } } /// @@ -160,9 +131,11 @@ public abstract bool TryCreateFromJniHandle ( "which all canonicalize to the same __Canon template that T already roots. Value-type arguments never reach this helper; " + "they are rejected or built by ValueTypeFactory, which roots the exact instantiation.")] [UnconditionalSuppressMessage ("Trimming", "IL2055:MakeGenericType", - Justification = "The generic type definition comes from a known Java collection wrapper T, not an arbitrary user type, and the arguments are reference types. " + - "The constructed wrapper shares the __Canon template of T, whose constructors are rooted by the DynamicallyAccessedMembers(Constructors) annotation on T. " + - "The generic element arguments are not activated by this helper; element peer creation still goes through the normal JavaConvert/trimmable typemap path.")] + Justification = "IL2055 is raised because the runtime arguments cannot be statically proven to satisfy the DynamicallyAccessedMembers(Constructors) " + + "requirement that the wrapper (e.g. JavaList<[DAM(Constructors)] TElement>) places on its element type parameter. That requirement exists for the " + + "dynamic-code path, where the wrapper reflectively activates element peers from their constructors. On the trimmable typemap path taken here the wrapper " + + "never activates its elements: element peer creation goes through JavaConvert and the typemap's registered activation constructors, so the unmet element " + + "requirement is never exercised. The wrapper's own constructor is separately rooted by the DynamicallyAccessedMembers(Constructors) annotation on T.")] protected static Type MakeGenericType<[DynamicallyAccessedMembers (SafeJavaCollectionFactory.Constructors)] T> (Type[] arguments) { Debug.Assert (typeof (T).IsGenericType && typeof (T) != typeof (T).GetGenericTypeDefinition ()); diff --git a/src/Mono.Android/Java.Interop/ValueTypeFactory.cs b/src/Mono.Android/Java.Interop/ValueTypeFactory.cs index f08c2f4e27a..c64344d869d 100644 --- a/src/Mono.Android/Java.Interop/ValueTypeFactory.cs +++ b/src/Mono.Android/Java.Interop/ValueTypeFactory.cs @@ -122,8 +122,11 @@ internal override IDictionary CreateDictionaryWithReferenceValue (Type valueType "The reference argument canonicalizes to __Canon and T stays value-specific, so the result shares the JavaDictionary / JavaDictionary " + "template that TExemplar already roots.")] [UnconditionalSuppressMessage ("Trimming", "IL2055:MakeGenericType", - Justification = "The generic definition comes from the known JavaDictionary<,> wrapper via TExemplar, not an arbitrary user type. " + - "The wrapper constructors are rooted by the DynamicallyAccessedMembers(Constructors) annotation on TExemplar.")] + Justification = "IL2055 is raised because the runtime key/value arguments cannot be statically proven to satisfy the DynamicallyAccessedMembers(Constructors) " + + "requirement that JavaDictionary<[DAM(Constructors)] TKey, [DAM(Constructors)] TValue> places on its element type parameters. That requirement exists for the " + + "dynamic-code path, where the wrapper reflectively activates key/value peers from their constructors. On the trimmable typemap path taken here the wrapper never " + + "activates its elements: element peer creation goes through JavaConvert and the typemap's registered activation constructors, so the unmet element requirement is " + + "never exercised. The wrapper's own constructor is separately rooted by the DynamicallyAccessedMembers(Constructors) annotation on TExemplar.")] [UnconditionalSuppressMessage ("Trimming", "IL2072:UnrecognizedReflectionPattern", Justification = "The constructed dictionary type rides the TExemplar canonical template whose constructors are rooted by DynamicallyAccessedMembers(Constructors). " + "Only the known JavaDictionary constructor is invoked here.")] From 92ddb1ff0d867523811dc3a638541625d358392a Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 16 Jul 2026 17:27:41 +0200 Subject: [PATCH 6/8] [Mono.Android] Root canonical wrappers with concrete-literal branch, static factories Replace the DynamicallyAccessedMembers-based rooting of the reference-argument collection wrappers with a concrete-literal approach that is easier to reason about. Each factory now branches on the IJavaPeerable element shape: that branch is a statically reachable reference to the concrete JavaList / JavaCollection / JavaDictionary instantiation, so the trimmer/ILC keeps the (IntPtr, JniHandleOwnership) activation constructor of the shared __Canon template that the reflective else branch reuses for every other reference-argument instantiation. Also: - Reject open and partially-open constructed target types (ContainsGenericParameters) up front so they fail cleanly instead of throwing ArgumentException during activation. - Convert the per-container factories to static classes with a single static TryCreateFromJniHandle method (no abstract base class or instances). - Trim the trimming/AOT suppressions to the verified minimum (IL3050 + IL2055 on MakeGenericType) and correct the IL2055 justification: it fires because MakeGenericType cannot prove the runtime element type satisfies the wrapper's DAM(Constructors) element-parameter requirement, which is never exercised on the trimmable path since the wrapper does not activate its elements. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 50ab60e5-5b4d-4e5f-abd2-2878d87a10c9 --- .../Java.Interop/SafeJavaCollectionFactory.cs | 187 ++++++++---------- 1 file changed, 80 insertions(+), 107 deletions(-) diff --git a/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs b/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs index 520a4d1b765..5935ed9e590 100644 --- a/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs +++ b/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs @@ -1,7 +1,6 @@ #nullable enable using System; using System.Collections.Generic; -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Reflection; @@ -27,9 +26,9 @@ namespace Java.Interop; /// The per-container factories therefore split construction into explicit, non-overlapping paths: /// /// -/// reference element arguments ride the __Canon template via -/// plus -/// ; +/// reference element arguments ride the __Canon template: each factory reflectively +/// closes its wrapper definition and invokes the activation constructor, which is kept alive by a concrete-literal +/// IJavaPeerable rooting branch inside the same factory; /// mapped primitive/nullable arguments go through , /// which roots the exact instantiation with a direct new; /// any other value type throws (no reflection @@ -41,9 +40,8 @@ static class SafeJavaCollectionFactory internal const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; - static readonly SafeContainerTypeFactory ListFactory = new JavaListTypeFactory (); - static readonly SafeContainerTypeFactory CollectionFactory = new JavaCollectionTypeFactory (); - static readonly SafeContainerTypeFactory DictionaryFactory = new JavaDictionaryTypeFactory (); + /// Binding flags matching the public activation constructor of the Java collection wrappers. + internal const BindingFlags ActivationConstructorBinding = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; internal static bool TryCreateConverter ( Type targetType, @@ -51,6 +49,14 @@ internal static bool TryCreateConverter ( { ArgumentNullException.ThrowIfNull (targetType); + // Reject open and partially-open constructed types (e.g. IList<>, IDictionary). These have + // ContainsGenericParameters == true and would produce an open wrapper from MakeGenericType (), whose + // activation would throw ArgumentException. Fail cleanly here rather than crash during construction. + if (targetType.ContainsGenericParameters) { + converter = null; + return false; + } + if (targetType.IsGenericType && !targetType.IsGenericTypeDefinition) { var genericDefinition = targetType.GetGenericTypeDefinition (); if (IsKnownContainerDefinition (genericDefinition)) { @@ -78,9 +84,9 @@ static bool IsKnownContainerDefinition (Type genericDefinition) } object? result; - if (ListFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result) - || CollectionFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result) - || DictionaryFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result)) { + if (JavaListTypeFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result) + || JavaCollectionTypeFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result) + || JavaDictionaryTypeFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result)) { return result; } @@ -88,99 +94,19 @@ static bool IsKnownContainerDefinition (Type genericDefinition) } } -/// -/// Base class for the per-container factories. One factory owns a single Java collection container -/// so that the reference-vs-value construction paths stay explicit and statically analyzable -/// (correctness over reuse). The reflection-based construction helpers live here (rather than being -/// shared through ) so their trimming/AOT suppressions are -/// scoped to the factories that actually build the wrapper types. -/// -abstract class SafeContainerTypeFactory +/// Builds (and ) wrappers for the trimmable typemap. +static class JavaListTypeFactory { - /// - /// Constructs the managed wrapper for a Java collection handle. - /// - /// - /// when is not this factory's container, - /// so callers can chain the factories with ||. (and - /// set) once the matching container is constructed; an unsupported (non-primitive) value-type argument - /// throws instead of falling through. - /// - public abstract bool TryCreateFromJniHandle ( - Type genericDefinition, - Type[] arguments, - IntPtr handle, - JniHandleOwnership transfer, - out object? result); - - /// - /// Closes the generic definition of the reference-type collection wrapper - /// over . - /// - /// - /// A concrete reference-argument instantiation of the wrapper (for example - /// JavaList<IJavaPeerable>). Its roots the - /// wrapper's constructors on the __Canon template, which every reference-argument instantiation - /// (such as JavaList<string>) shares. This makes the AOT safety of the construction obvious: - /// the exact template the runtime resolves for is the one already rooted here. - /// - /// The reference-type generic arguments to close the definition over. [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", - Justification = "NativeAOT's Type.MakeGenericType() is annotated because arbitrary constructed generics can lack a runtime template. " + - "This helper only closes the generic definition of the reference-argument wrapper T (e.g. JavaList) over reference-type arguments, " + - "which all canonicalize to the same __Canon template that T already roots. Value-type arguments never reach this helper; " + - "they are rejected or built by ValueTypeFactory, which roots the exact instantiation.")] + Justification = "MakeGenericType () and Activator.CreateInstance () are annotated because arbitrary constructed generics can lack a runtime template. " + + "elementType is always a reference type here (value types are diverted to ValueTypeFactory above), so JavaList canonicalizes to the " + + "JavaList<__Canon> template whose activation constructor is rooted by the reflective JavaList anchor in the other branch.")] [UnconditionalSuppressMessage ("Trimming", "IL2055:MakeGenericType", - Justification = "IL2055 is raised because the runtime arguments cannot be statically proven to satisfy the DynamicallyAccessedMembers(Constructors) " + - "requirement that the wrapper (e.g. JavaList<[DAM(Constructors)] TElement>) places on its element type parameter. That requirement exists for the " + - "dynamic-code path, where the wrapper reflectively activates element peers from their constructors. On the trimmable typemap path taken here the wrapper " + - "never activates its elements: element peer creation goes through JavaConvert and the typemap's registered activation constructors, so the unmet element " + - "requirement is never exercised. The wrapper's own constructor is separately rooted by the DynamicallyAccessedMembers(Constructors) annotation on T.")] - protected static Type MakeGenericType<[DynamicallyAccessedMembers (SafeJavaCollectionFactory.Constructors)] T> (Type[] arguments) - { - Debug.Assert (typeof (T).IsGenericType && typeof (T) != typeof (T).GetGenericTypeDefinition ()); - foreach (var argument in arguments) { - Debug.Assert (!argument.IsValueType); - } - - return typeof (T).GetGenericTypeDefinition ().MakeGenericType (arguments); - } - - /// - /// Activates using its (IntPtr, JniHandleOwnership) constructor. - /// - /// - /// is intentionally not annotated with - /// : the only supported callers construct it with - /// , whose T already roots the wrapper constructors on the - /// __Canon template. The suppressions below capture that guarantee rather than a static annotation. - /// - [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", - Justification = "Activator.CreateInstance() targets only collection wrapper types produced by the per-container factories. " + - "Reference-argument wrappers use NativeAOT's canonical generic construction rooted by MakeGenericType, " + - "and exact value-type wrappers are rooted by ValueTypeFactory.")] - [UnconditionalSuppressMessage ("Trimming", "IL2067:UnrecognizedReflectionPattern", - Justification = "collectionType is a JavaList, JavaCollection, or JavaDictionary produced by MakeGenericType, " + - "whose T (a reference-argument wrapper instantiation) roots the wrapper constructors on the __Canon template. " + - "Only that activation constructor is invoked here.")] - protected static object CreateInstance (Type collectionType, params object?[] arguments) - { - var instance = Activator.CreateInstance ( - collectionType, - BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, - binder: null, - args: arguments, - culture: CultureInfo.InvariantCulture); - if (instance == null) { - throw new InvalidOperationException ($"Unable to create an instance of collection type '{collectionType}'."); - } - return instance; - } -} - -sealed class JavaListTypeFactory : SafeContainerTypeFactory -{ - public override bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) + Justification = "IL2055 fires because MakeGenericType () cannot statically prove the runtime elementType satisfies the DynamicallyAccessedMembers(Constructors) " + + "requirement that JavaList<[DAM(Constructors)] TElement> places on its element parameter. That requirement exists only for the dynamic-code path, where the wrapper " + + "reflectively activates element peers from their constructors. On the trimmable typemap path the wrapper never activates its elements — element peer creation goes " + + "through JavaConvert and the typemap's registered activation constructors — so the unsatisfied element requirement is never exercised.")] + public static bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) { if (genericDefinition != typeof (IList<>) && genericDefinition != typeof (JavaList<>)) { result = null; @@ -196,14 +122,33 @@ public override bool TryCreateFromJniHandle (Type genericDefinition, Type[] argu return true; } - result = CreateInstance (MakeGenericType> ([elementType]), handle, transfer); + if (elementType == typeof (IJavaPeerable)) { + // Concrete-literal rooting branch. Taken only when marshaling the JavaList shape itself + // (uncommon), its main purpose is to keep the reflection metadata + invoke stub of the (IntPtr, + // JniHandleOwnership) constructor on the JavaList<__Canon> template alive for the trimmer/ILC. The else + // branch reuses that same canonical constructor for every other JavaList. + result = Activator.CreateInstance (typeof (JavaList), SafeJavaCollectionFactory.ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); + } else { + var listType = typeof (JavaList<>).MakeGenericType (elementType); + result = Activator.CreateInstance (listType, SafeJavaCollectionFactory.ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); + } return true; } } -sealed class JavaCollectionTypeFactory : SafeContainerTypeFactory +/// Builds (and ) wrappers for the trimmable typemap. +static class JavaCollectionTypeFactory { - public override bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) + [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", + Justification = "MakeGenericType () and Activator.CreateInstance () are annotated because arbitrary constructed generics can lack a runtime template. " + + "elementType is always a reference type here (value types are diverted to ValueTypeFactory above), so JavaCollection canonicalizes to the " + + "JavaCollection<__Canon> template whose activation constructor is rooted by the reflective JavaCollection anchor in the other branch.")] + [UnconditionalSuppressMessage ("Trimming", "IL2055:MakeGenericType", + Justification = "IL2055 fires because MakeGenericType () cannot statically prove the runtime elementType satisfies the DynamicallyAccessedMembers(Constructors) " + + "requirement that JavaCollection<[DAM(Constructors)] TElement> places on its element parameter. That requirement exists only for the dynamic-code path, where the " + + "wrapper reflectively activates element peers from their constructors. On the trimmable typemap path the wrapper never activates its elements — element peer creation " + + "goes through JavaConvert and the typemap's registered activation constructors — so the unsatisfied element requirement is never exercised.")] + public static bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) { if (genericDefinition != typeof (ICollection<>) && genericDefinition != typeof (JavaCollection<>)) { result = null; @@ -219,14 +164,33 @@ public override bool TryCreateFromJniHandle (Type genericDefinition, Type[] argu return true; } - result = CreateInstance (MakeGenericType> ([elementType]), handle, transfer); + if (elementType == typeof (IJavaPeerable)) { + // Concrete-literal rooting branch. Taken only when marshaling the JavaCollection shape + // itself (uncommon), its main purpose is to keep the reflection metadata + invoke stub of the (IntPtr, + // JniHandleOwnership) constructor on the JavaCollection<__Canon> template alive for the trimmer/ILC. The + // else branch reuses that same canonical constructor for every other JavaCollection. + result = Activator.CreateInstance (typeof (JavaCollection), SafeJavaCollectionFactory.ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); + } else { + var collectionType = typeof (JavaCollection<>).MakeGenericType (elementType); + result = Activator.CreateInstance (collectionType, SafeJavaCollectionFactory.ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); + } return true; } } -sealed class JavaDictionaryTypeFactory : SafeContainerTypeFactory +/// Builds (and ) wrappers for the trimmable typemap. +static class JavaDictionaryTypeFactory { - public override bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) + [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", + Justification = "MakeGenericType () and Activator.CreateInstance () are annotated because arbitrary constructed generics can lack a runtime template. " + + "The reflective branch is reached only when both arguments are reference types, so JavaDictionary canonicalizes to the " + + "JavaDictionary<__Canon,__Canon> template whose activation constructor is rooted by the reflective JavaDictionary anchor in the other branch.")] + [UnconditionalSuppressMessage ("Trimming", "IL2055:MakeGenericType", + Justification = "IL2055 fires because MakeGenericType () cannot statically prove the runtime keyType/valueType satisfy the DynamicallyAccessedMembers(Constructors) " + + "requirement that JavaDictionary<[DAM(Constructors)] TKey, [DAM(Constructors)] TValue> places on its element parameters. That requirement exists only for the " + + "dynamic-code path, where the wrapper reflectively activates key/value peers from their constructors. On the trimmable typemap path the wrapper never activates its " + + "elements — element peer creation goes through JavaConvert and the typemap's registered activation constructors — so the unsatisfied element requirement is never exercised.")] + public static bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) { if (genericDefinition != typeof (IDictionary<,>) && genericDefinition != typeof (JavaDictionary<,>)) { result = null; @@ -258,7 +222,16 @@ public override bool TryCreateFromJniHandle (Type genericDefinition, Type[] argu } // Both arguments are reference types: JavaDictionary rides the __Canon template. - result = CreateInstance (MakeGenericType> ([keyType, valueType]), handle, transfer); + if (keyType == typeof (IJavaPeerable) && valueType == typeof (IJavaPeerable)) { + // Concrete-literal rooting branch. Taken only when marshaling the JavaDictionary shape itself (uncommon), its main purpose is to keep the reflection metadata + invoke stub + // of the (IntPtr, JniHandleOwnership) constructor on the JavaDictionary<__Canon,__Canon> template alive for + // the trimmer/ILC. The else branch reuses that same canonical constructor for every other reference pair. + result = Activator.CreateInstance (typeof (JavaDictionary), SafeJavaCollectionFactory.ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); + } else { + var dictionaryType = typeof (JavaDictionary<,>).MakeGenericType (keyType, valueType); + result = Activator.CreateInstance (dictionaryType, SafeJavaCollectionFactory.ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); + } return true; } From b0788d510814d6f3066b5e30cb1a67a94d3a0862 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 16 Jul 2026 17:44:56 +0200 Subject: [PATCH 7/8] [Mono.Android] Collapse container factories into private methods Merge the three static per-container factory classes into three private static methods on SafeJavaCollectionFactory (TryCreateListFromJniHandle, TryCreateCollectionFromJniHandle, TryCreateDictionaryFromJniHandle), moving their suppressions and helpers onto the class and making ActivationConstructorBinding a private const. No behavior change. Also rename the entry point TryCreateConverter back to TryGetFromJniHandleConverter, which restores the original name and leaves JavaConvert.cs identical to main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 50ab60e5-5b4d-4e5f-abd2-2878d87a10c9 --- src/Mono.Android/Java.Interop/JavaConvert.cs | 2 +- .../Java.Interop/SafeJavaCollectionFactory.cs | 48 +++++++------------ 2 files changed, 19 insertions(+), 31 deletions(-) diff --git a/src/Mono.Android/Java.Interop/JavaConvert.cs b/src/Mono.Android/Java.Interop/JavaConvert.cs index 56d4512f957..c02a02e00fe 100644 --- a/src/Mono.Android/Java.Interop/JavaConvert.cs +++ b/src/Mono.Android/Java.Interop/JavaConvert.cs @@ -76,7 +76,7 @@ static class JavaConvert { if (target.IsGenericType && !target.IsGenericTypeDefinition) { if (RuntimeFeature.TrimmableTypeMap) { - if (SafeJavaCollectionFactory.TryCreateConverter (target, out var collectionConverter)) + if (SafeJavaCollectionFactory.TryGetFromJniHandleConverter (target, out var collectionConverter)) return collectionConverter; } else if (System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported) { var factoryConverter = TryMakeGenericCollectionTypeFactory (target); diff --git a/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs b/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs index 5935ed9e590..b06b695c3b3 100644 --- a/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs +++ b/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs @@ -23,12 +23,12 @@ namespace Java.Interop; /// value-specific. Consequently, JavaList<string> can share the JavaList<__Canon> /// template, but JavaList<int> and JavaList<int?> need exact rooted instantiations. /// -/// The per-container factories therefore split construction into explicit, non-overlapping paths: +/// The per-container construction therefore splits into explicit, non-overlapping paths: /// /// -/// reference element arguments ride the __Canon template: each factory reflectively -/// closes its wrapper definition and invokes the activation constructor, which is kept alive by a concrete-literal -/// IJavaPeerable rooting branch inside the same factory; +/// reference element arguments ride the __Canon template: the wrapper definition is +/// reflectively closed and its activation constructor invoked, kept alive by a concrete-literal +/// IJavaPeerable rooting branch in the same method; /// mapped primitive/nullable arguments go through , /// which roots the exact instantiation with a direct new; /// any other value type throws (no reflection @@ -41,9 +41,9 @@ static class SafeJavaCollectionFactory DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; /// Binding flags matching the public activation constructor of the Java collection wrappers. - internal const BindingFlags ActivationConstructorBinding = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; + const BindingFlags ActivationConstructorBinding = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; - internal static bool TryCreateConverter ( + internal static bool TryGetFromJniHandleConverter ( Type targetType, [NotNullWhen (true)] out Func? converter) { @@ -84,19 +84,15 @@ static bool IsKnownContainerDefinition (Type genericDefinition) } object? result; - if (JavaListTypeFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result) - || JavaCollectionTypeFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result) - || JavaDictionaryTypeFactory.TryCreateFromJniHandle (genericDefinition, arguments, handle, transfer, out result)) { + if (TryCreateListFromJniHandle (genericDefinition, arguments, handle, transfer, out result) + || TryCreateCollectionFromJniHandle (genericDefinition, arguments, handle, transfer, out result) + || TryCreateDictionaryFromJniHandle (genericDefinition, arguments, handle, transfer, out result)) { return result; } throw new NotSupportedException ($"Unsupported Java container type with generic definition '{genericDefinition}'."); } -} -/// Builds (and ) wrappers for the trimmable typemap. -static class JavaListTypeFactory -{ [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", Justification = "MakeGenericType () and Activator.CreateInstance () are annotated because arbitrary constructed generics can lack a runtime template. " + "elementType is always a reference type here (value types are diverted to ValueTypeFactory above), so JavaList canonicalizes to the " + @@ -106,7 +102,7 @@ static class JavaListTypeFactory "requirement that JavaList<[DAM(Constructors)] TElement> places on its element parameter. That requirement exists only for the dynamic-code path, where the wrapper " + "reflectively activates element peers from their constructors. On the trimmable typemap path the wrapper never activates its elements — element peer creation goes " + "through JavaConvert and the typemap's registered activation constructors — so the unsatisfied element requirement is never exercised.")] - public static bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) + static bool TryCreateListFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) { if (genericDefinition != typeof (IList<>) && genericDefinition != typeof (JavaList<>)) { result = null; @@ -127,18 +123,14 @@ public static bool TryCreateFromJniHandle (Type genericDefinition, Type[] argume // (uncommon), its main purpose is to keep the reflection metadata + invoke stub of the (IntPtr, // JniHandleOwnership) constructor on the JavaList<__Canon> template alive for the trimmer/ILC. The else // branch reuses that same canonical constructor for every other JavaList. - result = Activator.CreateInstance (typeof (JavaList), SafeJavaCollectionFactory.ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); + result = Activator.CreateInstance (typeof (JavaList), ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); } else { var listType = typeof (JavaList<>).MakeGenericType (elementType); - result = Activator.CreateInstance (listType, SafeJavaCollectionFactory.ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); + result = Activator.CreateInstance (listType, ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); } return true; } -} -/// Builds (and ) wrappers for the trimmable typemap. -static class JavaCollectionTypeFactory -{ [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", Justification = "MakeGenericType () and Activator.CreateInstance () are annotated because arbitrary constructed generics can lack a runtime template. " + "elementType is always a reference type here (value types are diverted to ValueTypeFactory above), so JavaCollection canonicalizes to the " + @@ -148,7 +140,7 @@ static class JavaCollectionTypeFactory "requirement that JavaCollection<[DAM(Constructors)] TElement> places on its element parameter. That requirement exists only for the dynamic-code path, where the " + "wrapper reflectively activates element peers from their constructors. On the trimmable typemap path the wrapper never activates its elements — element peer creation " + "goes through JavaConvert and the typemap's registered activation constructors — so the unsatisfied element requirement is never exercised.")] - public static bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) + static bool TryCreateCollectionFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) { if (genericDefinition != typeof (ICollection<>) && genericDefinition != typeof (JavaCollection<>)) { result = null; @@ -169,18 +161,14 @@ public static bool TryCreateFromJniHandle (Type genericDefinition, Type[] argume // itself (uncommon), its main purpose is to keep the reflection metadata + invoke stub of the (IntPtr, // JniHandleOwnership) constructor on the JavaCollection<__Canon> template alive for the trimmer/ILC. The // else branch reuses that same canonical constructor for every other JavaCollection. - result = Activator.CreateInstance (typeof (JavaCollection), SafeJavaCollectionFactory.ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); + result = Activator.CreateInstance (typeof (JavaCollection), ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); } else { var collectionType = typeof (JavaCollection<>).MakeGenericType (elementType); - result = Activator.CreateInstance (collectionType, SafeJavaCollectionFactory.ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); + result = Activator.CreateInstance (collectionType, ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); } return true; } -} -/// Builds (and ) wrappers for the trimmable typemap. -static class JavaDictionaryTypeFactory -{ [UnconditionalSuppressMessage ("AOT", "IL3050:RequiresDynamicCode", Justification = "MakeGenericType () and Activator.CreateInstance () are annotated because arbitrary constructed generics can lack a runtime template. " + "The reflective branch is reached only when both arguments are reference types, so JavaDictionary canonicalizes to the " + @@ -190,7 +178,7 @@ static class JavaDictionaryTypeFactory "requirement that JavaDictionary<[DAM(Constructors)] TKey, [DAM(Constructors)] TValue> places on its element parameters. That requirement exists only for the " + "dynamic-code path, where the wrapper reflectively activates key/value peers from their constructors. On the trimmable typemap path the wrapper never activates its " + "elements — element peer creation goes through JavaConvert and the typemap's registered activation constructors — so the unsatisfied element requirement is never exercised.")] - public static bool TryCreateFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) + static bool TryCreateDictionaryFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) { if (genericDefinition != typeof (IDictionary<,>) && genericDefinition != typeof (JavaDictionary<,>)) { result = null; @@ -227,10 +215,10 @@ public static bool TryCreateFromJniHandle (Type genericDefinition, Type[] argume // IJavaPeerable> shape itself (uncommon), its main purpose is to keep the reflection metadata + invoke stub // of the (IntPtr, JniHandleOwnership) constructor on the JavaDictionary<__Canon,__Canon> template alive for // the trimmer/ILC. The else branch reuses that same canonical constructor for every other reference pair. - result = Activator.CreateInstance (typeof (JavaDictionary), SafeJavaCollectionFactory.ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); + result = Activator.CreateInstance (typeof (JavaDictionary), ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); } else { var dictionaryType = typeof (JavaDictionary<,>).MakeGenericType (keyType, valueType); - result = Activator.CreateInstance (dictionaryType, SafeJavaCollectionFactory.ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); + result = Activator.CreateInstance (dictionaryType, ActivationConstructorBinding, binder: null, args: [handle, transfer], culture: CultureInfo.InvariantCulture); } return true; } From 0d6c5163072df5067820783dc6c6b1fac6eabf23 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 16 Jul 2026 21:19:32 +0200 Subject: [PATCH 8/8] [Mono.Android] Address collection factory review Add the missing trim suppressions, clarify activation behavior, simplify the mixed dictionary helper, and cover unsupported value-type containers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa --- .../Java.Interop/SafeJavaCollectionFactory.cs | 11 ++++++- .../Java.Interop/ValueTypeFactory.cs | 6 ++-- .../Java.Interop/JavaConvertTest.cs | 30 ++++++++++++++++++- 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs b/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs index b06b695c3b3..bb342ac344d 100644 --- a/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs +++ b/src/Mono.Android/Java.Interop/SafeJavaCollectionFactory.cs @@ -40,7 +40,7 @@ static class SafeJavaCollectionFactory internal const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors; - /// Binding flags matching the public activation constructor of the Java collection wrappers. + /// Binding flags used to find the activation constructor of the Java collection wrappers. const BindingFlags ActivationConstructorBinding = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance; internal static bool TryGetFromJniHandleConverter ( @@ -102,6 +102,9 @@ static bool IsKnownContainerDefinition (Type genericDefinition) "requirement that JavaList<[DAM(Constructors)] TElement> places on its element parameter. That requirement exists only for the dynamic-code path, where the wrapper " + "reflectively activates element peers from their constructors. On the trimmable typemap path the wrapper never activates its elements — element peer creation goes " + "through JavaConvert and the typemap's registered activation constructors — so the unsatisfied element requirement is never exercised.")] + [UnconditionalSuppressMessage ("Trimming", "IL2072:UnrecognizedReflectionPattern", + Justification = "The dynamically constructed JavaList rides the JavaList canonical template whose activation constructor is rooted by the " + + "concrete-literal branch. Only the known JavaList activation constructor is invoked here.")] static bool TryCreateListFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) { if (genericDefinition != typeof (IList<>) && genericDefinition != typeof (JavaList<>)) { @@ -140,6 +143,9 @@ static bool TryCreateListFromJniHandle (Type genericDefinition, Type[] arguments "requirement that JavaCollection<[DAM(Constructors)] TElement> places on its element parameter. That requirement exists only for the dynamic-code path, where the " + "wrapper reflectively activates element peers from their constructors. On the trimmable typemap path the wrapper never activates its elements — element peer creation " + "goes through JavaConvert and the typemap's registered activation constructors — so the unsatisfied element requirement is never exercised.")] + [UnconditionalSuppressMessage ("Trimming", "IL2072:UnrecognizedReflectionPattern", + Justification = "The dynamically constructed JavaCollection rides the JavaCollection canonical template whose activation constructor is rooted " + + "by the concrete-literal branch. Only the known JavaCollection activation constructor is invoked here.")] static bool TryCreateCollectionFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) { if (genericDefinition != typeof (ICollection<>) && genericDefinition != typeof (JavaCollection<>)) { @@ -178,6 +184,9 @@ static bool TryCreateCollectionFromJniHandle (Type genericDefinition, Type[] arg "requirement that JavaDictionary<[DAM(Constructors)] TKey, [DAM(Constructors)] TValue> places on its element parameters. That requirement exists only for the " + "dynamic-code path, where the wrapper reflectively activates key/value peers from their constructors. On the trimmable typemap path the wrapper never activates its " + "elements — element peer creation goes through JavaConvert and the typemap's registered activation constructors — so the unsatisfied element requirement is never exercised.")] + [UnconditionalSuppressMessage ("Trimming", "IL2072:UnrecognizedReflectionPattern", + Justification = "The dynamically constructed JavaDictionary rides the JavaDictionary canonical template whose activation " + + "constructor is rooted by the concrete-literal branch. Only the known JavaDictionary activation constructor is invoked here.")] static bool TryCreateDictionaryFromJniHandle (Type genericDefinition, Type[] arguments, IntPtr handle, JniHandleOwnership transfer, out object? result) { if (genericDefinition != typeof (IDictionary<,>) && genericDefinition != typeof (JavaDictionary<,>)) { diff --git a/src/Mono.Android/Java.Interop/ValueTypeFactory.cs b/src/Mono.Android/Java.Interop/ValueTypeFactory.cs index c64344d869d..2b692f74cc1 100644 --- a/src/Mono.Android/Java.Interop/ValueTypeFactory.cs +++ b/src/Mono.Android/Java.Interop/ValueTypeFactory.cs @@ -135,9 +135,11 @@ internal override IDictionary CreateDictionaryWithReferenceValue (Type valueType IntPtr handle, JniHandleOwnership transfer) { - Debug.Assert (typeof (TExemplar).IsGenericType && typeof (TExemplar) != typeof (TExemplar).GetGenericTypeDefinition ()); + var exemplarType = typeof (TExemplar); + Debug.Assert (exemplarType.IsGenericType && !exemplarType.IsGenericTypeDefinition); - var dictionaryType = typeof (TExemplar).GetGenericTypeDefinition ().MakeGenericType (arguments); + var definition = exemplarType.GetGenericTypeDefinition (); + var dictionaryType = definition.MakeGenericType (arguments); var instance = Activator.CreateInstance ( dictionaryType, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, diff --git a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaConvertTest.cs b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaConvertTest.cs index f62b3c54fdd..4a5e75c997b 100644 --- a/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaConvertTest.cs +++ b/tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaConvertTest.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; +using System.Runtime.ExceptionServices; using Android.App; using Android.Content; @@ -221,6 +222,27 @@ public void FromJniHandle_IListByte () } } + [TestCase (typeof (IList))] + [TestCase (typeof (JavaList))] + [TestCase (typeof (ICollection))] + [TestCase (typeof (JavaCollection))] + [TestCase (typeof (IDictionary))] + [TestCase (typeof (JavaDictionary))] + [TestCase (typeof (IDictionary))] + [TestCase (typeof (JavaDictionary))] + [Category ("NativeAOTTrimmable")] + public void FromJniHandle_UnsupportedValueTypeThrows (Type targetType) + { + if (!Microsoft.Android.Runtime.RuntimeFeature.TrimmableTypeMap) { + Assert.Ignore ("This test validates unsupported value-type container arguments on the trimmable typemap path."); + } + + using (var source = new JavaList ()) { + Assert.Throws (() => + InvokeJavaConvertFromJniHandle (targetType, source.Handle, JniHandleOwnership.DoNotTransfer)); + } + } + static Java.Util.ArrayList CreateList (params int[][] items) { var list = new Java.Util.ArrayList (); @@ -246,7 +268,13 @@ static object InvokeJavaConvertFromJniHandle (Type targetType, IntPtr handle, Jn modifiers: null); Assert.IsNotNull (method); - var value = method.Invoke (null, new object [] { handle, transfer, targetType }); + object value; + try { + value = method.Invoke (null, new object [] { handle, transfer, targetType }); + } catch (TargetInvocationException e) when (e.InnerException != null) { + ExceptionDispatchInfo.Capture (e.InnerException).Throw (); + throw; + } Assert.IsNotNull (value); return value; }