Skip to content

Commit 47b6e7f

Browse files
radekdoulikjonpryor
authored andcommitted
[jnimarshalmethod-gen] Do not use method cache for generic types (#372)
Commit 7587345 updated `jnimarshalmethod-gen.exe` so that instead of emitting the marshal methods into a new assembly, they would instead by inserted into the declaring assembly. This was done in order to avoid visibility issues: if the declaring assembly has a `private` method for which we need to emit a marshal method, the `-JniMarshalMethod.dll` assembly won't be able to reference the method, which resulted in runtime exceptions due to visibility. Updating the original assembly was done by generating the previously used `-JniMarshalMethod.dll` assembly, then "moving" the contents of the new assembly into the original assembly, via `TypeMover`. For performance reasons, `TypeMover` uses a variety of caches in order to speed up speed up member lookup while moving type members. Unfortunately, `TypeMover.GetUpdatedMethod()`'s cache use had a bug: 1. Marshal method *registration* involves generic types such as `System.Func<...>` and `System.Action<...>`. 2. `TypeMover.GetUpdatedMethod()` knew ~nothing of generics. 3. `TypeMover.GetUpdatedMethod()` is used with constructors, too. Thus, consider the following: partial class ExportTest : JavaObject { [JavaCallable ("staticActionInt", Signature="(I)V")] public static void StaticActionInt (int i) {} [JavaCallable ("staticActionFloat", Signature="(F)V")] public static void StaticActionFloat (float f) {} } Here we have two methods which will use an ``Action`3<,,>`` for method registration purposes. `StaticActionInt()` should use an `Action<IntPtr, IntPtr, int>`, while `StaticActionFloat()` should use an `Action<IntPtr, IntPtr, float>`. What we *should* wind up with is: partial class ExportTest { // generated via jnimarshalmethod-gen.exe partial class '__<$>_jni_marshal_methods' { [JniAddNativeMethodRegistration] public static void __RegisterNativeMembers (JniNativeMethodRegistrationArguments args) { args.AddRegistrations ( new JniNativeMethodRegistration ("staticActionInt", "(I)V", (Action<IntPtr, IntPtr, int>) StaticActionInt), new JniNativeMethodRegistration ("staticActionFloat", "(F)V", (Action<IntPtr, IntPtr, float>) StaticActionFloat) ); } public static void StaticActionInt (IntPtr __jnienv, IntPtr __class, int i) {...} public static void StaticActionFloat (IntPtr __jnienv, IntPtr __class, float f) {...} } } What we *actually* would up with was that the delegate types used in the `JniNativeMethodRegistration` used were identical! new JniNativeMethodRegistration ("staticActionInt", "(I)V", (Action<IntPtr, IntPtr, float>) StaticActionInt), new JniNativeMethodRegistration ("staticActionFloat", "(F)V", (Action<IntPtr, IntPtr, float>) StaticActionFloat), `ExportTest.__<$>_jni_marshal_methods.StaticActionInt()` cannot be converted to an `Action<IntPtr, IntPtr, float>`, and attempting to do so would fail at runtime: EXEC : 1) error : Java.InteropTests.MarshalMemberBuilderTest.AddExportMethods [/Users/rodo/git/java.interop/build-tools/scripts/RunNUnitTests.targets] Java.Interop.JavaException : Could not initialize class com.xamarin.interop.export.ExportType at Java.Interop.JniEnvironment+StaticMethods.GetStaticMethodID (Java.Interop.JniObjectReference type, System.String name, System.String signature) [0x00076] in <3847561eba6c484999618e90b1233f35>:0 at Java.Interop.JniType.GetStaticMethod (System.String name, System.String signature) [0x0000e] in <3847561eba6c484999618e90b1233f35>:0 at Java.InteropTests.MarshalMemberBuilderTest.AddExportMethods () [0x000ca] in <60aa2b03bbb74190a217a527c1414772>:0 at (wrapper managed-to-native) System.Reflection.MonoMethod.InternalInvoke(System.Reflection.MonoMethod,object,object[],System.Exception&) at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x0003b] in <8a67b847911f4fb589a9e7588d87a55a>:0 --- End of managed Java.Interop.JavaException stack trace --- java.lang.NoClassDefFoundError: Could not initialize class com.xamarin.interop.export.ExportType Notably, the above error doesn't make immediate sense -- it's not that the Java-side type couldn't be initialized! -- but it is indicative that something "wasn't right". Equally importantly, `peverify` wasn't happy: $ peverify bin/TestDebug/Java.Interop.Export-Tests.dll In method: Java.InteropTests.ExportTest/__<$>_jni_marshal_methods::__RegisterNativeMembers(JniNativeMethodRegistrationArguments) Not Verifiable: Function pointer signature 'intptr,intptr,int' doesn't match delegate's signature 'intptr,intptr,single' at 0x012c Not Verifiable: Function pointer signature 'intptr,intptr,intptr' doesn't match delegate's signature 'intptr,intptr,single' at 0x010a Not Verifiable: Function pointer signature 'intptr,intptr,intptr' doesn't match delegate's signature 'intptr,intptr,single' at 0x00e8 Not Verifiable: Function pointer signature 'intptr,intptr' doesn't match delegate's signature 'intptr,intptr' at 0x00a4 In method: Java.InteropTests.MarshalMemberBuilderTest::CreateExportTest(JniType) Not Verifiable: Incompatible parameter with function signature: Calling method with signature (Java.Interop.JniArgumentValue*) but for argument 1 there is a (intptr (Native Int)) on stack at 0x0011 Fix `TypeMover.GetUpdatedMethod()` so that methods on generic types are not cached. This will cause some method lookups to be slower, but allows the resulting IL to be semantically correct.
1 parent 89385a2 commit 47b6e7f

File tree

4 files changed

+20
-4
lines changed

4 files changed

+20
-4
lines changed

src/Java.Interop.Export/Tests/Java.Interop/ExportTest.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,16 @@ public void InstanceActionIJavaObject (JavaObject test)
7373
public static void StaticActionIJavaObject (JavaObject test)
7474
{
7575
}
76+
77+
[JavaCallable ("staticActionInt", Signature="(I)V")]
78+
public static void StaticActionInt (int i)
79+
{
80+
}
81+
82+
[JavaCallable ("staticActionFloat", Signature="(F)V")]
83+
public static void StaticActionFloat (float f)
84+
{
85+
}
7686
}
7787

7888
[JniValueMarshaler (typeof (MyColorValueMarshaler))]

src/Java.Interop.Export/Tests/Java.Interop/MarshalMemberBuilderTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public void AddExportMethods ()
2222
var methods = CreateBuilder ()
2323
.GetExportedMemberRegistrations (typeof (ExportTest))
2424
.ToList ();
25-
Assert.AreEqual (8, methods.Count);
25+
Assert.AreEqual (10, methods.Count);
2626

2727
Assert.AreEqual ("action", methods [0].Name);
2828
Assert.AreEqual ("()V", methods [0].Signature);

src/Java.Interop.Export/Tests/java/com/xamarin/interop/export/ExportType.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,19 @@ public void testMethods () {
4040
throw new Error ("funcInt64() should return 42!");
4141

4242
Object o = funcIJavaObject ();
43-
if (o != this)
44-
throw new Error ("funcIJavaObject() should return `this`!");
43+
if (o != this)
44+
throw new Error ("funcIJavaObject() should return `this`!");
45+
46+
staticActionInt (1);
47+
staticActionFloat (2.0f);
4548
}
4649

4750
public native void action ();
4851
public native void actionIJavaObject (Object test);
4952
public native long funcInt64 ();
5053
public native Object funcIJavaObject ();
54+
public native void staticActionInt (int i);
55+
public native void staticActionFloat (float f);
5156

5257
ArrayList<Object> managedReferences = new ArrayList<Object>();
5358

tools/jnimarshalmethod-gen/TypeMover.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,8 @@ static MethodReference GetUpdatedMethod (MethodReference method, ModuleDefinitio
268268
foreach (var p in mr.Parameters)
269269
p.ParameterType = GetUpdatedType (p.ParameterType, module);
270270

271-
methodMap [method] = mr;
271+
if (method.DeclaringType != null && !method.DeclaringType.HasGenericParameters)
272+
methodMap [method] = mr;
272273

273274
return mr;
274275
}

0 commit comments

Comments
 (0)