Reflection value manager doesn't marshal generic collection interfaces other than IList<T>

[simonrozsival]

Summary

JniRuntime.ReflectionJniValueManager only special-cases IList<> when selecting a value marshaler. IDictionary<TKey,TValue>, ICollection<T>, and ISet<T> all fall through to ProxyValueMarshaler, which cannot marshal them and ends up calling CreatePeer with an interface targetType.

This affects the non-trimmable (llvm-ir) typemap on both runtimes:

Value manager	Runtime	Selected when	Affected
AndroidValueManager	MonoVM	!TrimmableTypeMap	❌ yes
JavaMarshalValueManager	CoreCLR / NativeAOT	!TrimmableTypeMap	❌ yes
TrimmableTypeMapValueManager	any	TrimmableTypeMap	✅ no

This is a long-standing hole, not a regression — it has never worked on the reflection-based path.

Root cause

ReflectionJniValueManager.GetValueMarshalerCore resolves marshalers in this order: [JniValueMarshaler] attribute → IJavaPeerable → builtins → GetListType → IJavaPeerable-assignable → ProxyValueMarshaler.

The GetListType helper matches IList<> only:

static Type? GetListType (Type type)
{
	foreach (var iface in type.GetInterfaces ().Concat (new [] { type })) {
		if (typeof (IList<>).IsAssignableFrom (iface.IsGenericType ? iface.GetGenericTypeDefinition () : iface))
			return iface;
	}
	return null;
}

typeof (IList<>).IsAssignableFrom (typeof (IDictionary<,>)) and typeof (IList<>).IsAssignableFrom (typeof (ICollection<>)) are both false, so those types reach ProxyValueMarshaler.CreateGenericValue. That method asks the value manager for a marshaler for the same type, gets itself back, and falls through to its // Punt! Hope it's a java.lang.Object branch.

Why the trimmable path is unaffected

TrimmableTypeMapValueManager.CreateValueCore routes everything through JavaConvert.FromObjectReference, and SafeJavaCollectionFactory.IsKnownContainerDefinition already covers IList<>/JavaList<>, ICollection<>/JavaCollection<>, and IDictionary<,>/JavaDictionary<,>. The typemap rewrite closed this incidentally.

The trimmable manager's behavior is the reference for what the reflection path should do.

Scope

1. IDictionary<TKey,TValue> / JavaDictionary<TKey,TValue> — confirmed broken; a targeted fix was prototyped in Fix legacy generic dictionary conversion #12116 (closed in favor of this issue).
2. ICollection<T> / JavaCollection<T> — same code path, expected to be broken; needs a test to confirm. JavaConvert.TryMakeGenericCollectionTypeFactory already knows how to convert these, so only the marshaler-selection step is missing.
3. ISet<T> / JavaSet<T> — missing from JavaConvert.GetJniHandleConverter and SafeJavaCollectionFactory, so this one is broken on all paths, including trimmable. Larger scope than the other two.

Suggested approach

Rather than overriding GetValueCore in each Mono.Android value manager subclass (which is what #12116 did — it jumps ahead of the base class's EnsureNotDisposed() / reference.IsValid / PeekValue / targetType validation, re-derives the base contract, and duplicates the same block in two subclasses), follow the precedent set by 7b0b406 ("Guard primitive array value-manager routing", #12114).

That commit added a protected virtual object? CreateNonArrayListValue (...) hook in ReflectionJniValueManager at the point of failure, letting Mono.Android supply only the Android-specific conversion while the base class keeps owning the surrounding contract.

The analogous fix here is a hook at the marshaler-selection point, so that:

• the base class continues to own dispose-checking, reference validation, peer-cache peeking, and targetType/T compatibility validation;
• Mono.Android supplies only the JavaConvert-backed converter;
• both AndroidValueManager and JavaMarshalValueManager inherit the fix with no duplicated code, as would any future value manager.

Because ReflectionJniValueManager lives in external/Java.Interop, this needs a Java.Interop-side change plus a submodule bump.

Test coverage

tests/Mono.Android-Tests/Mono.Android-Tests/Java.Interop/JavaConvertTest.cs is the right home; #12116 has on-device tests for the dictionary cases that can be reused, and should be extended with ICollection<T> (and ISet<T> if that is taken on). Tests must run under both -p:AndroidTypeMapImplementation=llvm-ir and trimmable, and under both -p:UseMonoRuntime=true and false, since only the llvm-ir lanes exercise the broken path.

Related

• Fix legacy generic dictionary conversion #12116 — closed; contains a working dictionary-only fix and on-device regression tests
• Guard primitive array value-manager routing #12114 / 7b0b406 — precedent for the protected virtual hook pattern in ReflectionJniValueManager