jnimarshalmethod-gen should be used & usable with Java.Base-Tests
#1159
Labels
enhancement
Proposed change to current functionality
java-interop
Runtime bridge between .NET and Java
Comments
jpobst
added
enhancement
jonpryor
added a commit
that referenced
this issue
Nov 11, 2023
Context: #1153 Context: 4787e01 Context: 77800dd PR #1153 is exploring the use of [.NET Native AOT][0] to produce a native library which is used *within* a `java`-originated process: % java -cp … com/microsoft/hello_from_jni/App # launches NativeAOT-generated native lib, executes C# code… As NativeAOT has no support for `System.Reflection.Emit`, the only way for Java code to invoke managed code -- in a Desktop Java.Base environment! [^0] see 4787e01 -- would be to pre-generate the required marshal methods via `jnimarshalmethod-gen`. This in turn requires updating `jcw-gen` to support the pre-existing `Java.Interop.JavaCallableAttribute`, so that C# code could reasonably declare methods visible to Java, along with the introduction of, and support for, a new `Java.Interop.JavaCallableConstructorAttribute` type. This allows straightforward usage: [JniTypeSignature ("example/ManagedType")] // for a nice Java name! class ManagedType : Java.Lang.Object { int value; [JavaCallableConstructor(SuperConstructorExpression="")] public ManagedType (int value) { this.value = value; } [JavaCallable ("getString")] public Java.Lang.String GetString () { return new Java.Lang.String ($"Hello from C#, via Java.Interop! Value={value}"); } } Run this through `jcw-gen` and `jnimarshalmethod-gen`, run the app, and nothing worked (?!), because not all pieces were in agreement. Java `native` method registration is One Of Those Things™ that involves lots of moving pieces: * `generator` emits bindings for Java types, which includes Java method names, signatures, and (on .NET Android) the "connector method" to use: [Register ("toString", "()Ljava/lang/String;", "GetToStringHandler")] // .NET Android [JniMethodSignature ("toString", "()Ljava/lang/String;")] // Java.Base public override unsafe string? ToString () {…} * `jcw-gen` uses `generator` output, *prefixing* Java method names with `n_` for `native` method declarations, along with a "normal" method wrapper [^1] public String toString() {return n_toString();} private native String n_toString(); * `jnimarshalmethod-gen` emits marshal methods for Java.Base, and needs to register the `native` methods declared by `jcw-gen`. `jnimarshalmethod-gen` and `jcw-gen` need to be consistent with each other. Turns Out, `jcw-gen` and `jnimarshalmethod-gen` were *not* consistent. The only "real" `jnimarshalmethod-gen` usage (77800dd) is with the `Java.Interop.Export-Tests` unit test assembly, which *did not use* `jcw-gen`; it contained only hand-written Java code. Consequently, *none* of the Java `native` methods declared within it had an `n_` prefix, and since this worked with `jnimarshalmethod-gen`, this means that `jnimarshalmethod-gen` registration logic likewise didn't use `n_` prefixed method names. The result is that in the NativeAOT app, it would attempt to register the `native` Java method `ManagedType.getString()`, while what `jcw-gen` declared was `ManagedType.n_getString()`! Java promptly threw an exception, and the app crashed. Update `Java.Interop.Export-Tests` so that all the methods used with `MarshalMemberBuilder` are declared with `n_` prefixes, and add a `Java.Lang.Object` subclass example to the unit tests: Update `tests/Java.Interop.Tools.JavaCallableWrappers-Tests` to add a test for `.CodeGenerationTarget==JavaInterop1`. Add `$(NoWarn)` to `Java.Interop.Tools.JavaCallableWrappers-Tests.csproj` in order to "work around" warnings-as-errors: …/src/Java.Interop.NamingCustomAttributes/Java.Interop/ExportFieldAttribute.cs(19,63): error CA1019: Remove the property setter from Name or reduce its accessibility because it corresponds to positional argument name …/src/Java.Interop.NamingCustomAttributes/Android.Runtime/RegisterAttribute.cs(53,4): error CA1019: Remove the property setter from Name or reduce its accessibility because it corresponds to positional argument name …/src/Java.Interop.NamingCustomAttributes/Java.Interop/ExportFieldAttribute.cs(12,16): error CA1813: Avoid unsealed attributes … These are "weird"; the warnings/errors appear to come in because `Java.Interop.Tools.JavaCallableWrappers-Tests.csproj` now has: <Compile Include="..\..\src\Java.Interop\Java.Interop\JniTypeSignatureAttribute.cs" /> which appears to pull in `src/Java.Interop/.editorconfig`, which makes CA1019 and CA1813 errors. (I do not understand what is happening.) Update `jnimarshalmethod-gen` so that the Java `native` methods it registers have an `n_` prefix. Refactor `ExpressionAssemblyBuilder.CreateRegistrationMethod()` to `ExpressionAssemblyBuilder.AddRegistrationMethod()`, so that the `EmitConsoleWriteLine()` invocation can provide the *full* type name of the `__RegisterNativeMembers()` method, which helps when there is more than one such method running around… Update `ExpressionAssemblyBuilder` so that the delegate types it creates for marshal method registration all have `[UnmanagedFunctionPointer(CallingConvention.Winapi)]`. (This isn't needed *here*, but is needed in the context of NativeAOT, as NativeAOT will only emit "marshal stubs" for delegate types which have `[UnmanagedFunctionPointer]`.) Unfortunately, adding `[UnmanagedFunctionPointer]` broke things: error JM4006: jnimarshalmethod-gen: Unable to process assembly '…/Hello-NativeAOTFromJNI.dll' Failed to resolve System.Runtime.InteropServices.CallingConvention Mono.Cecil.ResolutionException: Failed to resolve System.Runtime.InteropServices.CallingConvention at Mono.Cecil.Mixin.CheckedResolve(TypeReference self) at Mono.Cecil.SignatureWriter.WriteCustomAttributeEnumValue(TypeReference enum_type, Object value) … The problem is that `CallingConvention` was resolved from `System.Private.CoreLib`, and when we removed that assembly reference, the `CallingConvention` couldn't be resolved at all. We could "fix" this by explicitly adding a reference to `System.Runtime.InteropServices.dll`, but how many more such corner cases exist? The current approach is not viable. Remove the code from 77800dd which attempts to remove `System.Private.CoreLib`. So long as `ExpressionAssemblyBuilder` output is *only* used in "completed" apps (not distributed in NuGet packages or some "intermediate" form), referencing `System.Private.CoreLib` is "fine". Update `jnimarshalmethod-gen` assembly location probing: in #1153, it was attempting to resolve the *full assembly name* of `Java.Base`, as `Java.Base, Version=7.0.0.0, Culture=neutral, PublicKeyToken=null`, causing it to attempt to load the file `Java.Base, Version=7.0.0.0, Culture=neutral, PublicKeyToken=null.dll`, which doesn't exist. Use `AssemblyName` to parse the string and extract out the assembly name, so that `Java.Base.dll` is probed for and found. Update `JreTypeManager` to *also* register the marshal methods generated by `Runtime.MarshalMemberBuilder.GetExportedMemberRegistrations()`. With all that, the updated `Java.Interop.Export-Tests` test now work both before and after `jnimarshalmethod-gen` is run: % dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll && dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll && dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll … TODO: * `generator --codegen-target=JavaInterop1` should emit JNI method signature information for constructors! This would likely remove the need for `[JavaCallableConstructor(SuperConstructorExpression="")`. * #1159 [0]: https://learn.microsoft.com/dotnet/core/deploying/native-aot [^0]: In a .NET Android environment, marshal methods are part of `generator` output, so things would be more straightforward there, though all the `_JniMarshal_*` types that are declared would also need to have `[UnmanagedFunctionPointer(CallingConvention.Winapi)]`… [^1]: Why not just declare `toString()` as `native`? Why have the separate `n_`-prefixed version? To make the `#if MONODROID_TIMING` block more consistent within `JavaCallableWrapperGenerator.GenerateMethod()`. It's likely "too late" to *easily* change this now.
jonpryor
added a commit
that referenced
this issue
Nov 13, 2023
Context: #1153 Context: 4787e01 Context: 77800dd PR #1153 is exploring the use of [.NET Native AOT][0] to produce a native library which is used *within* a `java`-originated process: % java -cp … com/microsoft/hello_from_jni/App # launches NativeAOT-generated native lib, executes C# code… As NativeAOT has no support for `System.Reflection.Emit`, the only way for Java code to invoke managed code -- in a Desktop Java.Base environment! [^0] see 4787e01 -- would be to pre-generate the required marshal methods via `jnimarshalmethod-gen`. This in turn requires updating `jcw-gen` to support the pre-existing `Java.Interop.JavaCallableAttribute`, so that C# code could reasonably declare methods visible to Java, along with the introduction of, and support for, a new `Java.Interop.JavaCallableConstructorAttribute` type. This allows straightforward usage: [JniTypeSignature ("example/ManagedType")] // for a nice Java name! class ManagedType : Java.Lang.Object { int value; [JavaCallableConstructor(SuperConstructorExpression="")] public ManagedType (int value) { this.value = value; } [JavaCallable ("getString")] public Java.Lang.String GetString () { return new Java.Lang.String ($"Hello from C#, via Java.Interop! Value={value}"); } } Run this through `jcw-gen` and `jnimarshalmethod-gen`, run the app, and nothing worked (?!), because not all pieces were in agreement. Java `native` method registration is One Of Those Things™ that involves lots of moving pieces: * `generator` emits bindings for Java types, which includes Java method names, signatures, and (on .NET Android) the "connector method" to use: [Register ("toString", "()Ljava/lang/String;", "GetToStringHandler")] // .NET Android [JniMethodSignature ("toString", "()Ljava/lang/String;")] // Java.Base public override unsafe string? ToString () {…} * `jcw-gen` uses `generator` output, *prefixing* Java method names with `n_` for `native` method declarations, along with a method wrapper [^1] public String toString() {return n_toString();} private native String n_toString(); * `jnimarshalmethod-gen` emits marshal methods for Java.Base, and needs to register the `native` methods declared by `jcw-gen`. `jnimarshalmethod-gen` and `jcw-gen` need to be consistent with each other. * `MarshalMemberbuilder.CreateMarshalToManagedMethodRegistration()` creates a `JniNativeMethodRegistration` instance which contains the name of the Java `native` method to register, and was using a name inconsistent with `jcw-gen`. Turns Out, `jcw-gen`, `jnimarshalmethod-gen`, and `MarshalMemberBuilder` were *not* consistent. The only "real" `jnimarshalmethod-gen` usage (77800dd) is with the `Java.Interop.Export-Tests` unit test assembly, which *did not use* `jcw-gen`; it contained only hand-written Java code. Consequently, *none* of the Java `native` methods declared within it had an `n_` prefix, and since this worked with `jnimarshalmethod-gen`, this means that `jnimarshalmethod-gen` registration logic likewise didn't use `n_` prefixed method names. The result is that in the NativeAOT app, it would attempt to register the `native` Java method `ManagedType.getString()`, while what `jcw-gen` declared was `ManagedType.n_getString()`! Java promptly threw an exception, and the app crashed. Update `Java.Interop.Export-Tests` so that all the methods used with `MarshalMemberBuilder` are declared with `n_` prefixes, and add a `Java.Lang.Object` subclass example to the unit tests: Update `tests/Java.Interop.Tools.JavaCallableWrappers-Tests` to add a test for `.CodeGenerationTarget==JavaInterop1`. Add `$(NoWarn)` to `Java.Interop.Tools.JavaCallableWrappers-Tests.csproj` in order to "work around" warnings-as-errors: …/src/Java.Interop.NamingCustomAttributes/Java.Interop/ExportFieldAttribute.cs(19,63): error CA1019: Remove the property setter from Name or reduce its accessibility because it corresponds to positional argument name …/src/Java.Interop.NamingCustomAttributes/Android.Runtime/RegisterAttribute.cs(53,4): error CA1019: Remove the property setter from Name or reduce its accessibility because it corresponds to positional argument name …/src/Java.Interop.NamingCustomAttributes/Java.Interop/ExportFieldAttribute.cs(12,16): error CA1813: Avoid unsealed attributes … These are "weird"; the warnings/errors appear to come in because `Java.Interop.Tools.JavaCallableWrappers-Tests.csproj` now includes: <Compile Include="..\..\src\Java.Interop\Java.Interop\JniTypeSignatureAttribute.cs" /> which appears to pull in `src/Java.Interop/.editorconfig`, which makes CA1019 and CA1813 errors. (I do not understand what is happening.) Update `jnimarshalmethod-gen` so that the Java `native` methods it registers have an `n_` prefix. Refactor `ExpressionAssemblyBuilder.CreateRegistrationMethod()` to `ExpressionAssemblyBuilder.AddRegistrationMethod()`, so that the `EmitConsoleWriteLine()` invocation can provide the *full* type name of the `__RegisterNativeMembers()` method, which helps when there is more than one such method running around… Update `ExpressionAssemblyBuilder` so that the delegate types it creates for marshal method registration all have `[UnmanagedFunctionPointer(CallingConvention.Winapi)]`. (This isn't needed *here*, but is needed in the context of NativeAOT, as NativeAOT will only emit "marshal stubs" for delegate types which have `[UnmanagedFunctionPointer]`.) Unfortunately, adding `[UnmanagedFunctionPointer]` broke things: error JM4006: jnimarshalmethod-gen: Unable to process assembly '…/Hello-NativeAOTFromJNI.dll' Failed to resolve System.Runtime.InteropServices.CallingConvention Mono.Cecil.ResolutionException: Failed to resolve System.Runtime.InteropServices.CallingConvention at Mono.Cecil.Mixin.CheckedResolve(TypeReference self) at Mono.Cecil.SignatureWriter.WriteCustomAttributeEnumValue(TypeReference enum_type, Object value) … The problem is that `CallingConvention` was resolved from `System.Private.CoreLib`, and when we removed that assembly reference, the `CallingConvention` couldn't be resolved at all. We could "fix" this by explicitly adding a reference to `System.Runtime.InteropServices.dll`, but how many more such corner cases exist? The current approach is not viable. Remove the code from 77800dd which attempts to remove `System.Private.CoreLib`. So long as `ExpressionAssemblyBuilder` output is *only* used in "completed" apps (not distributed in NuGet packages or some "intermediate" form), referencing `System.Private.CoreLib` is "fine". Update `jnimarshalmethod-gen` assembly location probing: in #1153, it was attempting to resolve the *full assembly name* of `Java.Base`, as `Java.Base, Version=7.0.0.0, Culture=neutral, PublicKeyToken=null`, causing it to attempt to load the file `Java.Base, Version=7.0.0.0, Culture=neutral, PublicKeyToken=null.dll`, which doesn't exist. Use `AssemblyName` to parse the string and extract out the assembly name, so that `Java.Base.dll` is probed for and found. Update `JreTypeManager` to *also* register the marshal methods generated by `Runtime.MarshalMemberBuilder.GetExportedMemberRegistrations()`. With all that, the updated `Java.Interop.Export-Tests` test now work both before and after `jnimarshalmethod-gen` is run: % dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll && dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll && dotnet test --logger "console;verbosity=detailed" bin/TestDebug-net7.0/Java.Interop.Export-Tests.dll … TODO: * `generator --codegen-target=JavaInterop1` should emit JNI method signature information for constructors! This would likely remove the need for `[JavaCallableConstructor(SuperConstructorExpression="")`. * #1159 [0]: https://learn.microsoft.com/dotnet/core/deploying/native-aot [^0]: In a .NET Android environment, marshal methods are part of `generator` output, so things would be more straightforward there, though all the `_JniMarshal_*` types that are declared would also need to have `[UnmanagedFunctionPointer(CallingConvention.Winapi)]`… [^1]: Why not just declare `toString()` as `native`? Why have the separate `n_`-prefixed version? To make the `#if MONODROID_TIMING` block more consistent within `JavaCallableWrapperGenerator.GenerateMethod()`. It's likely "too late" to *easily* change this now.
jonpryor
added a commit
that referenced
this issue
Nov 16, 2023
Fixes: #1159 Context: c93fea0 `jnimarshalmethod-gen` is intended to generate marshal methods for any type that that: * Has `[JavaCallable]` (tested in `Java.Interop.Export-Tests.dll` and c93fea0), or * Overrides a `virtual` method which has `[JniMethodSignature]`, or * Implements an interface method which has `[JniMethodSignature]`. Thus, the intention was that it should generate marshal methods for `Java.Base-Tests`: % dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll -v bin/TestDebug-net7.0/Java.Base-Tests.dll Unable to read assembly 'bin/TestDebug-net7.0/Java.Base-Tests.dll' with symbols. Retrying to load it without them. Preparing marshal method assembly 'Java.Base-Tests-JniMarshalMethods' Processing Java.BaseTests.JavaInvoker type Processing Java.BaseTests.MyRunnable type Processing Java.BaseTests.MyIntConsumer type Marshal method assembly 'Java.Base-Tests-JniMarshalMethods' created Notably missing? No messages stating: Adding marshal method for … Also missing? `ikdasm bin/TestDebug-net7.0/Java.Base-Tests.dll` showed that there were no `__RegisterNativeMembers()` methods emitted. The `jnimarshalmethod-gen` invocation was a no-op! The problems were twofold: 1. It was only looking for methods with `Android.Runtime.RegisterAttribute`. This was useful for Xamarin.Android (when we were trying to make it work), but doesn't work with Java.Base. We need to *also* look for `Java.Interop.JniMethodSignature`. Relatedly, the attempt to use `registerAttribute.Constructor.Parameters` to determine parameter names didn't work; the parameter name was always `""`. 2. A'la c93fea0, we need to ensure that the Java `native` methods we register are consistent with `jcw-gen` output. Fix these two problems, which allows `jnimarshalmethod-gen` to now emit marshal methods for types within `Java.Base-Tests.dll`. Additionally, rework the `jnimarshalmethod-gen -f` logic to *remove* the existing `__<$>_jni_marshal_methods` nested type when present.
jonpryor
added a commit
that referenced
this issue
Nov 22, 2023
Fixes: #1159 Context: c93fea0 `jnimarshalmethod-gen` is intended to generate marshal methods for any type that that: * Has `[JavaCallable]` (tested in `Java.Interop.Export-Tests.dll` and c93fea0), or * Overrides a `virtual` method which has `[JniMethodSignature]`, or * Implements an interface method which has `[JniMethodSignature]`. Thus, the intention was that it should generate marshal methods for `Java.Base-Tests`: % dotnet bin/Debug-net7.0/jnimarshalmethod-gen.dll -v bin/TestDebug-net7.0/Java.Base-Tests.dll Unable to read assembly 'bin/TestDebug-net7.0/Java.Base-Tests.dll' with symbols. Retrying to load it without them. Preparing marshal method assembly 'Java.Base-Tests-JniMarshalMethods' Processing Java.BaseTests.JavaInvoker type Processing Java.BaseTests.MyRunnable type Processing Java.BaseTests.MyIntConsumer type Marshal method assembly 'Java.Base-Tests-JniMarshalMethods' created Notably missing? No messages stating: Adding marshal method for … Also missing? `ikdasm bin/TestDebug-net7.0/Java.Base-Tests.dll` showed that there were no `__RegisterNativeMembers()` methods emitted. The `jnimarshalmethod-gen` invocation was a no-op! The problems were twofold: 1. It was only looking for methods with `Android.Runtime.RegisterAttribute`. This was useful for Xamarin.Android (when we were trying to make it work), but doesn't work with Java.Base. We need to *also* look for `Java.Interop.JniMethodSignature`. Relatedly, the attempt to use `registerAttribute.Constructor.Parameters` to determine parameter names didn't work; the parameter name was always `""`. 2. A'la c93fea0, we need to ensure that the Java `native` methods we register are consistent with `jcw-gen` output. Fix these two problems, which allows `jnimarshalmethod-gen` to now emit marshal methods for types within `Java.Base-Tests.dll`. Additionally, rework the `jnimarshalmethod-gen -f` logic to *remove* the existing `__<$>_jni_marshal_methods` nested type when present.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
In the same way that
jnimarshalmethod-genis run againstJava.Interop.Export-Testsin 77800dd,jnimarshalmethod-genshould also be run againstJava.Base-Tests, andJava.Base-Testsshould continue passing after the fixup, a'la:One problem with this now is that
jnimarshalmethod-gendoesn't do anything toJava.Base-Tests.dll, even though it has at least oneJava.Lang.Objectsubclass, and thus requires marshal methods.The text was updated successfully, but these errors were encountered: