Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Hello-NativeAOTFromJNI] Add NativeAOT sample #1153

Merged
merged 52 commits into from
Feb 22, 2024

Conversation

jonpryor
Copy link
Member

Instead of a managed app creating a Java VM and calling into Java, do the opposite: have a Java app call into Native Library built using NativeAOT, using [UnmanagedCallersOnly] to provide the entry points.

Instead of a managed app creating a Java VM and calling into Java,
do the opposite: have a Java app call into Native Library built using
NativeAOT, using `[UnmanagedCallersOnly]` to provide the entry points.
BROKEN BROKEN BROKEN BROKEN

What do we want?  To create a `JreRuntime` instance!

…which is shockingly difficult.

But first, *why* do we want to create a `JreRuntime` instance?
(It's not like we've needed it so far!)

Force the issue by changing `NativeAOTInit.sayHello()` to return a
`String` instance, thus requiring that for `NativeAOTInit.sayHello()`
to return a non-`null` value, it needs a `JreRuntime` instance!

This is where everything falls apart.  `JreRuntime` and
`JreRuntimeOptions` were not designed with this scenario in mind!
Update `JreRuntime` & co so that it doesn't require `JvmLibraryPath`
when `InvocationPointer` isn't null, and don't require that
`JreRuntimeOptions.ClassPath` contain the path to `java-interop.jar`
when `InvocationPointer` isn't null.

This allows us to create the `JreRuntimeOptions` instance.  Yay!

The next problem is that setting
`%(ProjectReference.AdditionalProperties)` to include `Standalone=true`
doesn't appear to work, in that the `Java.Interop.dll` that is
included still contains P/Invokes instead of the `JniNativeMethods`
class.  Update `Java.Interop.csproj` so that `$(Standalone)`=true
is now the default.  (This *shouldn't* break anybody, but… it now means
the P/Invoke backend is getting NO testing.  ¯\_(ツ)_/¯ )

The next part is where it blows up GOOD: `options.CreateJreVM()`
throws, which aborts everything:

	% (cd bin/Release/osx-x64/publish ; java -cp hello-from-java.jar:java-interop.jar com/microsoft/hello_from_jni/App)
	Hello from Java!
	# jonp: JNI_OnLoad: vm=10a9c0b00
	# jonp: JNI_OnLoad: created options…
	# jonp: builder.InvocationPointer=10a9c0b00
	JNI_OnLoad: error: System.TypeInitializationException: A type initializer threw an exception. To determine which type, inspect the InnerException's StackTrace property.
	 ---> System.NotSupportedException: 'Java.Interop.ManagedPeer+ConstructMarshalMethod' is missing delegate marshalling data. This can happen for code that is not compatible with AOT. Inspect and fix AOT related warnings that were generated when the app was published. For more information see https://aka.ms/nativeaot-compatibility
	   at Internal.Runtime.CompilerHelpers.RuntimeInteropData.GetDelegateMarshallingStub(RuntimeTypeHandle, Boolean) + 0x78
	   at System.Runtime.InteropServices.PInvokeMarshal.AllocateThunk(Delegate del) + 0x6b
	   at System.Runtime.CompilerServices.ConditionalWeakTable`2.GetValueLocked(TKey, ConditionalWeakTable`2.CreateValueCallback) + 0x27
	   at System.Runtime.CompilerServices.ConditionalWeakTable`2.GetValue(TKey, ConditionalWeakTable`2.CreateValueCallback) + 0x41
	   at System.Runtime.InteropServices.PInvokeMarshal.GetFunctionPointerForDelegate(Delegate) + 0xd5
	   at libHello-NativeAOTFromJNI!<BaseAddress>+0x8adeb
	   at Java.Interop.JniEnvironment.Types._RegisterNatives(JniObjectReference, JniNativeMethodRegistration[], Int32) + 0x90
	   at Java.Interop.JniEnvironment.Types.RegisterNatives(JniObjectReference, JniNativeMethodRegistration[], Int32) + 0x65
	   at Java.Interop.JniType.RegisterNativeMethods(JniNativeMethodRegistration[]) + 0x30
	   at Java.Interop.ManagedPeer..cctor() + 0x175
	   at System.Runtime.CompilerServices.ClassConstructorRunner.EnsureClassConstructorRun(StaticClassConstructionContext*) + 0xb4
	   --- End of inner exception stack trace ---
	   at System.Runtime.CompilerServices.ClassConstructorRunner.EnsureClassConstructorRun(StaticClassConstructionContext*) + 0x147
	   at System.Runtime.CompilerServices.ClassConstructorRunner.CheckStaticClassConstructionReturnGCStaticBase(StaticClassConstructionContext*, Object) + 0x9
	   at Java.Interop.JniRuntime..ctor(JniRuntime.CreationOptions) + 0x5a2
	   at Java.Interop.JreRuntime..ctor(JreRuntimeOptions) + 0x22
	   at Hello_NativeAOTFromJNI.JNIEnvInit.JNI_OnLoad(IntPtr, IntPtr) + 0x9e
	Exception in thread "main" java.lang.UnsatisfiedLinkError: unsupported JNI version 0x00000000 required by /Volumes/Xamarin-Work/src/xamarin/Java.Interop/samples/Hello-NativeAOTFromJNI/bin/Release/osx-x64/publish/libHello-NativeAOTFromJNI.dylib
		at java.base/jdk.internal.loader.NativeLibraries.load(Native Method)
		at java.base/jdk.internal.loader.NativeLibraries$NativeLibraryImpl.open(NativeLibraries.java:388)
		at java.base/jdk.internal.loader.NativeLibraries.loadLibrary(NativeLibraries.java:232)
		at java.base/jdk.internal.loader.NativeLibraries.loadLibrary(NativeLibraries.java:174)
		at java.base/jdk.internal.loader.NativeLibraries.findFromPaths(NativeLibraries.java:315)
		at java.base/jdk.internal.loader.NativeLibraries.loadLibrary(NativeLibraries.java:287)
		at java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:2427)
		at java.base/java.lang.Runtime.loadLibrary0(Runtime.java:818)
		at java.base/java.lang.System.loadLibrary(System.java:1989)
		at com.microsoft.hello_from_jni.NativeAOTInit.<clinit>(NativeAOTInit.java:5)
		at com.microsoft.hello_from_jni.App.main(App.java:7)

Within the `JniRuntime` constructor, we're hitting:

	#if !XA_JI_EXCLUDE
		ManagedPeer.Init ();
	#endif  // !XA_JI_EXCLUDE

This invokes the `ManagedPeer` static constructor, which attempts to
call `JniType.RegisterNativeMethods()`, which attempts to call
`JNIEnv::RegisterNatives()`, but before it can get that far it needs
to marshal things:

 1. The `JniNativeMethodRegistration` struct, which in turn requires
 2. The `JniNativeMethodRegistration.Marshaler` delegate field.

This apparently isn't supported by NativeAOT, at least not without
additional work that I am not currently privy to.

Given that `JniNativeMethodRegistration` structure marshaling is how
*all* JNI method registration is done in .NET Android, this is a bit
of a blocker for this sample.

