Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,9 @@ void EmitCreateInstance (JavaPeerProxyData proxy)
if (jiCtor.IsOnLeafType) {
EmitCreateInstanceViaJavaInteropNewobj (targetRef);
} else {
EmitCreateInstanceInheritedJavaInteropCtor (targetRef, jiCtor);
// Legacy GetConstructor() doesn't find inherited ctors —
// match that behavior by returning null.
EmitCreateInstanceNoActivation ();
}
}
return;
Expand All @@ -633,7 +635,9 @@ void EmitCreateInstance (JavaPeerProxyData proxy)
if (activationCtor.IsOnLeafType) {
EmitCreateInstanceViaNewobj (targetTypeRef);
} else {
EmitCreateInstanceInheritedCtor (targetTypeRef, activationCtor);
// Legacy GetConstructor() doesn't find inherited ctors —
// match that behavior by returning null.
EmitCreateInstanceNoActivation ();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,13 @@ internal RegisterInfo ParseJniTypeSignatureAttribute (CustomAttribute ca)
doNotGenerateAcw = !generateJavaPeer;
}

var isArrayType = TryGetNamedArgument<int> (value, "ArrayRank", out var rank) && rank > 0;

return new RegisterInfo {
JniName = jniName.Replace ('.', '/'),
DoNotGenerateAcw = doNotGenerateAcw,
IsFromJniTypeSignature = true,
IsArrayType = isArrayType,
};
}

