Skip to content

Commit

Permalink
[generator] Use proper syntax for nested classes for DIM invokers (#662)
Browse files Browse the repository at this point in the history 
Fixes: #661

App crashes with a `TypeLoadException` when instantiating a class
which implements an interface containing default interface members:

 1. Create a new app within Visual Studio 16.6 or 16.7.

 2. Set the `$(TargetFrameworkVersion)` to v10.0.99.  This pulls in a
    `Mono.Android.dll` which uses default interface methods.

 3. Add the following class to the project.  The
    `Android.App.Application.IActivityLifecycleCallbacks` interface
    is (1) a nested type, and (2) contains default interface methods.

        class ActivityLifecycleContextListener : Java.Lang.Object, Android.App.Application.IActivityLifecycleCallbacks
        {
            public void OnActivityCreated (Activity activity, Bundle savedInstanceState)    => throw new NotImplementedException ();
            public void OnActivityDestroyed (Activity activity)                             => throw new NotImplementedException ();
            public void OnActivityPaused (Activity activity)                                => throw new NotImplementedException ();
            public void OnActivityResumed (Activity activity)                               => throw new NotImplementedException ();
            public void OnActivitySaveInstanceState (Activity activity, Bundle outState)    => throw new NotImplementedException ();
            public void OnActivityStarted (Activity activity)                               => throw new NotImplementedException ();
            public void OnActivityStopped (Activity activity)                               => throw new NotImplementedException ();
        }

 4. Instantiate `ActivityLifecycleContextListener` within the app's
    `OnCreate()` method:

        var a = new ActivityLifecycleContextListener ();

Expected results: it works!

Actual results:

	System.TypeLoadException: Could not load type 'Android.App.Application.IActivityLifecycleCallbacks' from assembly 'Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'.
	  at System.RuntimeType.GetType (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, Sys06-08 19:19:38.543 I/MonoDroid( 5187):   at (wrapper managed-to-native) System.RuntimeTypeHandle.internal_from_name(string,System.Threading.StackCrawlMark&,System.Reflection.Assembly,bool,bool,bool)
	  at System.RuntimeTypeHandle.GetTypeByName (System.String typeName, System.Boolean throwOnError, System.Boolean ignoreCase, System.Boolean reflectionOnly, System.Threading.StackCrawlMark& stackMark, System.Boolean loadTypeFromPartialName)
	  at (wrapper managed-to-native) Java.Interop.NativeM06-08
	  at System.Type.GetType (System.String typeName, System.Boolean throwOnError)
	  at Android.Runtime.AndroidTypeManager.RegisterNativeMembers (Java.Interop.JniType nativeClass, System.Type type, System.String methods)
	  at Android.Runtime.JNIEnv.RegisterJniNatives (System.IntPtr typeName_ptr, System.Int32 typeName_len, System.IntPtr jniClass, System.IntPtr methods_ptr, System.Int32 methods_len)
	  …

The cause of the bug is that Xamarin.Android attempts to call:

	Type.GetType ("Android.App.Application.IActivityLifecycleCallbacks, Mono.Android");

which fails, because this is the wrong syntax for looking up nested
types via Reflection.  Xamarin.Android should instead be doing:

	Type.GetType ("Android.App.Application/IActivityLifecycleCallbacks, Mono.Android");

Note `/` instead of `.` in the type name.

Why was Xamarin.Android trying to load the wrong type?  Because the
*Java Callable Wrapper* contained the wrong type, and Xamarin.Android
used the string as-is:

	public /* partial */ class ActivityLifecycleContextListener extends java.lang.Object
		implements mono.android.IGCUserPeer, android.app.Application.ActivityLifecycleCallbacks
	{
	/** @hide */
	    public static final String __md_methods;
	    static {
	        __md_methods = 
	            "n_onActivityCreated:(Landroid/app/Activity;Landroid/os/Bundle;)V:GetOnActivityCreated_Landroid_app_Activity_Landroid_os_Bundle_Handler:Android.App.Application/IActivityLifecycleCallbacksInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null\n" +
	            …
	            "n_onActivityPreStopped:(Landroid/app/Activity;)V:GetOnActivityPreStopped_Landroid_app_Activity_Handler:Android.App.Application.IActivityLifecycleCallbacks, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null\n" +
	            "";
	        …
	    }
	}

"Interesting" here is the difference between the first and the last
entry: the first entry is for a "normal" interface method, which uses
the required `/` nested type separator, while the last entry is for a
Java default interface method.

Aside/TODO: Why is `n_onActivityPreStopped()` being registered when
`Application.IActivityLifecycleCallbacks.OnActivityPreStopped()` is
not implemented in `ActivityLifecycleContextListener`?

Where do the `__md_methods` entries come from?  From the binding
assembly; from `generator` output!

	public partial interface IActivityLifecycleCallbacks : IJavaObject, IJavaPeerable {
	    // Metadata.xml XPath method reference: path="/api/package[@name='android.app']/interface[@name='Application.ActivityLifecycleCallbacks']/method[@name='onActivityPreStopped' and count(parameter)=1 and parameter[1][@type='android.app.Activity']]"
	    [Register ("onActivityPreStopped", "(Landroid/app/Activity;)V", "GetOnActivityPreStopped_Landroid_app_Activity_Handler:Android.App.Application.IActivityLifecycleCallbacks, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", ApiSince = 29)]
	    virtual unsafe void OnActivityPreStopped (Android.App.Activity activity) {…}
	}

The 3rd `RegisterAttribute` parameter, which becomes the
`RegisterAttribute.Connector` property, is where the offending string
comes from:

	…:Android.App.Application.IActivityLifecycleCallbacks, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null"

The fix is to update `Method.GetConectorNameFull()` to use
`DeclaringType.AssemblyQualifiedName`, *not* `DeclaringType.FullName`,
as the former is what's needed for reflection compatibility, while the
latter is a "C#" name, generally more useful for error messages.
  • Loading branch information
@jpobst
jpobst authored Jun 11, 2020
1 parent 267c3f3 commit 9a56465
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
31 changes: 31 additions & 0 deletions tests/generator-Tests/Unit-Tests/DefaultInterfaceMethodsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -431,5 +431,36 @@ public void DontInvalidateInterfaceDueToStaticOrDefaultMethods ()
Assert.False (generated.Contains ("StaticMethod"));
Assert.False (generated.Contains ("DefaultMethod"));
}

[Test]
public void GenerateProperNestedInterfaceSignatures ()
{
// https://github.com/xamarin/java.interop/issues/661
// Ensure that when we write the invoker type for a nested default interface method
// we use `/` to denote nested as needed by Type.GetType ()
var xml = @"<api>
<package name='java.lang' jni-name='java/lang'>
<class abstract='false' deprecated='not deprecated' final='false' name='Object' static='false' visibility='public' jni-signature='Ljava/lang/EmptyOverrideClass;' />
</package>
<package name='com.xamarin.android' jni-name='com/xamarin/android'>
<class extends='java.lang.Object' abstract='true' deprecated='not deprecated' final='false' name='Application' static='true' visibility='public' jni-signature='Landroid/app/Application$ActivityLifecycleCallbacks;' />
<interface abstract='true' deprecated='not deprecated' final='false' name='Application.ActivityLifecycleInterface' static='true' visibility='public' jni-signature='Landroid/app/Application$ActivityLifecycleCallbacks;'>
<method abstract='false' deprecated='not deprecated' final='false' name='onActivityDestroyed' jni-signature='(Landroid/app/Activity;)V' bridge='false' native='false' return='void' jni-return='V' static='false' synchronized='false' synthetic='false' visibility='public'>
<parameter name='activity' type='int' jni-type='I' not-null='true'></parameter>
</method>
</interface>
</package>
</api>";

var gens = ParseApiDefinition (xml);
var iface = gens[1].NestedTypes.OfType<InterfaceGen> ().Single ();

generator.WriteInterface (iface, string.Empty, new GenerationInfo (string.Empty, string.Empty, "MyAssembly"));

var generated = writer.ToString ();

Assert.True (generated.Contains ("GetOnActivityDestroyed_IHandler:Com.Xamarin.Android.Application/IActivityLifecycleInterface, MyAssembly"));
Assert.False (generated.Contains ("GetOnActivityDestroyed_IHandler:Com.Xamarin.Android.Application.IActivityLifecycleInterface, MyAssembly"));
}
}
}
2 changes: 1 addition & 1 deletion tools/generator/Java.Interop.Tools.Generator.ObjectModel/Method.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ internal string GetAdapterName (CodeGenerationOptions opt, string adapter)
}

// Connectors for DIM are defined on the interface, not the implementing type
public string GetConnectorNameFull (CodeGenerationOptions opt) => ConnectorName + (opt.SupportDefaultInterfaceMethods && IsInterfaceDefaultMethod ? $":{DeclaringType.FullName}, " + (AssemblyName ?? opt.AssemblyName) : string.Empty);
public string GetConnectorNameFull (CodeGenerationOptions opt) => ConnectorName + (opt.SupportDefaultInterfaceMethods && IsInterfaceDefaultMethod ? $":{DeclaringType.AssemblyQualifiedName}, " + (AssemblyName ?? opt.AssemblyName) : string.Empty);

internal string GetDelegateType (CodeGenerationOptions opt) => opt.GetJniMarshalDelegate (this);

Expand Down

0 comments on commit 9a56465

Please sign in to comment.