Skip to content
This repository was archived by the owner on Jul 6, 2026. It is now read-only.

[Java.Interop.Export] Remove Action/Func use. - #635

Merged
jonpryor merged 1 commit into
dotnet:masterfrom
jonpryor:jonp-no-action-in-MemberBuilder
May 1, 2020
Merged

[Java.Interop.Export] Remove Action/Func use.#635
jonpryor merged 1 commit into
dotnet:masterfrom
jonpryor:jonp-no-action-in-MemberBuilder

Conversation

@jonpryor

Copy link
Copy Markdown
Contributor

Context: dotnet/android#4631
Context: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3685184&view=logs&j=b1314ebb-4483-559c-0367-730fc62c4440&t=5022c07e-db44-5ce4-1a38-092215b61c50
Context: 70fc4c0

Commit 70fc4c0 broke use of jnimarshalmethod-gen.exe within
xamarin-android and Mono.Android.dll, as it allows methods taking
more than 14 parameters to be bound, which results in
jnimarshalmethod-gen.exe trying to use an Action<…> or
Func<…> that takes more than 16 parameters, which isn't allowed:

EXEC : error : jnimarshalmethod-gen: Unable to process assembly '…/xamarin-android/bin/Release/lib/xamarin.android/xbuild-frameworks/MonoAndroid/v10.0/Mono.Android.dll'
  An incorrect number of type arguments were specified for the declaration of an Action type.
  Parameter name: typeArgs
  System.ArgumentException: An incorrect number of type arguments were specified for the declaration of an Action type.
  Parameter name: typeArgs
    at System.Linq.Expressions.Expression.GetActionType (System.Type[] typeArgs)
    at Java.Interop.MarshalMemberBuilder.CreateMarshalToManagedExpression (System.Reflection.MethodInfo method, Java.Interop.JavaCallableAttribute callable, System.Type type)
    at Java.Interop.MarshalMemberBuilder.CreateMarshalToManagedExpression (System.Reflection.MethodInfo method)
    at Xamarin.Android.Tools.JniMarshalMethodGenerator.App.CreateMarshalMethodAssembly (System.String path)
    at Xamarin.Android.Tools.JniMarshalMethodGenerator.App.ProcessAssemblies (System.Collections.Generic.List`1[T] assemblies)

Looks like we don't actually need to lookup a delegate type, as
there's an Expression.Lambda() overload which only requires the
lambda body and parameters. Use that instead.

Remove use of Expression.GetActionType() and
Expression.GetFuncType(), as these will fail when dealing with
methods that contain more than 14 parameters:

Context: dotnet/android#4631
Context: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3685184&view=logs&j=b1314ebb-4483-559c-0367-730fc62c4440&t=5022c07e-db44-5ce4-1a38-092215b61c50
Context: 70fc4c0

Commit 70fc4c0 broke use of `jnimarshalmethod-gen.exe` within
xamarin-android and `Mono.Android.dll`, as it allows methods taking
more than 14 parameters to be bound, which results in
`jnimarshalmethod-gen.exe` trying to use an `Action<…>` or
`Func<…>` that takes more than 16 parameters, which isn't allowed:

	EXEC : error : jnimarshalmethod-gen: Unable to process assembly '…/xamarin-android/bin/Release/lib/xamarin.android/xbuild-frameworks/MonoAndroid/v10.0/Mono.Android.dll'
	  An incorrect number of type arguments were specified for the declaration of an Action type.
	  Parameter name: typeArgs
	  System.ArgumentException: An incorrect number of type arguments were specified for the declaration of an Action type.
	  Parameter name: typeArgs
	    at System.Linq.Expressions.Expression.GetActionType (System.Type[] typeArgs)
	    at Java.Interop.MarshalMemberBuilder.CreateMarshalToManagedExpression (System.Reflection.MethodInfo method, Java.Interop.JavaCallableAttribute callable, System.Type type)
	    at Java.Interop.MarshalMemberBuilder.CreateMarshalToManagedExpression (System.Reflection.MethodInfo method)
	    at Xamarin.Android.Tools.JniMarshalMethodGenerator.App.CreateMarshalMethodAssembly (System.String path)
	    at Xamarin.Android.Tools.JniMarshalMethodGenerator.App.ProcessAssemblies (System.Collections.Generic.List`1[T] assemblies)