Expand Down Expand Up @@ -529,6 +532,7 @@ sealed record RegisterInfo
public string? Connector { get; init; }
public bool DoNotGenerateAcw { get; init; }
public bool IsFromJniTypeSignature { get; init; }
public bool IsArrayType { get; init; }
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,15 @@ void ScanAssembly (AssemblyIndex index, Dictionary<(string ManagedName, string A
index.AttributesByType.TryGetValue (typeHandle, out var attrInfo);

if (registerInfo is not null && !string.IsNullOrEmpty (registerInfo.JniName)) {
// [JniTypeSignature] with ArrayRank > 0 represents a JNI array wrapper
// (e.g., JavaBooleanArray, JavaObjectArray<T>, JavaPrimitiveArray<T>).
// These are handled by the built-in tables in JniRuntime.JniTypeManager
// and must not be added to the typemap — keyword types (Z, B, etc.)
// would collide with GetPrimitiveArrayTypesForSimpleReference, and
// non-keyword array types would add unnecessary aliases.
if (registerInfo.IsArrayType) {
continue;
}
jniName = registerInfo.JniName;
compatJniName = jniName;
doNotGenerateAcw = registerInfo.DoNotGenerateAcw;
Expand Down Expand Up @@ -229,6 +238,14 @@ void ScanAssembly (AssemblyIndex index, Dictionary<(string ManagedName, string A
invokerTypeName = TryFindInvokerTypeName (fullName, typeHandle, index);
}

// Interface peers have no constructors of their own, so ResolveActivationCtor
// returns null. The invoker type holds the activation ctor — resolve it from
// there so the generator can pick the correct ctor signature
// (XamarinAndroid (IntPtr, JniHandleOwnership) vs JavaInterop (ref JniObjectReference, JniObjectReferenceOptions)).
if (activationCtor is null && invokerTypeName is not null) {
activationCtor = TryResolveActivationCtorOnInvoker (invokerTypeName);
}

var peer = new JavaPeerInfo {
JavaName = jniName,
CompatJniName = compatJniName,
Expand Down Expand Up @@ -1280,6 +1297,24 @@ string ManagedTypeToJniDescriptor (string managedType)
return null;
}

/// <summary>
/// Resolve the activation ctor on a known invoker type (search all loaded assemblies).
/// Used for interface peers, whose own type definition has no constructors.
/// The assemblyCache typically contains 10–30 entries (app + framework assemblies),
/// and each lookup is an O(1) dictionary probe, so the linear scan is cheap.
/// </summary>
ActivationCtorInfo? TryResolveActivationCtorOnInvoker (string invokerTypeName)
{
foreach (var assembly in assemblyCache.Values) {
if (!assembly.TypesByFullName.TryGetValue (invokerTypeName, out var invokerHandle)) {
continue;
}
var invokerDef = assembly.Reader.GetTypeDefinition (invokerHandle);
return ResolveActivationCtor (invokerTypeName, invokerDef, assembly);
}
return null;
}

public void Dispose ()
{
foreach (var index in assemblyCache.Values) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -514,9 +514,28 @@ void ProcessContext (HandleContext* context)

if (RuntimeFeature.TrimmableTypeMap) {
try {
// Mirror legacy GetPeerType: callers commonly request universal
// interfaces / boxes (IJavaPeerable, object, Exception) — map these
// to a concrete peer type so the proxy lookup can succeed.
var resolvedTargetType = ResolvePeerType (targetType);

var typeMap = TrimmableTypeMap.Instance;
var proxy = typeMap.GetProxyForJavaObject (reference.Handle, targetType);
var peer = proxy?.CreateInstance (reference.Handle, JniHandleOwnership.DoNotTransfer);
var proxy = typeMap.GetProxyForJavaObject (reference.Handle, resolvedTargetType);

// Open-generic proxies (e.g. JavaList<>) cannot create closed
// instantiations (e.g. JavaList<int>) via CreateInstance because
// the generated IL can't newobj an open generic type. Activate the
// closed targetType directly via its (IntPtr, JniHandleOwnership)
// ctor — [DAM(Constructors)] on targetType guarantees the trimmer
// preserves the ctor metadata.
IJavaPeerable? peer;
if (proxy is not null && proxy.TargetType.IsGenericTypeDefinition &&
resolvedTargetType is not null &&
resolvedTargetType.IsGenericType && !resolvedTargetType.IsGenericTypeDefinition) {
peer = ActivateUsingReflection (resolvedTargetType, reference.Handle, JniHandleOwnership.DoNotTransfer);
} else {
peer = proxy?.CreateInstance (reference.Handle, JniHandleOwnership.DoNotTransfer);
}
if (peer is not null) {
var peerState = peer.JniManagedPeerState | JniManagedPeerStates.Replaceable;
if (global::Java.Interop.Runtime.IsGCUserPeer (peer.PeerReference.Handle)) {
Expand All @@ -526,7 +545,22 @@ void ProcessContext (HandleContext* context)
return peer;
}

var targetName = targetType?.AssemblyQualifiedName ?? "<null>";
// Disambiguate the failure — match the contract of the base
// JniRuntime.JniValueManager.CreatePeer so JavaCast / JavaAs
// surface the right exception (or null) to callers:
//
// (a) target type has no Java mapping at all → ArgumentException
// (b) Java instance is not assignable to the target's Java class
// → return null (JavaAs returns null; JavaCast wraps to
// InvalidCastException via its `??` clause)
// (c) classes are compatible but no proxy / activation failed
// → NotSupportedException (genuine generator gap)
if (resolvedTargetType is not null &&
IsIncompatibleCast (typeMap, ref reference, resolvedTargetType)) {
return null;
}

var targetName = resolvedTargetType?.AssemblyQualifiedName ?? "<null>";
var javaType = JniEnvironment.Types.GetJniTypeNameFromInstance (reference);

throw new NotSupportedException (
Expand All @@ -541,6 +575,80 @@ void ProcessContext (HandleContext* context)
return base.CreatePeer (ref reference, transfer, targetType);
}

[return: DynamicallyAccessedMembers (Constructors)]
static Type? ResolvePeerType ([DynamicallyAccessedMembers (Constructors)] Type? type)
{
if (type is null) {
return null;
}
if (type == typeof (object) || type == typeof (IJavaPeerable)) {
return typeof (global::Java.Interop.JavaObject);
}
if (type == typeof (Exception)) {
return typeof (JavaException);
}
return type;
}

static IJavaPeerable? ActivateUsingReflection (
[DynamicallyAccessedMembers (Constructors)]
Type closedType,
IntPtr handle,
JniHandleOwnership transfer)
{
var ctor = closedType.GetConstructor (ActivationConstructorBindingFlags, null, XAConstructorSignature, null);
if (ctor is null) {
return null;
}

return (IJavaPeerable) ctor.Invoke ([handle, transfer]);
}

/// <summary>
/// When the trimmable typemap proxy lookup yields no peer for a non-null
/// <paramref name="targetType"/>, decide whether the caller's request is a
/// genuine bad cast (return true → caller returns null) or a missing typemap
/// entry (throw <see cref="ArgumentException"/>).
/// Returns false when the target's Java class IS compatible with the
/// instance — letting the caller fall through to its NotSupportedException
/// branch.
/// </summary>
static bool IsIncompatibleCast (
TrimmableTypeMap typeMap,
ref JniObjectReference reference,
Type targetType)
{
if (!typeMap.TryGetJniNameForManagedType (targetType, out var targetJniName)) {
throw new ArgumentException (
$"Could not determine Java type corresponding to '{targetType.AssemblyQualifiedName}'.",
nameof (targetType));
}

var instanceClass = JniEnvironment.Types.GetObjectClass (reference);
JniObjectReference targetClass = default;
try {
try {
targetClass = JniEnvironment.Types.FindClass (targetJniName);
} catch (Exception e) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 ⚠️ Error handling — This catches Exception but the analogous FindClass call in TrimmableTypeMap.cs:243 catches only Java.Lang.ClassNotFoundException. The broad catch here could mask unexpected JNI failures (e.g., NoClassDefFoundError from a class whose static initializer failed, or OutOfMemoryError) by wrapping them as ArgumentException, making the root cause harder to diagnose.

Consider narrowing to catch (Java.Lang.ClassNotFoundException e) for consistency with TrimmableTypeMap.TryGetProxyFromTargetType, or if intentionally broader, add a comment explaining why.

Rule: Challenge exception swallowing / Differentiate similar error messages (Postmortem #7)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot address this comment - change the code so that we catch Java.Lang.ClassNotFoundException

throw new ArgumentException (
$"Could not find Java class '{targetJniName}'.",
nameof (targetType), e);
}

if (!JniEnvironment.Types.IsAssignableFrom (instanceClass, targetClass)) {
// Genuine bad cast — return null, callers translate this.
return true;
}
} finally {
JniObjectReference.Dispose (ref instanceClass);
JniObjectReference.Dispose (ref targetClass);
}

// Classes are compatible — caller should throw NotSupportedException
// (real proxy/activation gap).
return false;
}

protected override bool TryConstructPeer (
IJavaPeerable self,
ref JniObjectReference reference,
Expand Down
37 changes: 23 additions & 14 deletions src/Mono.Android/Microsoft.Android.Runtime/TrimmableTypeMap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,16 @@ internal bool TryGetJniNameForManagedType (Type managedType, [NotNullWhen (true)
var targetClass = default (JniObjectReference);
try {
objClass = JniEnvironment.Types.GetObjectClass (selfRef);
targetClass = JniEnvironment.Types.FindClass (targetJniName);
try {
targetClass = JniEnvironment.Types.FindClass (targetJniName);
} catch (Java.Lang.ClassNotFoundException) {
// FindClass throws for managed types whose Java peer class is
// not present in the APK (e.g. test types annotated with
// [JniTypeSignature("__missing__")]). Treat as "no match" so
// JavaMarshalValueManager.CreatePeer can surface the correct
// ArgumentException instead of leaking ClassNotFoundException.
return null;
}
var isAssignable = JniEnvironment.Types.IsAssignableFrom (objClass, targetClass);
return isAssignable ? proxy : null;
} finally {
Expand Down Expand Up @@ -273,22 +282,22 @@ internal bool TryGetJniNameForManagedType (Type managedType, [NotNullWhen (true)
/// </remarks>
internal static bool TargetTypeMatches (Type targetType, Type proxyTargetType)
{
if (targetType.IsAssignableFrom (proxyTargetType)) {
return true;
}

if (!proxyTargetType.IsGenericTypeDefinition) {
return false;
}

for (Type? t = targetType; t is not null; t = t.BaseType) {
if (t.IsGenericType && !t.IsGenericTypeDefinition &&
t.GetGenericTypeDefinition () == proxyTargetType) {
return true;
// Open generic proxy: match only when targetType is a closed instantiation
// of this generic (e.g. JavaList<int> matches the JavaList<> proxy).
// IsAssignableFrom alone would incorrectly match unrelated open generics
// that are technically subclasses (e.g. JavaArray<> is assignable to
// JavaObject), and proxy.CreateInstance for an open generic always throws.
if (proxyTargetType.IsGenericTypeDefinition) {
for (Type? t = targetType; t is not null; t = t.BaseType) {
if (t.IsGenericType && !t.IsGenericTypeDefinition &&
t.GetGenericTypeDefinition () == proxyTargetType) {
return true;
}
}
return false;
}

return false;
return targetType.IsAssignableFrom (proxyTargetType);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,20 @@ public void Scan_JniTypeSignature_SubclassExtendsJavaPeer ()
var peer = FindFixtureByJavaName ("net/dot/jni/test/JavaDisposedObject");
Assert.NotNull (peer);
}

[Fact]
public void Scan_JniTypeSignature_ArrayRank_IsExcluded ()
{
// Types with [JniTypeSignature(ArrayRank > 0)] represent JNI array wrappers
// (e.g., JavaBooleanArray with IsKeyword=true, or JavaObjectArray<T> without).
// The scanner must skip all of them — they are handled by the built-in tables
// in JniRuntime.JniTypeManager, not the typemap.
var peers = ScanFixtures ();

// Keyword primitive array (e.g., JavaBooleanArray with "Z")
Assert.DoesNotContain (peers, p => p.ManagedTypeName == "Java.Interop.TestTypes.KeywordPrimitiveArray");

// Non-keyword array (e.g., JavaObjectArray<T> with "java/lang/Object", ArrayRank=1)
Assert.DoesNotContain (peers, p => p.ManagedTypeName == "Java.Interop.TestTypes.NonKeywordArrayType");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -969,4 +969,23 @@ public class NonGeneratedJavaObject : JavaObject
{
public NonGeneratedJavaObject () { }
}

/// <summary>
/// Mimics Java.Interop.JavaBooleanArray — a primitive array type with IsKeyword=true.
/// The scanner must skip all ArrayRank > 0 types because they are handled by the
/// built-in tables in JniRuntime.JniTypeManager.
/// </summary>
[Java.Interop.JniTypeSignature ("Z", IsKeyword = true, ArrayRank = 1, GenerateJavaPeer = false)]
public sealed class KeywordPrimitiveArray : JavaObject
{
}

/// <summary>
/// Mimics Java.Interop.JavaObjectArray — a non-keyword array type with ArrayRank=1.
/// The scanner must also skip these to avoid adding unnecessary aliases.
/// </summary>
[Java.Interop.JniTypeSignature ("java/lang/Object", ArrayRank = 1, GenerateJavaPeer = false)]
public class NonKeywordArrayType : JavaObject
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,40 @@
</TestJarEntry>
</ItemDefinitionGroup>
<ItemGroup>
<TestJarEntry Include="$(MSBuildThisFileDirectory)..\..\..\external\Java.Interop\tests\Java.Interop-Tests\java\**\*.java" />
<!--
java-trimmable/net/dot/jni/test/GetThis.java is added explicitly via
TestJarEntry below when building for the trimmable typemap; it must be
removed from the implicit AndroidJavaSource glob so the .NET SDK does
not also try to compile it as a regular AndroidJavaSource (which would
collide with the desktop GetThis.java in the test JAR).
-->
<AndroidJavaSource Remove="java-trimmable\**\*.java" />
</ItemGroup>

<ItemGroup>
<!--
java-trimmable/net/dot/jni/test/GetThis.java is the Android trimmable
typemap variant of the Java.Interop GetThis.java test fixture. It omits
the desktop-JVM static initializer (net.dot.jni.ManagedPeer.registerNativeMembers /
ManagedPeer.construct) that is only registered by the Java.Interop test
JVM and throws UnsatisfiedLinkError on Android. Both files declare
`public class net.dot.jni.test.GetThis`, so include exactly one based on
$(_AndroidTypeMapImplementation). The trimmable variant lives in this
project (not the submodule) and in a parallel directory because javac
requires the file name match the public class name.
-->
<!--
Both branches below are mutually exclusive on $(_AndroidTypeMapImplementation).
Switching between trimmable and non-trimmable requires a clean rebuild because
MSBuild's up-to-date check doesn't track the conditional swap automatically.
-->
<TestJarEntry Include="$(MSBuildThisFileDirectory)..\..\..\external\Java.Interop\tests\Java.Interop-Tests\java\**\*.java"
Exclude="$(MSBuildThisFileDirectory)..\..\..\external\Java.Interop\tests\Java.Interop-Tests\java\net\dot\jni\test\GetThis.java"
Condition=" '$(_AndroidTypeMapImplementation)' == 'trimmable' " />
<TestJarEntry Include="$(MSBuildThisFileDirectory)..\..\..\external\Java.Interop\tests\Java.Interop-Tests\java\**\*.java"
Condition=" '$(_AndroidTypeMapImplementation)' != 'trimmable' " />
<TestJarEntry Include="$(MSBuildThisFileDirectory)java-trimmable\net\dot\jni\test\GetThis.java"
Condition=" '$(_AndroidTypeMapImplementation)' == 'trimmable' " />
</ItemGroup>
<Target Name="_CopyTestJarFiles">
<Copy
Expand Down
Loading
Loading