Skip to content

Commit

Permalink
Fix Application subclass usage.
Browse files Browse the repository at this point in the history 
Context: https://discord.com/channels/732297728826277939/732297837953679412/1334614545871929345

PR #9716 was crashing with a stack overflow:

	I NativeAotFromAndroid:    at Java.Interop.JniEnvironment.InstanceMethods.CallVoidMethod(JniObjectReference, JniMethodInfo, JniArgumentValue*) + 0xa8
	I NativeAotFromAndroid:    at Java.Interop.JniPeerMembers.JniInstanceMethods.InvokeVirtualVoidMethod(String, IJavaPeerable, JniArgumentValue*) + 0x184
	I NativeAotFromAndroid:    at Android.App.Application.n_OnCreate(IntPtr jnienv, IntPtr native__this) + 0xa8
	I NativeAotFromAndroid:    at libNativeAOT!<BaseAddress>+0x4f3e44

The cause was the topmost frame: `CallVoidMethod()`, which performs a
*virtual* method invocation.  The stack overflow was that
Java `MainApplication.onCreate()` called C# `Application.n_OnCreate()`,
which called `InvokeVirtualVoidMethod()`, which did a *virtual*
invocation back on `MainApplication.onCreate()`, …

`InvokeVirtualVoidMethod()` should have been calling
`CallNonvirtualVoidMethod()`; why wasn't it?

Further investigation showed:

	Created PeerReference=0x2d06/G IdentityHashCode=0x8edcb07 Instance=0x957d2a Instance.Type=Android.App.Application, Java.Type=my/MainApplication

which at a glance seems correct, but isn't: the `Instance.Type` for
a `Java.Type` of `my/MainApplication` should be `MainApplication`,
*not* `Android.App.Application`!

Because the runtime type of this value was `Application`,
it was warranted and expected that `InvokeVirtualVoidMethod()`
would do a virtual invocation!

So, why did the avove `Created PeerReference …` line show the wrong
type?  Because `NativeAotTypeManager.CreatePeer()` needs to check
for bindings of the the runtime type of the Java handle *before*
using the `targetType` parameter, because the targetType parameter
will *never* be for that of a custom subclass.

Copy *lots* of code from dotnet/java-interop -- showing that this
needs some major cleanup & refactoring -- so that we properly check
the runtime type of `reference` + base classes when trying to determine
the type of the proxy to create.

This fixes the stack overflow.
  • Loading branch information
@jonpryor
jonpryor committed Jan 31, 2025
1 parent d54546a commit 694e975
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 73 deletions.
1 change: 1 addition & 0 deletions samples/NativeAOT/MainApplication.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public class MainApplication : Application
public MainApplication (IntPtr handle, JniHandleOwnership transfer)
: base (handle, transfer)
{
Log.Debug ("NativeAOT", $"Application..ctor({handle.ToString ("x2")}, {transfer})");
}

public override void OnCreate ()
Expand Down
223 changes: 150 additions & 73 deletions samples/NativeAOT/NativeAotValueManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ namespace NativeAOT;

internal class NativeAotValueManager : JniRuntime.JniValueManager
{
const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors;

readonly NativeAotTypeManager TypeManager;
Dictionary<int, List<IJavaPeerable>>? RegisteredInstances = new Dictionary<int, List<IJavaPeerable>>();

Expand Down Expand Up @@ -253,105 +255,180 @@ public override List<JniSurfacedPeerInfo> GetSurfacedPeers ()

public override IJavaPeerable? CreatePeer (
ref JniObjectReference reference,
JniObjectReferenceOptions options,
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)]
JniObjectReferenceOptions transfer,
[DynamicallyAccessedMembers (Constructors)]
Type? targetType)
{
if (!reference.IsValid)
if (!reference.IsValid) {
return null;
}

var peer = CreateInstance (reference.Handle, JniHandleOwnership.DoNotTransfer, targetType);
JniObjectReference.Dispose (ref reference, options);
return peer;
}
targetType = targetType ?? typeof (global::Java.Interop.JavaObject);
targetType = GetPeerType (targetType);