Update `jnimarshalmethod-gen.exe` so that it *skips* methods which
take more than 14 parameters.  This is to unblock xamarin-android
integrations.

(Hopefully this in itself will be enough for xamarin-android to build.)

Next, *begin* plumbing support for this scenario.

To remove the workaround and get things fully working (failing),
rebuild `jnimarshalmethod-gen.exe` with:

	msbuild /restore tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj /p:_AllTheArguments=True

Also update `jnimarshalmethod-gen.exe` so that it *optionally* uses
`Mono.Linq.Expressions`, for additional debugging output:

	msbuild /restore tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj /p:_DumpRegisterNativeMembers=True

(Use `/p:_AllTheArguments=True /p:_DumpRegisterNativeMembers=True`
for both!)

Add a method to `ExportTest.cs` / `ExportType.java` which takes more
than 14 parameters, and update `MarshalMemberBuilder` so that it can
generate the appropriate JNI Marshal Method for such a method.
(This should help avoid the need for xamarin-android integrations to
see what's broken.)

The complexity of supporting more than 14 parameters doesn't like in
`MarshalMemberBuilder`, per-se; `Java.Interop.Export-Tests.dll` runs
just fine when just using `Expression.Lambda(body, bodyParams)`.
Things get much more complicated when `jnimarshalmethod-gen.exe`
enters the picture, and interacts with `MarshalMemberBuilder`.
`jnimarshalmethod-gen.exe` *requires* an "appropriate" delegate type.

Update `MarshalMemberBuilder` to "know about" the `_JniMarshal*`
delegates added in cbb50af, and attempt to use them from the assembly
that is being processed, if they exist.

This got further, resulting in an error that
`_JniMarshal_PPZBCSIJFDLLLLLDFJ_Z` can't be used as a constant (?!).

Address *that* error by updating `App.cs` so that it emits a
`Type.GetType("_JniMarshal_PPZBCSIJFDLLLLLDFJ_Z", true)`.  This allows
`jnimarshalmethod-gen.exe` to execute successfully.

Then we try *running* it, and it fails:

	$ make run-test-jnimarshal
	…
	EXEC : 1) error : Java.InteropTests.MarshalMemberBuilderTest.AddExportMethods […/Java.Interop/build-tools/scripts/RunNUnitTests.targets]
	  Java.Interop.JavaException : com.xamarin.interop.export.ExportType.staticAction()V
	    at Java.Interop.JniEnvironment+StaticMethods.CallStaticVoidMethod (Java.Interop.JniObjectReference type, Java.Interop.JniMethodInfo method)
	    at Java.InteropTests.MarshalMemberBuilderTest.AddExportMethods ()
	    at (wrapper managed-to-native) System.Reflection.RuntimeMethodInfo.InternalInvoke(System.Reflection.RuntimeMethodInfo,object,object[],System.Exception&)
	    at System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
	    --- End of managed Java.Interop.JavaException stack trace ---
	  java.lang.UnsatisfiedLinkError: com.xamarin.interop.export.ExportType.staticAction()V
	  	at com.xamarin.interop.export.ExportType.staticAction(Native Method)
	  	at com.xamarin.interop.export.ExportType.testStaticMethods(ExportType.java:26)>

Add more printfs, and we get the problem:

	System.InvalidProgramException: Invalid IL code in Java.InteropTests.ExportTest/__<$>_jni_marshal_methods:__RegisterNativeMembers (Java.Interop.JniNativeMethodRegistrationArguments): IL_0176: ldloc.0