TODO? Figure out how to allow NativeAOT to marshal
`JniNativeMethodRegistration` with a minimum of effort?

TODO? *Require* `jnimarshalmethod-gen`, and update it so that it
emits `[UnmanagedCallersOnlyAttribute]` on all generated marshal
methods *and* emits `UnmanagedCallersOnlyAttribute.EntryPoint` to
a `Java_…` symbol name so that we *don't* hit the
`JniNativeMethodRegistration` struct marshaling codepath.
(See also 77800dd).

[0]:https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#RegisterNatives
Commit db84cc3 "didn't work" because the "traditional" form of
method registration through `JNIEnv::RegisterNatives()` used a
`System.Delegate` instance to refer to a C# method to invoke, but
this fails under NativeAOT because the NativeAOT infrastructure needs
to be able to produce "stubs" for the methods based on compile-time
scans, and it doesn't represent our code pattern and/or our pattern
is not supportable with how NativeAOT works.

Oops.

"Ecosystem-wise", I'm not sure how to solve this, other than
handy-wavy ideas such as "rely on `jnimarshalmethod-gen`!".

Short-term, we can at least resolve `ManagedPeer`.

Create a new `JniBlittableNativeMethodRegistration` struct which
contains only blittable types, vs. `JniNativeMethodRegistration`
which contains `string` fields and `Delegate` fields and is not
easily supportable by NativeAOT.

	public partial struct JniBlittableNativeMethodRegistration {
	    public JniBlittableNativeMethodRegistration (IntPtr name, IntPtr signature, IntPtr marshaler);
	    // To support `new JniBlittableNativeMethodRegistration("name"u8, "signature"u8, marshaler)`
	    public JniBlittableNativeMethodRegistration (ReadOnlySpan<byte> name, ReadOnlySpan<byte> signature, IntPtr marshaler);
	}

"Plumb through" use of `JniBlittableNativeMethodRegistration` to
`JniEnvironment.Types.RegisterNatives()` overloads:

	partial class JniEnvironment {
	    partial class Types {
	        public static void RegisterNatives(JniObjectReference type, ReadOnlySpan<JniBlittableNativeMethodRegistration> methods);
	    }
	}

This in turn requires updating `jnienv-gen` so that
`RegisterNatives()` is bound with the `const JNINativeMethod *methods`
parameter bound as an `IntPtr` instead of a
`JniNativeMethodRegistration[]`.