internal IJavaPeerable? CreateInstance (
IntPtr handle,
JniHandleOwnership transfer,
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)]
Type? targetType)
{
if (targetType.IsInterface || targetType.IsAbstract) {
var invokerType = JavaObjectExtensions.GetInvokerType (targetType);
if (invokerType == null)
throw new NotSupportedException ("Unable to find Invoker for type '" + targetType.FullName + "'. Was it linked away?",
CreateJavaLocationException ());
targetType = invokerType;
}
if (!typeof (IJavaPeerable).IsAssignableFrom (targetType))
throw new ArgumentException ($"targetType `{targetType.AssemblyQualifiedName}` must implement IJavaPeerable!", nameof (targetType));

var typeSig = TypeManager.GetTypeSignature (targetType);
if (!typeSig.IsValid || typeSig.SimpleReference == null) {
var targetSig = Runtime.TypeManager.GetTypeSignature (targetType);
if (!targetSig.IsValid || targetSig.SimpleReference == null) {
throw new ArgumentException ($"Could not determine Java type corresponding to `{targetType.AssemblyQualifiedName}`.", nameof (targetType));
}

JniObjectReference typeClass = default;
JniObjectReference handleClass = default;
var refClass = JniEnvironment.Types.GetObjectClass (reference);
JniObjectReference targetClass;
try {
try {
typeClass = JniEnvironment.Types.FindClass (typeSig.SimpleReference);
} catch (Exception e) {
throw new ArgumentException ($"Could not find Java class `{typeSig.SimpleReference}`.",
nameof (targetType),
e);
}
targetClass = JniEnvironment.Types.FindClass (targetSig.SimpleReference);
} catch (Exception e) {
JniObjectReference.Dispose (ref refClass);
throw new ArgumentException ($"Could not find Java class `{targetSig.SimpleReference}`.",
nameof (targetType),
e);
}

if (!JniEnvironment.Types.IsAssignableFrom (refClass, targetClass)) {
JniObjectReference.Dispose (ref refClass);
JniObjectReference.Dispose (ref targetClass);
return null;
}

JniObjectReference.Dispose (ref targetClass);

var proxy = CreatePeerProxy (ref refClass, targetType, ref reference, transfer);

if (proxy == null) {
throw new NotSupportedException (string.Format ("Could not find an appropriate constructable wrapper type for Java type '{0}', targetType='{1}'.",
JniEnvironment.Types.GetJniTypeNameFromInstance (reference), targetType));
}

proxy.SetJniManagedPeerState (proxy.JniManagedPeerState | JniManagedPeerStates.Replaceable);
return proxy;
}

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

static readonly Type ByRefJniObjectReference = typeof (JniObjectReference).MakeByRefType ();

IJavaPeerable? CreatePeerProxy (
ref JniObjectReference klass,
[DynamicallyAccessedMembers (Constructors)]
Type fallbackType,
ref JniObjectReference reference,
JniObjectReferenceOptions options)
{
var jniTypeName = JniEnvironment.Types.GetJniTypeNameFromClass (klass);

handleClass = JniEnvironment.Types.GetObjectClass (new JniObjectReference (handle));
if (!JniEnvironment.Types.IsAssignableFrom (handleClass, typeClass)) {
Type? type = null;
while (jniTypeName != null) {
JniTypeSignature sig;
if (!JniTypeSignature.TryParse (jniTypeName, out sig))
return null;

type = Runtime.TypeManager.GetType (sig);

if (type != null) {
var peer = TryCreatePeerProxy (type, ref reference, options);
if (peer != null) {
return peer;
}
}
} finally {
JniObjectReference.Dispose (ref handleClass);
JniObjectReference.Dispose (ref typeClass);

var super = JniEnvironment.Types.GetSuperclass (klass);
jniTypeName = super.IsValid
? JniEnvironment.Types.GetJniTypeNameFromClass (super)
: null;

JniObjectReference.Dispose (ref klass, JniObjectReferenceOptions.CopyAndDispose);
klass = super;
}
JniObjectReference.Dispose (ref klass, JniObjectReferenceOptions.CopyAndDispose);

IJavaPeerable? result = null;
return TryCreatePeerProxy (fallbackType, ref reference, options);
}

try {
result = (IJavaPeerable) CreateProxy (targetType, handle, transfer);
//if (JNIEnv.IsGCUserPeer (result.PeerReference.Handle)) {
result.SetJniManagedPeerState (JniManagedPeerStates.Replaceable | JniManagedPeerStates.Activatable);
//}
} catch (MissingMethodException e) {
var key_handle = JNIEnv.IdentityHash (handle);
JNIEnv.DeleteRef (handle, transfer);
throw new NotSupportedException (FormattableString.Invariant (
$"Unable to activate instance of type {targetType} from native handle 0x{handle:x} (key_handle 0x{key_handle:x})."), e);
static ConstructorInfo? GetActivationConstructor (
[DynamicallyAccessedMembers (Constructors)]
Type type)
{
if (type.IsAbstract || type.IsInterface) {
type = GetInvokerType (type) ?? type;
}
return result;
foreach (var c in type.GetConstructors (BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)) {
var p = c.GetParameters ();
if (p.Length == 2 && p [0].ParameterType == ByRefJniObjectReference && p [1].ParameterType == typeof (JniObjectReferenceOptions))
return c;
if (p.Length == 2 && p [0].ParameterType == typeof (IntPtr) && p [1].ParameterType == typeof (JniHandleOwnership))
return c;
}
return null;
}

[return: DynamicallyAccessedMembers (Constructors)]
static Type? GetInvokerType (Type type)
{
// https://github.com/xamarin/xamarin-android/blob/5472eec991cc075e4b0c09cd98a2331fb93aa0f3/src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs#L176-L186
const string makeGenericTypeMessage = "Generic 'Invoker' types are preserved by the MarkJavaObjects trimmer step.";

[UnconditionalSuppressMessage ("Trimming", "IL2055", Justification = makeGenericTypeMessage)]
[return: DynamicallyAccessedMembers (Constructors)]
static Type MakeGenericType (
[DynamicallyAccessedMembers (Constructors)]
Type type,
Type [] arguments) =>
// FIXME: https://github.com/dotnet/java-interop/issues/1192
#pragma warning disable IL3050
type.MakeGenericType (arguments);
#pragma warning restore IL3050

var signature = type.GetCustomAttribute<JniTypeSignatureAttribute> ();
if (signature == null || signature.InvokerType == null) {
return null;
}

Type[] arguments = type.GetGenericArguments ();
if (arguments.Length == 0)
return signature.InvokerType;

return MakeGenericType (signature.InvokerType, arguments);
}

const BindingFlags ActivationConstructorBindingFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance;


static readonly Type[] XAConstructorSignature = new Type [] { typeof (IntPtr), typeof (JniHandleOwnership) };
static readonly Type[] JIConstructorSignature = new Type [] { typeof (JniObjectReference).MakeByRefType (), typeof (JniObjectReferenceOptions) };

internal static object CreateProxy (
[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)]
Type type,
IntPtr handle,
JniHandleOwnership transfer)
protected virtual IJavaPeerable? TryCreatePeerProxy (Type type, ref JniObjectReference reference, JniObjectReferenceOptions options)
{
// Skip Activator.CreateInstance() as that requires public constructors,
// and we want to hide some constructors for sanity reasons.
BindingFlags flags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance;
var c = type.GetConstructor (flags, null, XAConstructorSignature, null);
var c = type.GetConstructor (ActivationConstructorBindingFlags, null, XAConstructorSignature, null);
if (c != null) {
return c.Invoke (new object [] { handle, transfer });
var args = new object[] {
reference.Handle,
JniHandleOwnership.DoNotTransfer,
};
var p = (IJavaPeerable) c.Invoke (args);
JniObjectReference.Dispose (ref reference, options);
return p;
}
c = type.GetConstructor (flags, null, JIConstructorSignature, null);
c = type.GetConstructor (ActivationConstructorBindingFlags, null, JIConstructorSignature, null);
if (c != null) {
JniObjectReference r = new JniObjectReference (handle);
JniObjectReferenceOptions o = JniObjectReferenceOptions.Copy;
var peer = (IJavaPeerable) c.Invoke (new object [] { r, o });
JNIEnv.DeleteRef (handle, transfer);
return peer;
var args = new object[] {
reference,
options,
};
var p = (IJavaPeerable) c.Invoke (args);
reference = (JniObjectReference) args [0];
return p;
}
throw new MissingMethodException (
"No constructor found for " + type.FullName + "::.ctor(System.IntPtr, Android.Runtime.JniHandleOwnership)",
CreateJavaLocationException ());
}

static Exception CreateJavaLocationException ()
{
using (var loc = new Java.Lang.Error ("Java callstack:"))
return new JavaLocationException (loc.ToString ());
return null;
}
}

0 comments on commit 694e975

Please sign in to comment.