At this point things don't make sense to me.
The `__RegisterNativeMembers()` method is:

	## Dumping contents of `Java.InteropTests.ExportTest::__RegisterNativeMembers`:
	void (JniNativeMethodRegistrationArguments args)
	{
	    Type targetType;

	    targetType = Type.GetType("Java.InteropTests.ExportTest");
	    args.AddRegistrations(new JniNativeMethodRegistration[] {
	        new JniNativeMethodRegistration("funcIJavaObject", "()Ljava/lang/Object;",
	            Delegate.CreateDelegate(System.Func`3[System.IntPtr,System.IntPtr,System.IntPtr], targetType, "FuncIJavaObject")),
	        new JniNativeMethodRegistration("funcInt64", "()J",
	            Delegate.CreateDelegate(System.Func`3[System.IntPtr,System.IntPtr,System.Int64], targetType, "FuncInt64")),
	        new JniNativeMethodRegistration("action", "()V",
	            Delegate.CreateDelegate(System.Action`2[System.IntPtr,System.IntPtr], targetType, "InstanceAction")),
	        new JniNativeMethodRegistration("actionIJavaObject", "(Ljava/lang/Object;)V",
	            Delegate.CreateDelegate(System.Action`3[System.IntPtr,System.IntPtr,System.IntPtr], targetType, "InstanceActionIJavaObject")),
	        new JniNativeMethodRegistration("staticAction", "()V",
	            Delegate.CreateDelegate(System.Action`2[System.IntPtr,System.IntPtr], targetType, "StaticAction")),
	        new JniNativeMethodRegistration("staticActionFloat", "(F)V",
	            Delegate.CreateDelegate(System.Action`3[System.IntPtr,System.IntPtr,System.Single], targetType, "StaticActionFloat")),
	        new JniNativeMethodRegistration("staticActionIJavaObject", "(Ljava/lang/Object;)V",
	            Delegate.CreateDelegate(System.Action`3[System.IntPtr,System.IntPtr,System.IntPtr], targetType, "StaticActionIJavaObject")),
	        new JniNativeMethodRegistration("staticActionInt", "(I)V",
	            Delegate.CreateDelegate(System.Action`3[System.IntPtr,System.IntPtr,System.Int32], targetType, "StaticActionInt")),
	        new JniNativeMethodRegistration("staticActionInt32String", "(ILjava/lang/String;)V",
	            Delegate.CreateDelegate(System.Action`4[System.IntPtr,System.IntPtr,System.Int32,System.IntPtr], targetType, "StaticActionInt32String")),
	        new JniNativeMethodRegistration("staticFuncMyLegacyColorMyColor_MyColor", "(II)I",
	            Delegate.CreateDelegate(System.Func`5[System.IntPtr,System.IntPtr,System.Int32,System.Int32,System.Int32], targetType, "StaticFuncMyLegacyColorMyColor_MyColor")),
	        new JniNativeMethodRegistration("staticFuncThisMethodTakesLotsOfParameters", "(ZBCSIJFDLjava/lang/Object;Ljava/lang/String;Ljava/util/ArrayList;Ljava/lang/String;Ljava/lang/Object;DFJ)Z",
	            Delegate.CreateDelegate((Type)Type.GetType("_JniMarshal_PPZBCSIJFDLLLLLDFJ_Z", true), targetType, "StaticFuncThisMethodTakesLotsOfParameters"))
	    });
	}