Update the `ManagedPeer` static constructor to use
`[UnmanagedCallersOnly]` methods and native function pointers to
register the marshal methods that `ManagedPeer` requires.
@@ -19,18 +20,27 @@ namespace Java.Interop {

static readonly JniPeerMembers _members = new JniPeerMembers (JniTypeName, typeof (ManagedPeer));

static ManagedPeer ()
static unsafe ManagedPeer ()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, instead of introducing a "blittable method registration struct", we can instead use [UnmanagedFunctionPointer] to coerce NativeAOT to produce the required stubs:

diff --git a/src/Java.Interop/Java.Interop/ManagedPeer.cs b/src/Java.Interop/Java.Interop/ManagedPeer.cs
index eb43a9e8..044b9424 100644
--- a/src/Java.Interop/Java.Interop/ManagedPeer.cs
+++ b/src/Java.Interop/Java.Interop/ManagedPeer.cs
@@ -6,6 +6,7 @@ using System.Linq;
 using System.Linq.Expressions;
 using System.Reflection;
 using System.Reflection.Emit;
+using System.Runtime.InteropServices;
 using System.Runtime.Serialization;
 using System.Text;
 
@@ -50,6 +51,7 @@ namespace Java.Interop {
 		const string ConstructSignature = "(Ljava/lang/Object;Ljava/lang/String;Ljava/lang/String;[Ljava/lang/Object;)V";
 
 		// TODO: Keep in sync with the code generated by ExportedMemberBuilder
+		[UnmanagedFunctionPointer (CallingConvention.Winapi)]
 		delegate void ConstructMarshalMethod (IntPtr jnienv,
 				IntPtr klass,
 				IntPtr n_self,
@@ -177,6 +179,7 @@ namespace Java.Interop {
 
 		const   string  RegisterNativeMembersSignature  = "(Ljava/lang/Class;Ljava/lang/String;Ljava/lang/String;)V";
 
+		[UnmanagedFunctionPointer (CallingConvention.Winapi)]
 		delegate void RegisterMarshalMethod (IntPtr jnienv,
 				IntPtr klass,
 				IntPtr n_nativeClass,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm uncertain how much I want to push unmanaged function pointers everywhere. I like the idea, but there are "unknown" implications, not least of which is JniNativeMethodRegistrationArguments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out that for NativeAOT, JniNativeMethodRegistrationArguments is unusable, because to get there we need to pass through Type.GetType("TypeToRegister, DeclaringAssembly"), which throws System.IO.FileNotFoundException: Could not resolve assembly 'DeclaringAssembly'..

Suggesting that there isn't actually any point in a blittable equivalent to JniNativeMethodRegistrationArguments, because we're gonna need to use Java_… native methods anyway. It's not optional for NativeAOT, it's required.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented a "workaround" by adding a new JniRuntime.JniTypeManager.RegisterNativeMembers() overload which takes a ReadOnlySpan<char> instead of a Type, allowing subclasses to avoid the Type.GetType() invocation.

This has two implications:

  1. Sample is able to "reasonably work" without implementing [UnmanagedCallersOnly(EntryPoint="Java_…")] marshal methods, so score one for simplicity, and
  2. The question around a "blittable" equivalent of JniNativeMethodRegistrationArguments is reintroduced, as now it is plausible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

…and I in turn replaced the "new" (now removed) JniRuntime.JniTypeManager.RegisterNativeMembers() overload with an alternate approach involving JniRuntime.JniTypeManager.GetTypeFromAssemblyQualifiedName(), and "bulking up" the typemap dictionary within NativeAotTypeManager. This in no way verifies or removes the need for a blittable registration struct; it just further punts it down the line.

That said, a blittable struct would presumably be more efficient…

Commit db84cc3 noted that the sample failed to run because
NativeAOT didn't know how to produce the required "stubs" to allow
a `System.Delegate` field to be marshaled.

Commit a32e244 was an attempt to "fix" this by introducing a
`JniBlittableNativeMethodRegistration` and using C#9 function pointers
to register `ManagedPeer` marshal methods instead of delegates.
While this works, this is an extensive change, and leaves various
"corner cases" in how things fit together, such as the
`JniNativeMethodRegistrationArguments` struct used as part of
`jnimarshalmethod-gen`-based method registration.

@lambdageek found an easier solution: place
`[UnmanagedFunctionPointer]` onto the delegate declaration.  This
appears to tell NativeAOT to generate the required stubs, with
significantly fewer changes.  This also could fit nicely into a future
`generator` change to place `[UnmanagedFunctionPointer]` on all the
`_JniMarshal_*` declarations (56955d9).
@jonpryor
Copy link
Member Author

Should further extend sample to:

  • Make use of Java.Base + jcw-gen for a custom Java.Lang.Object subclass
  • Integrate jnimarshalmethod-gen

I suspect that jnimarshalmethod-gen (+changes) will be required in a NativeAOT environment, as "normal" Java.Base use in tests/Java.Base-Tests makes use of MarshalMemberBuilder, which requires System.Linq.Expressions, which is presumably unsupportable in a NativeAOT environment.

What `Hello-NativeAOTFromJNI` previously did was quite minimal:

 1. Use `[UnmanagedCallersOnly]` to provide a `JNI_OnLoad()` method which
 2. Initialized the Java.Interop runtime, allowing
 3. An `[UnmanagedCallersOnly]`-provided `Java_…` method which is called from Java.

All quite low level, all miles away from .NET Android.

Expand the sample to:

 1. Contain a `Java.Lang.Object` subclass, which contains a
    `[JavaCallable]` method.
 2. Call `jcw-gen` to generate Java Callable Wrappers for (1),
    containing the `[JavaCallable]` method.
 3. Call `jnimarshalmethod-gen` to generate marshal methods for (1),
    as NativeAOT doesn't support System.Reflection.Emit.
 4. Instantiate (1) *from Java*, and invoke the `[JavaCallable]` method.

*Now* we're (kinda) getting something that looks like .NET Android.

But first, we need to make that *work*:

Update `Java.Interop.Tools.JavaCallableWrappers` so that it will emit
`native` method declarations for `[JavaCallable]` methods, not just
method overrides and `[Export]` methods.

Update `Java.Interop.Tools.Expressions` so that the `_JniMarshal_*`
delegate types have `[UnmanagedFunctionPointer(CallingConvention.Winapi)]`,
as this is what allows NativeAOT to emit appropriate "stubs"; see
also da9f188.

Update `Java.Interop.Tools.Expressions.ExpressionAssemblyBuilder` to
no longer attempt to "remove and fixup" `System.Private.CoreLib`.
So long as `ExpressionAssemblyBuilder` output is *only* used in
"completed" apps (not distributed in NuGet packages in some
"intermediate" form), referencing `System.Private.CoreLib` is "fine".
Additionally, trying to remove `System.Private.CoreLib` broke things
when adding `[UnmanagedFunctionPointer]`, as `CallingConvention`
could not be resolved, resulting in `jnimarshalmethod-gen` erroring
out with:

	error JM4006: jnimarshalmethod-gen: Unable to process assembly '/Volumes/Xamarin-Work/src/xamarin/Java.Interop/samples/Hello-NativeAOTFromJNI/bin/Debug/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)
	   …

(This is because `CallingConvention` is in
`System.Runtime.InteropServices.dll`, which isn't referenced.)

We could "fix" this by explicitly adding a reference to
`System.Runtime.InteropServices.dll`, but this is just one of an
unknown number of corner cases.  Give up for now.

Update `jnimarshalmethod-gen` assembly location probing: it
was given the *full assembly name* of `Java.Base`:

	# jonp: resolving assembly: Java.Base, Version=7.0.0.0, Culture=neutral, PublicKeyToken=null

…and failing to find `Java.Base.dll`, because it was looking for
`Java.Base, Version=7.0.0.0, Culture=neutral, PublicKeyToken=null.dll`.
Oops.

Use `AssemblyName` to parse the string and extract out the assembly
name., so that `Java.Base.dll` is probed for and found.

With all that…it still fails:

	% (cd bin/Release/osx-x64/publish ; java -cp hello-from-java.jar:java-interop.jar com/microsoft/hello_from_jni/App)
	Hello from Java!
	C# init()
	Hello from .NET NativeAOT!
	String returned to Java: Hello from .NET NativeAOT!
	Exception in thread "main" com.xamarin.java_interop.internal.JavaProxyThrowable: System.IO.FileNotFoundException: Could not resolve assembly 'Hello-NativeAOTFromJNI'.
	   at System.Reflection.TypeNameParser.ResolveAssembly(String) + 0x97
	   at System.Reflection.TypeNameParser.GetType(String, ReadOnlySpan`1, String) + 0x32
	   at System.Reflection.TypeNameParser.NamespaceTypeName.ResolveType(TypeNameParser&, String) + 0x17
	   at System.Reflection.TypeNameParser.GetType(String, Func`2, Func`4, Boolean, Boolean, Boolean, String) + 0x99
	   at Java.Interop.ManagedPeer.RegisterNativeMembers(IntPtr jnienv, IntPtr klass, IntPtr n_nativeClass, IntPtr n_assemblyQualifiedName, IntPtr n_methods) + 0x103
		at com.xamarin.java_interop.ManagedPeer.registerNativeMembers(Native Method)
		at example.ManagedType.<clinit>(ManagedType.java:15)
		at com.microsoft.hello_from_jni.App.main(App.java:13)

`App.main()` has `new example.ManagedType()`, which hits the
`ManagedType` static constructor of:

	public /* partial */ class ManagedType
	  extends java.lang.Object
	  implements
	    com.xamarin.java_interop.GCUserPeerable
	{
	/** @hide */
	  public static final String __md_methods;
	  static {
	    __md_methods =
	      "n_GetString:()Ljava/lang/String;:__export__\n" +
	      "";
	    com.xamarin.java_interop.ManagedPeer.registerNativeMembers (ManagedType.class, "Example.ManagedType, Hello-NativeAOTFromJNI", __md_methods);
	  }
	}

The `ManagedPeer.registerNativeMembers()` call is what is needed
to register the native `ManagedPeer.getString()` method, so that
it can be called.  This is good.  (Though `__md_methods` containing
*anything* is not desired, but that's a different problem.)

`ManagedPeer.RegisterNativeMembers()` is given the assembly-qualified
name `Example.ManagedType, Hello-NativeAOTFromJNI`, and tries to:

	Type.GetType ("Example.ManagedType, Hello-NativeAOTFromJNI", throwOnError: true);

…which then proceeds to throw, because in NativeAOT
*there are no assemblies*, and thus `Type.GetType()` *cannot work*.

Oops.

Thus, the only way to make something remotely like .NET Android
infrastructure work is to *require* the use of `Java_…` native method
names and `[UnmanagedCallersOnly]` on marshal methods.
(In .NET Android parlance, the experimental
`$(AndroidEnableMarshalMethods)`=True is required.)
Commit 69c90ba expanded into a "Full(er)" sample, with just one issue:
it didn't fully work:

	Exception in thread "main" com.xamarin.java_interop.internal.JavaProxyThrowable: System.IO.FileNotFoundException: Could not resolve assembly 'Hello-NativeAOTFromJNI'.
	   at System.Reflection.TypeNameParser.ResolveAssembly(String) + 0x97
	   at System.Reflection.TypeNameParser.GetType(String, ReadOnlySpan`1, String) + 0x32
	   at System.Reflection.TypeNameParser.NamespaceTypeName.ResolveType(TypeNameParser&, String) + 0x17
	   at System.Reflection.TypeNameParser.GetType(String, Func`2, Func`4, Boolean, Boolean, Boolean, String) + 0x99
	   at Java.Interop.ManagedPeer.RegisterNativeMembers(IntPtr jnienv, IntPtr klass, IntPtr n_nativeClass, IntPtr n_assemblyQualifiedName, IntPtr n_methods) + 0x103
	        at com.xamarin.java_interop.ManagedPeer.registerNativeMembers(Native Method)
	        at example.ManagedType.<clinit>(ManagedType.java:15)
	        at com.microsoft.hello_from_jni.App.main(App.java:13)

The problem is that `ManagedPeer.RegisterNativeMembers()` calls
`Type.GetType("Example.ManagedType, Hello-NativeAOTFromJNI")`, which
throws `FileNotFoundException`.

Let's attempt to fix that:

Update `MangedPeer.RegisterNativeMembers()` to call the (new!) method:

	partial class JniRuntime {
	  partial class JniTypeManager {
	    public virtual void RegisterNativeMembers (
	        JniType nativeClass,
	        ReadOnlySpan<char> assmblyQualifiedTypeName,
	        ReadOnlySpan<char> methods);
	  }
	}

which allows a subclass to *avoid* the `Type.GetType()` call.

Add a `NativeAotTypeManager` class which subclasses
`JniRuntime.JniTypeManager`, overriding `RegisterNativeMembers()`
so as to avoid the `Type.GetType()` call.  (It also "fakes" its own
"typemap" implementation…)

Add `Hello-NativeAOTFromJNI.xml`, a Linker Descriptor, to preserve
the `JavaException` constructors, which are needed when things
break horrifically.

TODO: figure out the appropriate `DynamicDependencyAttribute`
incantations to replace `Hello-NativeAOTFromJNI.xml`.

Update the `_AddMarshalMethods` build task to *also* update
`$(IntermediateOutputPath)$(TargetFileName)`, as the copy in
`$(IntermediateOutputPath)` is used by the `IlcCompile` target.
*Not* updating the copy in `$(IntermediateOutputPath)` means that
we don't get *any* marshal methods, and things break.

Rename `ExpressionAssemblyBuilder.CreateRegistrationMethod()` to
Rename `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…

Various changes to `JniRuntime.JniTypeManager.cs`, to increase
logging verbosity, and to make the optimization effort in
`TryLoadJniMarshalMethods()` actually work; `Type.GetRuntimeMethod()`
*will not find* non-public methods, and `__RegisterNativeMembers()`
is rarely/never public.  Thus, it was basically dead code, and we'd
always fallback to the "look at all methods and see which ones have
`[JniAddNativeMethodRegistration]`" codepath, which is by definition
slower.  Use `Type.GetMethod()` instead.

Update `jnimarshalmethod-gen` & co so that they're consistent with
the output of `jcw-gen`.  Without these changes, the JCW would declare
`n_GetString()`, while `jnimarshalmethod-gen` would try to register
`getString`, and Java would thrown an exception because there is no
`getString` member to register.  Doh!

Finally, and the one thing that keeps this from being "perfect",
add an "activation constructor"
`Example.ManagedType(ref JniObjectReference, JniObjectReferenceOptions)`.

This constructor is currently needed because "JavaInterop1"-style
Java Callable Wrappers don't contain constructors (?!), so no
`Example.ManagedType` instance is created *until* the
`ManagedType.n_GetString()` marshal method is executed and attempts
to invoke the `ManagedType.GetString()` method.

We'll need to update `jcw-gen` & related to address this.

Current output:

	% (cd bin/Release/osx-x64/publish ; java -cp hello-from-java.jar:java-interop.jar com/microsoft/hello_from_jni/App)
	Hello from Java!
	C# init()
	Hello from .NET NativeAOT!
	String returned to Java: Hello from .NET NativeAOT!
	C# RegisterNativeMembers(JniType(Name='example/ManagedType' PeerReference=0x7fe545812ed8/G), "Example.ManagedType, Hello-NativeAOTFromJNI", "n_GetString:()Ljava/lang/String;:__export__
	")
	# jonp: called `Example.ManagedType/__<$>_jni_marshal_methods.__RegisterNativeMembers()` w/ 1 methods to register.
	mt.getString()=Hello from C#, via Java.Interop!
A funny thing happened on the way through CI:
`Java.Interop.Export-Tests` *with* `jnimarshalmethod-gen` failed!

	  Failed AddExportMethods [169 ms]
	  Error Message:
	   Java.Interop.JavaException : Could not initialize class com.xamarin.interop.export.ExportType
	  Stack Trace:
	     at Java.Interop.JniEnvironment.StaticMethods.GetStaticMethodID(JniObjectReference type, String name, String signature) in /Users/runner/work/1/s/src/Java.Interop/obj/Release/net7.0/JniEnvironment.g.cs:line 21407
	   at Java.Interop.JniType.GetStaticMethod(String name, String signature) in /Users/runner/work/1/s/src/Java.Interop/Java.Interop/JniType.cs:line 315
	   at Java.InteropTests.MarshalMemberBuilderTest.AddExportMethods()
	   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
	   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
	  --- End of managed Java.Interop.JavaException stack trace ---
	java.lang.NoClassDefFoundError: Could not initialize class com.xamarin.interop.export.ExportType

The cause?  e1822f0 updated `jnimarshalmethod-gen`:

	- registrations.Add (new ExpressionMethodRegistration (name, signature, mmDef));
	+ // Assume that `JavaCallableAttribute.Name` is "public" JCW name, and JCW's declare `n_`-prefixed `native` methods…
	+ registrations.Add (new ExpressionMethodRegistration ("n_" + method.Name, signature, mmDef));

That is, instead of attempting to register e.g.
`ExportType.action()V` (via `[JavaCallable("action")]`), it was
instead attempting to register `ExportType.n_action()V`, because of
the introduced `"n_" + method.Name` change.

The "problem" is that `jnimarshalmethod-gen` was written in the
context of Java.Interop, *not* .NET Android, and the existing
`Java.Interop.Export-Tests` unit tests assume that there is no
`n_`-prefix on native method declarations.

There are two plausible solutions: update the unit tests to conform
to existing .NET Android convention, and use an `n_` prefix on
`native` method declarations.

Or update `jcw-gen` so that when using
`jcw-gen --codegen-target=JavaInterop1`, the `JavaCallableAttribute.Name`
value is used for the `native` method declaration.

Because @jonpryor is a glutton for punishment and "cleanliness",
let's try the latter.  `JavaCallableWrapperGenerator` already has
a `.CodeGenerationTarget` property, How Hard Could It Be™?

Turns out, harder than expected: `JavaCallableWrapperGenerator`
was doing *lots* of work in its constructor, including the all
important bit of populating `Signature` values, and the constructor
runs *before* the `.CodeGenerationTarget` property is set.
This is a somewhat straightforward fix: turn most of the
`JavaCallableWrapperGenerator` constructor into a `Initialize()`
method, and call `Initialize()` from the public methods.
This creates a semantic change: some exceptions which were thrown
by the constructor are now thrown from the public methods.
We deem this as acceptable because all known uses of
`JavaCallableWrapperGenerator` ~immediately call `.Generate()` after
creating the instance, so the exception will still be thrown from a
site near where it previously would have been thrown.

The more annoying aspect is `Signature` initialization: we need to
pass in `.CodeGenerationTarget`, which is Yet Another Constructor
Parameter, and we already have A Lot™.  Attempt to bring sanity to
this mess by introducing a new `SignatureOptions` type to hold the
`Signature` parameters.

Update `jnimarshalmethod-gen` to undo the change which broke
`Java.Interop.Export-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" the 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
	…

This is "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.

(insert massively confused face here.  Like, wat?)
…rom-jni

All hail the new interface invokers in 1adb796!
9027591 tried to fix
Java.Interop.Export-Tests, and in the process broke
Java.Base-Tests!

	A total of 1 test files matched the specified pattern.
	  Failed InterfaceInvokerMethod [100 ms]
	  Error Message:
	   Java.Interop.JavaException : 'void example.MyIntConsumer.accept(int)'
	  Stack Trace:
	     at Java.Interop.JniEnvironment.InstanceMethods.CallVoidMethod(JniObjectReference instance, JniMethodInfo method, JniArgumentValue* args) in /Users/runner/work/1/s/src/Java.Interop/obj/Release/net7.0/JniEnvironment.g.cs:line 20370
	   at Java.Interop.JniPeerMembers.JniInstanceMethods.InvokeAbstractVoidMethod(String encodedMember, IJavaPeerable self, JniArgumentValue* parameters) in /Users/runner/work/1/s/src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods_Invoke.cs:line 47
	   at Java.Lang.IRunnableInvoker.Run() in /Users/runner/work/1/s/src/Java.Base/obj/Release-net7.0/mcw/Java.Lang.IRunnable.cs:line 34
	   at Java.BaseTests.JavaToManagedTests.InterfaceInvokerMethod() in /Users/runner/work/1/s/tests/Java.Base-Tests/Java.Base/JavaToManagedTests.cs:line 28
	   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
	   at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)
	  --- End of managed Java.Interop.JavaException stack trace ---
	java.lang.UnsatisfiedLinkError: 'void example.MyIntConsumer.accept(int)'
		at example.MyIntConsumer.accept(Native Method)
		at com.microsoft.java_base_tests.Invoker$1.run(Invoker.java:15)

The problem? That pesky `n_`!  Or rather, the lack thereof this time.

The issue is that `JreTypeManager`/etc. and `jcw-gen` need to be consistent.
9027591 altered the consistency,
breaking Java.Base tests.

Commit this to verify that things work on CI, not just locally.

Assuming it works on CI, next step will be to largely revert
9027591, and then fix
`Java.Interop.Export-Tests` to use an `n_` prefix on all `native`
method declarations.  That bit is hand-written; it can change.
Keeping the tooling consistent is more important.
Invoking the Java-side constructor should invoke the
C#-side constructor too!  And now it does!

    Hello from Java!
    C# init()
    Hello from .NET NativeAOT!
    String returned to Java: Hello from .NET NativeAOT!
    C# RegisterNativeMembers(JniType(Name='example/ManagedType' PeerReference=0x7f9392713098/G), "Example.ManagedType, Hello-NativeAOTFromJNI", "getString:()Ljava/lang/String;:__export__
    ")
    # jonp: called `Example.ManagedType/__<$>_jni_marshal_methods.__RegisterNativeMembers()` w/ 1 methods to register.
    # jonp: ActivatePeer( False, 0x700001bc6a70/I, Void .ctor(), System.Object[])
    mt.getString()=Hello from C#, via Java.Interop!
Continuing on the "activation constructor" train, we want to allow
Java to pass parameters to a constructor.

In .NET Android parlance:

	class Example : Java.Lang.Object {
	    [Export]
	    public Example (int value) {}
	}

Commit 9027591 updated `JavaCallableAttribute` to allow it to be
placed on constructors, but if we try that with
`samples/Hello-NativeAOTFromJNI`, the Java Callable Wrapper
doesn't build:

	// JCW
	/* partial */ class Example extends java.lang.Object {
	    public Example (int p0) {
	        super (p0);
	        if (getClass () == Example.class) ManagedPeer.construct(…);
	    }
	}

To make this work, we need `ExportAttribute.SuperArgumentsString`.

We *could* add this to `JavaCallableAttribute`, but that means
we'd have a property which can only be used from constructors.
(Yes, `ExportAttribute` has this "wonkiness", but that doens't mean
we should continue it!)

Add a new `JavaCallableConstructorAttribute`, which has a new
`SuperConstructorExpression` property, along with `jcw-gen` support.
This allows things to compile:

	// C#
	class Example : Java.Lang.Object {
	    [JavaCallableConstructor (SuperConstructorExpression="")]
	    public Example (int value) {}
	}

	// JCW
	/* partial */ class Example extends java.lang.Object {
	    public Example (int p0) {
	        super ();
	        if (getClass () == Example.class) ManagedPeer.construct(…);
	    }
	}

…but it's not quite right yet, because the wrong constructor is
invoked!

	ManagedPeer.construct (
	    /* self */      this,
	    /* aqn */       "Namespace.Example, Assembly",
	    /* ctor sig */  "",
	    /* arguments */ new java.lang.Object[] { p0 }
	);

Oops!

Update the `Signature`/`SignatureOptions` creation within
`JavaCallableWrapperGenerator.cs` so that `ManagedParameters`
is set for *all* constructor codepaths.

	ManagedPeer.construct (
	    /* self */      this,
	    /* aqn */       "Namespace.Example, Assembly",
	    /* ctor sig */  "System.Int32, System.Runtime",
	    /* arguments */ new java.lang.Object[] { p0 }
	);

This appears to fix a long-standing bug/"thinko" in JCW generation!
Remember 902fe28?

> Assuming it works on CI, next step will be to largely revert
> 9027591, and then fix
> `Java.Interop.Export-Tests` to use an `n_` prefix on all `native`
> method declarations.  That bit is hand-written; it can change.
> Keeping the tooling consistent is more important.

Let's Do It!  Revert most of the `JavaCallableWrapperGenerator`
bits of 9027591.

Update the `JavaInterop1` JCW output to conform, using `n_`-prefixed
native method declarations.

*Retain the constructor signature fix* in 93a901a..

Revert the `JreTypeManager` change in 902fe28.

Update `jnimarshalmethod-gen` to register `n_`-prefixed names.

Update `Java.Interop.Export-Tests` so that native method declarations
now have `n_` prefixes, so that all is in sync.
Remember the one-off comment in 93a901a?

> (Possible "quick hack": replace `Type.GetType()` use with calls to something
> on `JniRuntime.JniTypeManager`, allowing a subclass to provide its own
> mapping?  This feels "duplicative" of dotnet/runtime, though.)

The more I think about it, the saner this feels.

Add `JniRuntime.JniTypeManager.GetTypeFromAssemblyQualifiedName()`,
and use that instead of `Type.GetType()`.  This allows
`NativeAotTypeManager` to provide its equivalent functionality.

This in turn allows `ManagedType` to provide a constructor which
takes parameters, which previously broke because that codepath hit
`Type.GetType()`!
@jonpryor
Copy link
Member Author

jonpryor commented Nov 9, 2023

I previously wrote:

Should further extend sample to:

  • Make use of Java.Base + jcw-gen for a custom Java.Lang.Object subclass
  • Integrate jnimarshalmethod-gen

That was done, several commits ago, in: jonpryor@69c90ba

Fewer changes to jnimarshalmethod-gen were required than expected, actually; there was no need to implement [UnmanagedCallersOnly(EntryPoint="Java_…")] support, for example. We just needed to add [UnmanagedFunctionPointer] to the delegate types used by method registration.

Several incremental improvements since then.

jonpryor added a commit that referenced this pull request Nov 9, 2023
Context: #1153

[JNI][0] supports *two* modes of operation:

 1. Native code creates the JVM, e.g. via [`JNI_CreateJavaVM()`][1]

 2. The JVM already exists, and when Java code calls
    [`System.loadLibrary()`][3], the JVM calls the
    [`JNI_OnLoad()`][2] function on the specified library.

Java.Interop samples and unit tests rely on the first approach,
e.g. `TestJVM` subclasses `JreRuntime`, which is responsible for
calling `JNI_CreateJavaVM()` so that Java code can be used.

PR #1153 is exploring the use of [.NET Native AOT][4] to produce a
native library which is used with Java-originated initialization.

In order to make Java-originated initialization *work*, we need
to be able to initialize `JniRuntime` and `JreRuntime` around
existing JVM-provided pointers:

  * The `JavaVM*` provided to `JNI_OnLoad()`, which can be used to
    set `JniRuntime.CreationOptions.InvocationPointer`:

        [UnmanagedCallersOnly(EntryPoint="JNI_OnLoad")]
        int JNI_OnLoad(IntPtr vm, IntPtr reserved)
        {
            var options = new JreRuntimeOptions {
                InvocationPointer = vm,
            };
            var runtime = options.CreateJreVM ();
            return runtime.JniVersion;
            return JNI_VERSION_1_6;
        }

  * The [`JNIEnv*` value provided to Java `native` methods][5] when
    they are invoked, which can be used to set
    `JniRuntime.CreationOptions.EnvironmentPointer`:

        [UnmanagedCallersOnly(EntryPoint="Java_example_Whatever_init")]
        void Whatever_init(IntPtr jnienv, IntPtr Whatever_class)
        {
            var options = new JreRuntimeOptions {
                EnvironmentPointer = jnienv,
            };
            var runtime = options.CreateJreVM ();
        }

Update `JniRuntime` and `JreRuntime` to support these Java-originated
initialization strategies.  In particular, don't require that
`JreRuntimeOptions.JvmLibraryPath` be set, avoiding:

	System.InvalidOperationException: Member `JreRuntimeOptions.JvmLibraryPath` must be set.
	   at Java.Interop.JreRuntime.CreateJreVM(JreRuntimeOptions builder)
	   at Java.Interop.JreRuntime..ctor(JreRuntimeOptions builder)
	   at Java.Interop.JreRuntimeOptions.CreateJreVM()

[0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/jniTOC.html
[1]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/invocation.html#creating_the_vm
[2]: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/Runtime.html#loadLibrary(java.lang.String)
[3]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/invocation.html#JNJI_OnLoad
[4]: https://learn.microsoft.com/dotnet/core/deploying/native-aot/
[5]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#native_method_arguments
jonpryor added a commit that referenced this pull request Nov 9, 2023
Context: #1153

[JNI][0] supports *two* modes of operation:

 1. Native code creates the JVM, e.g. via [`JNI_CreateJavaVM()`][1]

 2. The JVM already exists, and when Java code calls
    [`System.loadLibrary()`][3], the JVM calls the
    [`JNI_OnLoad()`][2] function on the specified library.

Java.Interop samples and unit tests rely on the first approach,
e.g. `TestJVM` subclasses `JreRuntime`, which is responsible for
calling `JNI_CreateJavaVM()` so that Java code can be used.

PR #1153 is exploring the use of [.NET Native AOT][4] to produce a
native library which is used with Java-originated initialization.

In order to make Java-originated initialization *work*, we need
to be able to initialize `JniRuntime` and `JreRuntime` around
existing JVM-provided pointers:

  * The `JavaVM*` provided to `JNI_OnLoad()`, which can be used to
    set `JniRuntime.CreationOptions.InvocationPointer`:

        [UnmanagedCallersOnly(EntryPoint="JNI_OnLoad")]
        int JNI_OnLoad(IntPtr vm, IntPtr reserved)
        {
            var options = new JreRuntimeOptions {
                InvocationPointer = vm,
            };
            var runtime = options.CreateJreVM ();
            return runtime.JniVersion;
            return JNI_VERSION_1_6;
        }

  * The [`JNIEnv*` value provided to Java `native` methods][5] when
    they are invoked, which can be used to set
    `JniRuntime.CreationOptions.EnvironmentPointer`:

        [UnmanagedCallersOnly(EntryPoint="Java_example_Whatever_init")]
        void Whatever_init(IntPtr jnienv, IntPtr Whatever_class)
        {
            var options = new JreRuntimeOptions {
                EnvironmentPointer = jnienv,
            };
            var runtime = options.CreateJreVM ();
        }

Update `JniRuntime` and `JreRuntime` to support these Java-originated
initialization strategies.  In particular, don't require that
`JreRuntimeOptions.JvmLibraryPath` be set, avoiding:

	System.InvalidOperationException: Member `JreRuntimeOptions.JvmLibraryPath` must be set.
	   at Java.Interop.JreRuntime.CreateJreVM(JreRuntimeOptions builder)
	   at Java.Interop.JreRuntime..ctor(JreRuntimeOptions builder)
	   at Java.Interop.JreRuntimeOptions.CreateJreVM()

[0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/jniTOC.html
[1]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/invocation.html#creating_the_vm
[2]: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/Runtime.html#loadLibrary(java.lang.String)
[3]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/invocation.html#JNJI_OnLoad
[4]: https://learn.microsoft.com/dotnet/core/deploying/native-aot/
[5]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/design.html#native_method_arguments
jonpryor added a commit that referenced this pull request 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 pull request 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.
`Hello-NativeAOTFromJNI.csproj` isn't an ASP.NET project, so it isn't
a web project, and shouldn't be treated as such.

We want to invoke the "real" `dotnet publish` command.

Also use `projects: path/to/Hello-NativeAOTFromJNI.csproj`, just in case.
Currently fails with:

    /Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/Hello-NativeAOTFromJNI.targets(101,5): error MSB6003: The specified task executable "sh" could not be run. System.IO.DirectoryNotFoundException: The working directory "bin/Release/osx-x64/publish/" does not exist. [/Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/Hello-NativeAOTFromJNI.csproj]

Maybe if we set workingDirectory to `samples/Hello-NativeAOTFromJNI`
it will work?
Printf debug this; what is `$(PublishDir)` when running
`RunJavaSample`?  What files exist?
This is quite confusing; as per CI logs, during `dotnet publish`:

      Hello-NativeAOTFromJNI -> /Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/bin/Release/osx-x64/publish/

which implies to me that `/Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/bin/Release/osx-x64/publish`
*exists*.

However, when we get to `dotnet build -t:RunJavaSample`:

    ##[error]samples/Hello-NativeAOTFromJNI/Hello-NativeAOTFromJNI.targets(106,5): Error MSB6003: The specified task executable "sh" could not be run.
      System.IO.DirectoryNotFoundException: The working directory
      "/Users/runner/work/1/s/samples/Hello-NativeAOTFromJNI/bin/Release/osx-x64/publish/"
      does not exist.

Which looks the same to me!

Dump out dir contents.
Through the glory of printing the contents of the directory
after `dotnet publish` and as part of `dotnet build -t:RunJavaSample`,
we see that the `$(PublishDir)` `bin/Release/osx-x64/publish`
exists after `dotnet publish`, but is *gone* when `dotnet build`
runs (?!).  In its place is a `publish.zip` file, which doesn't do
us any good!

We thus infer and assume that the DotNetCoreCLI@2 task is creating
this `publish.zip` file and removing the directory, which prevents
the execution step from actually executing.

Replace DotNetCoreCLI@2 with a powershell script which directly invokes
`dotnet publish` & co.  Hopefully this will avoid the problem.
@jonpryor jonpryor marked this pull request as ready for review February 16, 2024 16:42
{
var declType = cinfo.DeclaringType ?? throw new NotSupportedException ("Do not know the type to create!");

#pragma warning disable IL2072
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we might have to use:

[UnconditionalSuppressMessage ("Trimming", "IL2072")]

#pragma won't work at trimming time, because it's only present in C# source code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's odd is that I don't see an IL2072 warning at trimming time, so I'm not convinced that we need [UnconditionalSuppressMessage] here. The IL2072 warning ignored by this #pragma was a build warning within Java.Runtime.Environment.csproj, iirc.

@@ -173,6 +173,21 @@ steps:
arguments: -c $(Build.Configuration) tools/java-source-utils/java-source-utils.csproj -t:RunTests
continueOnError: true

- powershell: >
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these actually need to be powershell or can they be script? I think script would be the same as cmd on Windows and bash on macOS.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea, I'm just going with @akoeplinger 's suggestion on Discord.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are only testing osx-x64 here. Should this PR work on Windows?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conditioning this off a display name seems brittle, we probably want:

condition: eq(variables['Agent.OS'], 'Darwin')

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

script would work since the command is the same i.e. it doesn't matter whether cmd or bash is used but I agree that this should probably only execute on Mac.

@jonathanpeppers
Copy link
Member

/cc @simonrozsival @vitek-karas

This is basically C# + Java "hello world" on NativeAOT.

Copy link
Contributor

@jpobst jpobst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the publish doesn't trigger WarnAsError, but note there are 11 warnings at the bottom of https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=9093736&view=logs&j=4ab9874f-3b82-50ef-2a6b-4340d0043621&t=3f720a70-9e37-509d-b752-3ff7ec647839.

I'm not saying they need to be fixed, but we should be aware of them in case they are telling us that something hasn't been properly annotated for the linker to preserve them.

@@ -173,6 +173,21 @@ steps:
arguments: -c $(Build.Configuration) tools/java-source-utils/java-source-utils.csproj -t:RunTests
continueOnError: true

- powershell: >
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are only testing osx-x64 here. Should this PR work on Windows?

@@ -173,6 +173,21 @@ steps:
arguments: -c $(Build.Configuration) tools/java-source-utils/java-source-utils.csproj -t:RunTests
continueOnError: true

- powershell: >
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conditioning this off a display name seems brittle, we probably want:

condition: eq(variables['Agent.OS'], 'Darwin')

Warnings fixed:

`Type.MakeGenericType()` needs to ensure constructors are preserved:

    src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs(378): Trim analysis warning IL2068: Java.Interop.JniRuntime.JniValueManager.<GetInvokerType>g__MakeGenericType|31_1(Type,Type[]): 'Java.Interop.JniRuntime.JniValueManager.<GetInvokerType>g__MakeGenericType|31_1(Type,Type[])' method return value does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' requirements. The parameter 'type' of method 'Java.Interop.JniRuntime.JniValueManager.<GetInvokerType>g__MakeGenericType|31_1(Type,Type[])' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

`Expression.New()` requires the constructor be kept, so "fake it" by
wrapping `Type.GetType()` w/ `[DynamicallyAccessedMembers]`:

    src/Java.Interop/Java.Interop/JniValueMarshaler.cs(175): Trim analysis warning IL2072: Java.Interop.JniValueMarshaler.CreateSelf(JniValueMarshalerContext,ParameterExpression): 'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Linq.Expressions.Expression.New(Type)'. The return value of method 'System.Object.GetType()' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

`[DynamicallyAccessedMembers]` should be on properties, not getters:

    src/Java.Interop/Java.Interop/JniValueMarshalerAttribute.cs(33): Trim analysis warning IL2078: Java.Interop.JniValueMarshalerAttribute.MarshalerType.get: 'Java.Interop.JniValueMarshalerAttribute.MarshalerType.get' method return value does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor', 'DynamicallyAccessedMemberTypes.Interfaces' requirements. The field 'Java.Interop.JniValueMarshalerAttribute.<MarshalerType>k__BackingField' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Silence use of `Assembly.Location`, when it's optional:

    src/Java.Runtime.Environment/Java.Interop/JreRuntime.cs(106): warning IL3000: Java.Interop.JreRuntime.CreateJreVM(JreRuntimeOptions): 'System.Reflection.Assembly.Location.get' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'.
    src/Java.Runtime.Environment/Java.Interop/JreRuntime.cs(323): warning IL3000: Java.Interop.JreNativeMethods..cctor(): 'System.Reflection.Assembly.Location.get' always returns an empty string for assemblies embedded in a single-file app. If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'.
@jonpryor
Copy link
Member Author

@jpobst asked:

We are only testing osx-x64 here. Should this PR work on Windows?

In theory, yes. In practice, it's never been run on Windows, so I have no idea. It'll certainly require a different $(RuntimeIdentifier) value, probably win-x64

@jonpryor
Copy link
Member Author

@jpobst: the PR now works on Windows.

@jonpryor
Copy link
Member Author

Context: 28849ec1e6b2400576f4f2a0d1fe7aabfbda2269
Context: bc5bcf4f0ef07aab898f2643d2a25f66512d98ed
Context: 25850ba741b9e3d9dbda353c03b80f7ccb4def42
Context: 56955d9ad3952070de3bb1718375b368437f7427
Context: https://github.com/xamarin/java.interop/issues/1157
Context: c6c487b62dab4ffec45e61b09dd43afc89898caf
Context: https://github.com/xamarin/xamarin-android/commit/180dd5205ab270bb74bb853754665db9cb5d65f1

Commit 28849ec1 noted:

> [JNI][0] supports *two* modes of operation:
> 
>  1. Native code creates the JVM, e.g. via [`JNI_CreateJavaVM()`][1]
> 
>  2. The JVM already exists, and when Java code calls
>     [`System.loadLibrary()`][3], the JVM calls the
>     [`JNI_OnLoad()`][2] function on the specified library.
>
> [0]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/jniTOC.html
> [1]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/invocation.html#creating_the_vm
> [2]: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/Runtime.html#loadLibrary(java.lang.String)
> [3]: https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/invocation.html#JNJI_OnLoad

Our existing unit tests, e.g. `tests/Java.Interop-Tests`, exercise
the first mode of operation.

The second mode of operation is how .NET Android executes.

Add `samples/Hello-NativeAOTFromJNI`, a new sample which exercises
the second mode of operation:

 1. `Hello-NativeAOTFromJNI` contains C# methods which use
    [`UnmanagedCallersOnlyAttribute`][4] to export `JNI_OnLoad()` and
    various other native symbols for JNI to execute.

 2. `dotnet publish Hello-NativeAOTFromJNI.csproj` will use
    [NativeAOT][5] to produce a native library which can be loaded
    and used by a desktop JVM.

 3. `Hello-NativeAOTFromJNI` references `Java.Base.dll` (bc5bcf4f),
     and contains a C# `Java.Lang.Object` subclass, which contains a
    `[JavaCallable]` method.

 4. `jcw-gen` is called during the build process to generate
    Java Callable Wrappers for (3), containing the `[JavaCallable]`
    method.

 5. `jnimarshalmethod-gen` (25850ba7) is called during the build
    process to generate marshal methods for (3), as `generator`
    output for JavaInterop1 doesn't contain marshal methods (unlike
    XAJavaInterop1, used by .NET Android), and NativeAOT doesn't
    support System.Reflection.Emit.

 6. The type in (3) is instantiated *from Java*, and Java invokes the
    `[JavaCallable]` method.

 7. `Hello-NativeAOTFromJNI` also contains Java code including a
    `main()` method (to start execution from Java)

Conceptually straightforward!

See `samples/Hello-NativeAOTFromJNI/README.md` for more details.


~~ `$(Standalone)`=true is now the default build config ~~

The `$(Standalone)` build config (c6c487b6) is now enabled by
default.  This means that neither `Java.Interop.dll` nor
`Java.Runtime.Environment.dll` *require* a `java-interop` native
library, which (1) was already the case for xamarin-android as of
xamarin/xamarin-android@180dd520, and (2) simplifies NativeAOT
support immensely.


~~ Avoid Adding Public API ~~

In order to avoid adding new public API to `Java.Interop.dll`, update
`Java.Interop.dll` to have `[InternalsVisibleTo]` for
`Java.Runtime.Environment.dll`.  This allows `JreTypeManager` to use
`JniRuntime.UseMarshalMemberBuilder`, without needing to worry about
`try`/`catch` blocks.

This in turn requires renaming `NativeMethods` within
`Java.Runtime.Environment.dll` to `JreNativeMethods`, as
`Java.Interop.dll` *also* has a `NativeMethods` type.


~~ NativeAOT Integration ~~

Conceptually straightforward, but not without lots of pitfalls.

In order for Java code to call into C# code, there must be a
Java Callable Wrapper, which contains Java `native` methods, and
those Java `native` methods must be *registered with Java* before
they are executed.

Method registration involves lots of delegates, which need to be
stored in the `JniNativeMethodRegistration.Delegate` field, of type
`System.Delegate`.

NativeAOT doesn't like this by default.  In order to convince
NativeAOT to support this, every delegate instance provided to
`JniNativeMethodRegistration.Delegate` must be of a delegate type
which has [`UnmanagedFunctionPointerAttribute`][6].  This coerces
NativeAOT to emit "stubs" for the referenced method, allowing things
to work with fewer changes.

Linux builds require `-Wl,-soname` to be set manually in order for
the resulting `.so` to work properly.


~~ Trimmer Warnings ~~

Fix additional trimmer warnings, a'la xamarin/java.interop#1157.

`Type.MakeGenericType()` needs to ensure constructors are preserved:

	src/Java.Interop/Java.Interop/JniRuntime.JniValueManager.cs(378): Trim analysis warning IL2068:
	  Java.Interop.JniRuntime.JniValueManager.<GetInvokerType>g__MakeGenericType|31_1(Type,Type[]):
	  'Java.Interop.JniRuntime.JniValueManager.<GetInvokerType>g__MakeGenericType|31_1(Type,Type[])' method return value does not satisfy
	  'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.NonPublicConstructors' requirements.
	  The parameter 'type' of method 'Java.Interop.JniRuntime.JniValueManager.<GetInvokerType>g__MakeGenericType|31_1(Type,Type[])'
	  does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.

`Expression.New()` requires the constructor be kept, so "fake it" by
wrapping `Type.GetType()` w/ `[DynamicallyAccessedMembers]`:

	src/Java.Interop/Java.Interop/JniValueMarshaler.cs(175): Trim analysis warning IL2072:
	  Java.Interop.JniValueMarshaler.CreateSelf(JniValueMarshalerContext,ParameterExpression):
	  'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor' in call to 'System.Linq.Expressions.Expression.New(Type)'.
	  The return value of method 'System.Object.GetType()' does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.

`[DynamicallyAccessedMembers]` should be on properties, not getters:

	src/Java.Interop/Java.Interop/JniValueMarshalerAttribute.cs(33): Trim analysis warning IL2078:
	  Java.Interop.JniValueMarshalerAttribute.MarshalerType.get:
	  'Java.Interop.JniValueMarshalerAttribute.MarshalerType.get' method return value does not satisfy
	  'DynamicallyAccessedMemberTypes.PublicParameterlessConstructor', 'DynamicallyAccessedMemberTypes.Interfaces' requirements.
	  The field 'Java.Interop.JniValueMarshalerAttribute.<MarshalerType>k__BackingField' does not have matching annotations.
	  The source value must declare at least the same requirements as those declared on the target location it is assigned to.

Silence use of `Assembly.Location`, when it's optional:

	src/Java.Runtime.Environment/Java.Interop/JreRuntime.cs(106): warning IL3000:
	  Java.Interop.JreRuntime.CreateJreVM(JreRuntimeOptions):
	  'System.Reflection.Assembly.Location.get' always returns an empty string for assemblies embedded in a single-file app.
	  If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'.
	src/Java.Runtime.Environment/Java.Interop/JreRuntime.cs(323): warning IL3000: Java.Interop.JreNativeMethods..cctor():
	  'System.Reflection.Assembly.Location.get' always returns an empty string for assemblies embedded in a single-file app.
	  If the path to the app directory is needed, consider calling 'System.AppContext.BaseDirectory'.


~~ Future Work ~~

Make `JniRuntime.UseMarshalMemberBuilder` public for .NET 9, and
remove the `InternalsVisibleTo` from `Java.Interop.dll` to
`Java.Runtime.Environment.dll`.

`generator` should be updated to apply
`UnmanagedFunctionPointerAttribute` onto all `_JniMarshal_*` delegate
type declarations (56955d9a).


[4]: https://learn.microsoft.com/dotnet/api/system.runtime.interopservices.unmanagedcallersonlyattribute?view=net-8.0
[5]: https://learn.microsoft.com/dotnet/core/deploying/native-aot
[6]: https://learn.microsoft.com/dotnet/api/system.runtime.interopservices.unmanagedfunctionpointerattribute?view=net-8.0

@jonpryor jonpryor merged commit 2197579 into main Feb 22, 2024
4 checks passed
@jonpryor jonpryor deleted the dev/jonp/jonp-hello-from-jni branch February 22, 2024 20:01
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 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.

4 participants