The offending IL fragment:

	…
	IL_0161:  ldstr      "staticFuncThisMethodTakesLotsOfParameters"
	IL_0166:  ldstr      "(ZBCSIJFDLjava/lang/Object;Ljava/lang/String;Ljava/util/ArrayList;Ljava/lang/String;Ljava/lang/Object;DFJ)Z"
	IL_016b:  ldstr      "_JniMarshal_PPZBCSIJFDLLLLLDFJ_Z"
	IL_0170:  ldc.i4.1
	IL_0171:  call       [mscorlib]System.Type [mscorlib]System.Type::GetType(string, bool)
	IL_0176:  ldloc.0
	IL_0177:  ldstr      "StaticFuncThisMethodTakesLotsOfParameters"
	IL_017c:  call       [mscorlib]System.Delegate [mscorlib]System.Delegate::CreateDelegate([mscorlib]System.Type, [mscorlib]System.Type, string)
	IL_0181:  newobj     instance void [Java.Interop]Java.Interop.JniNativeMethodRegistration::.ctor(string, string, [mscorlib]System.Delegate)

I don't understand where the `ldloc.0` is coming from, or why it's
there.

Punting a complete fix for now.

To repro the crash:

	$ rm -Rf bin/TestDebug
	$ msbuild
	$ msbuild /restore tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj /p:_AllTheArguments=True /p:_DumpRegisterNativeMembers=True
	$ make run-test-jnimarshal
@jonpryor
jonpryor force-pushed the jonp-no-action-in-MemberBuilder branch from 79eed70 to 8074e03 Compare May 1, 2020 20:43
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request May 1, 2020
@jonpryor
jonpryor merged commit 857b9a9 into dotnet:master May 1, 2020
jonpryor added a commit that referenced this pull request May 6, 2020
Context: dotnet/android#4631
Context: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=3685184&view=logs&j=b1314ebb-4483-559c-0367-730fc62c4440&t=5022c07e-db44-5ce4-1a38-092215b61c50
Context: 70fc4c0

Commit 70fc4c0 broke use of `jnimarshalmethod-gen.exe` within
xamarin-android and `Mono.Android.dll`, as it allows methods taking
more than 14 parameters to be bound, which results in
`jnimarshalmethod-gen.exe` trying to use an `Action<…>` or
`Func<…>` that takes more than 16 parameters, which isn't allowed:

	EXEC : error : jnimarshalmethod-gen: Unable to process assembly '…/xamarin-android/bin/Release/lib/xamarin.android/xbuild-frameworks/MonoAndroid/v10.0/Mono.Android.dll'
	  An incorrect number of type arguments were specified for the declaration of an Action type.
	  Parameter name: typeArgs
	  System.ArgumentException: An incorrect number of type arguments were specified for the declaration of an Action type.
	  Parameter name: typeArgs
	    at System.Linq.Expressions.Expression.GetActionType (System.Type[] typeArgs)
	    at Java.Interop.MarshalMemberBuilder.CreateMarshalToManagedExpression (System.Reflection.MethodInfo method, Java.Interop.JavaCallableAttribute callable, System.Type type)
	    at Java.Interop.MarshalMemberBuilder.CreateMarshalToManagedExpression (System.Reflection.MethodInfo method)
	    at Xamarin.Android.Tools.JniMarshalMethodGenerator.App.CreateMarshalMethodAssembly (System.String path)
	    at Xamarin.Android.Tools.JniMarshalMethodGenerator.App.ProcessAssemblies (System.Collections.Generic.List`1[T] assemblies)

Update `jnimarshalmethod-gen.exe` so that it *skips* methods which
take more than 14 parameters.  This is to unblock xamarin-android
integrations.

Next, *begin* plumbing support for this scenario.

To remove the workaround and get things fully working (failing),
rebuild `jnimarshalmethod-gen.exe` with:

	msbuild /restore tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj /p:_AllTheArguments=True

Also update `jnimarshalmethod-gen.exe` so that it *optionally* uses
`Mono.Linq.Expressions`, for additional debugging output:

	msbuild /restore tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj /p:_DumpRegisterNativeMembers=True

(Use `/p:_AllTheArguments=True /p:_DumpRegisterNativeMembers=True`
for both!)

Add a method to `ExportTest.cs` / `ExportType.java` which takes more
than 14 parameters, and update `MarshalMemberBuilder` so that it can
generate the appropriate JNI Marshal Method for such a method.
(This should help avoid the need for xamarin-android integrations to
see what's broken.)

The complexity of supporting more than 14 parameters doesn't lie in
`MarshalMemberBuilder`, per-se; `Java.Interop.Export-Tests.dll` runs
just fine when just using `Expression.Lambda(body, bodyParams)`.
Things get much more complicated when `jnimarshalmethod-gen.exe`
enters the picture, and interacts with `MarshalMemberBuilder`.
`jnimarshalmethod-gen.exe` *requires* an "appropriate" delegate type.

Update `MarshalMemberBuilder` to "know about" the `_JniMarshal*`
delegates added in cbb50af, and attempt to use them from the assembly
that is being processed, if they exist.

This got further, resulting in an error that
`_JniMarshal_PPZBCSIJFDLLLLLDFJ_Z` can't be used as a constant (?!).

Address *that* error by updating `App.cs` so that it emits a
`Type.GetType("_JniMarshal_PPZBCSIJFDLLLLLDFJ_Z", true)`.  This allows
`jnimarshalmethod-gen.exe` to execute successfully.

Then we try *running* it, and it fails:

	$ make run-test-jnimarshal
	…
	EXEC : 1) error : Java.InteropTests.MarshalMemberBuilderTest.AddExportMethods […/Java.Interop/build-tools/scripts/RunNUnitTests.targets]
	  Java.Interop.JavaException : com.xamarin.interop.export.ExportType.staticAction()V
	    at Java.Interop.JniEnvironment+StaticMethods.CallStaticVoidMethod (Java.Interop.JniObjectReference type, Java.Interop.JniMethodInfo method)
	    at Java.InteropTests.MarshalMemberBuilderTest.AddExportMethods ()
	    at (wrapper managed-to-native) System.Reflection.RuntimeMethodInfo.InternalInvoke(System.Reflection.RuntimeMethodInfo,object,object[],System.Exception&)
	    at System.Reflection.RuntimeMethodInfo.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture)
	    --- End of managed Java.Interop.JavaException stack trace ---
	  java.lang.UnsatisfiedLinkError: com.xamarin.interop.export.ExportType.staticAction()V
	  	at com.xamarin.interop.export.ExportType.staticAction(Native Method)
	  	at com.xamarin.interop.export.ExportType.testStaticMethods(ExportType.java:26)>

Add more printfs, and we get the problem:

	System.InvalidProgramException: Invalid IL code in Java.InteropTests.ExportTest/__<$>_jni_marshal_methods:__RegisterNativeMembers (Java.Interop.JniNativeMethodRegistrationArguments): IL_0176: ldloc.0

At this point things don't make sense to me.
The `__RegisterNativeMembers()` method is:

	## Dumping contents of `Java.InteropTests.ExportTest::__RegisterNativeMembers`:
	void (JniNativeMethodRegistrationArguments args)
	{
	    Type targetType;

	    targetType = Type.GetType("Java.InteropTests.ExportTest");
	    args.AddRegistrations(new JniNativeMethodRegistration[] {
	        new JniNativeMethodRegistration("funcIJavaObject", "()Ljava/lang/Object;",
	            Delegate.CreateDelegate(System.Func`3[System.IntPtr,System.IntPtr,System.IntPtr], targetType, "FuncIJavaObject")),
	        new JniNativeMethodRegistration("funcInt64", "()J",
	            Delegate.CreateDelegate(System.Func`3[System.IntPtr,System.IntPtr,System.Int64], targetType, "FuncInt64")),
	        new JniNativeMethodRegistration("action", "()V",
	            Delegate.CreateDelegate(System.Action`2[System.IntPtr,System.IntPtr], targetType, "InstanceAction")),
	        new JniNativeMethodRegistration("actionIJavaObject", "(Ljava/lang/Object;)V",
	            Delegate.CreateDelegate(System.Action`3[System.IntPtr,System.IntPtr,System.IntPtr], targetType, "InstanceActionIJavaObject")),
	        new JniNativeMethodRegistration("staticAction", "()V",
	            Delegate.CreateDelegate(System.Action`2[System.IntPtr,System.IntPtr], targetType, "StaticAction")),
	        new JniNativeMethodRegistration("staticActionFloat", "(F)V",
	            Delegate.CreateDelegate(System.Action`3[System.IntPtr,System.IntPtr,System.Single], targetType, "StaticActionFloat")),
	        new JniNativeMethodRegistration("staticActionIJavaObject", "(Ljava/lang/Object;)V",
	            Delegate.CreateDelegate(System.Action`3[System.IntPtr,System.IntPtr,System.IntPtr], targetType, "StaticActionIJavaObject")),
	        new JniNativeMethodRegistration("staticActionInt", "(I)V",
	            Delegate.CreateDelegate(System.Action`3[System.IntPtr,System.IntPtr,System.Int32], targetType, "StaticActionInt")),
	        new JniNativeMethodRegistration("staticActionInt32String", "(ILjava/lang/String;)V",
	            Delegate.CreateDelegate(System.Action`4[System.IntPtr,System.IntPtr,System.Int32,System.IntPtr], targetType, "StaticActionInt32String")),
	        new JniNativeMethodRegistration("staticFuncMyLegacyColorMyColor_MyColor", "(II)I",
	            Delegate.CreateDelegate(System.Func`5[System.IntPtr,System.IntPtr,System.Int32,System.Int32,System.Int32], targetType, "StaticFuncMyLegacyColorMyColor_MyColor")),
	        new JniNativeMethodRegistration("staticFuncThisMethodTakesLotsOfParameters", "(ZBCSIJFDLjava/lang/Object;Ljava/lang/String;Ljava/util/ArrayList;Ljava/lang/String;Ljava/lang/Object;DFJ)Z",
	            Delegate.CreateDelegate((Type)Type.GetType("_JniMarshal_PPZBCSIJFDLLLLLDFJ_Z", true), targetType, "StaticFuncThisMethodTakesLotsOfParameters"))
	    });
	}

The offending IL fragment:

	…
	IL_0161:  ldstr      "staticFuncThisMethodTakesLotsOfParameters"
	IL_0166:  ldstr      "(ZBCSIJFDLjava/lang/Object;Ljava/lang/String;Ljava/util/ArrayList;Ljava/lang/String;Ljava/lang/Object;DFJ)Z"
	IL_016b:  ldstr      "_JniMarshal_PPZBCSIJFDLLLLLDFJ_Z"
	IL_0170:  ldc.i4.1
	IL_0171:  call       [mscorlib]System.Type [mscorlib]System.Type::GetType(string, bool)
	IL_0176:  ldloc.0
	IL_0177:  ldstr      "StaticFuncThisMethodTakesLotsOfParameters"
	IL_017c:  call       [mscorlib]System.Delegate [mscorlib]System.Delegate::CreateDelegate([mscorlib]System.Type, [mscorlib]System.Type, string)
	IL_0181:  newobj     instance void [Java.Interop]Java.Interop.JniNativeMethodRegistration::.ctor(string, string, [mscorlib]System.Delegate)

I don't understand where the `ldloc.0` is coming from, or why it's
there.

Punting a complete fix for now.

To repro the crash:

	$ rm -Rf bin/TestDebug
	$ msbuild
	$ msbuild /restore tools/jnimarshalmethod-gen/Xamarin.Android.Tools.JniMarshalMethodGenerator.csproj /p:_AllTheArguments=True /p:_DumpRegisterNativeMembers=True
	$ make run-test-jnimarshal
@jpobst jpobst added this to the 10.4 (16.7 / 8.7) milestone May 11, 2020
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants