[Java.Interop] Optional "Standalone" Build Config

[jonpryor]

trafficstars

Context: 16e1ecd47caf02fefb2130f28c5894323f46c63a Context: 312fbf439ed874bb5f4f25ee6d2c9a2b3c2f5a8b

It occurs to me that it would be easier for "external" developers to use Java.Interop.dll if it didn't require building and distributing any native libraries. Furthermore, as of commit 312fbf43 (C#9 function pointer backend), it's plausible to make that work.

Let's do so.

Add new "pseudo" Standalone-Debug and Standalone-Release build configurations to Java.Interop.csproj which set the FEATURE_JNIENVIRONMENT_JI_FUNCTION_POINTERS compiler define instead of the FEATURE_JNIENVIRONMENT_JI_PINVOKES define. This enables JniEnvironment to use C#9 function pointers instead of P/Invokes to invoke the JNIEnv function pointers. They're "pseudo" configurations because they don't actually exist within Java.Interop.sln, and thus can only be built via command line, or via <ProjectReference AdditionalProperties="…"/> wonkery:

dotnet build src/Java.Interop/Java.Interop.csproj -c 'Standalone-Debug'

Update Java.Interop.dll to compile when FEATURE_JNIENVIRONMENT_JI_FUNCTION_POINTERS is set.

!!ABI BREAK!! [Obsolete] the method JniRuntime.GetAvailableInvocationPointers(). In retrospect this never should have been exposed at this level of the stack, and its existence was responsible for "really really bizarre" .NET Android app crashes (due to static constructor orderings) when sometimes JniRuntime.Current wasn't set "early enough":

D Mono    : AOT: FOUND method Java.Interop.JniRuntime:GetAvailableInvocationPointers () [0x78e4da7960 - 0x78e4da7a7c 0x78e4de6840]
D Mono    : AOT: FOUND method Java.Interop.JniRuntime:GetCreatedJavaVMs (intptr[],int,int&) [0x78e4ddd2b0 - 0x78e4ddd300 0x78e4de6bcd]
D Mono    : AOT: NOT FOUND: Java.Interop.NativeMethods:java_interop_jvm_list (intptr[],int,int&).
F monodroid-assembly: Internal p/invoke symbol 'java-interop @ java_interop_jvm_list' (hash: 0x58c48fc8b89cb484) not found in compile-time map.

Nobody should be using this method, largely given that only Xamarin.Android and .NET Android apps currently use Java.Interop.dll, and neither use JniRuntime.GetAvailableInvocationPointers(). Furthermore, it can't work on Android, as Android doesn't provide a public JNI_GetCreatedJavaVMs() symbol.

Update build-tools/jnienv-gen so that a JniNativeMethods class is defined which contains "human usable" ways to invoke JNIEnv function pointers. (Nobody wants to copy the expression (*((JNIEnv**)env))->ExceptionClear(env) more than once, ever. JniNativeMethods.ExceptionClear(env) is much nicer to write.)

Update samples/Hello-Core so that it uses the Standalone- pseudo configuration instead of the "regular" peer config.

Verification: After building samples/Hello-Core, the contained Java.Interop.dll doesn't contain any pinvokeimpl methods:

% dotnet build samples/Hello-Core
% ikdasm samples/Hello-Core/bin/Debug/Java.Interop.dll | grep pinvoke
# no matches

TODO: I also attempted to reduce the number of P/Invokes in Java.Runtime.Environment.dll, with the hope that when not using MonoVM it could be used without a native java-interop library. This used System.Runtime.InteropServices.NativeLibrary to load JniRuntime.CreationOptions.JvmLibraryPath and invoke the JNI_CreateJavaVM() and JNI_GetCreatedJavaVMs() exports.

Unfortunately, this new backend crashes inexplicably when using dotnet test. The backend can now be selected by setting the JI_LOADER_TYPE environment variable to one of:

• native-library: the NativeLibrary backened, or
• java-interop: the previous java-interop native lib backend.

This allows testing to work and CI to succeed:

% dotnet test bin/TestDebug-net7.0/Java.Interop-Tests.dll
# all good

while allowing us to separately explore why it crashes:

% JI_LOADER_TYPE=native-library dotnet test bin/TestDebug-net7.0/Java.Interop-Tests.dll
…
# jonp: LoadJvmLibrary(…/libjli.dylib)=9056174496
# jonp: JNI_CreateJavaVM=4561133901; JNI_GetCreatedJavaVMs=4561133970
# jonp: executing JNI_CreateJavaVM=10fdd614d
Error occurred during initialization of VM
Could not reserve enough space in CodeHeap 'non-nmethods' (2496K)
The active test run was aborted. Reason: Test host process crashed

Test Run Aborted with error System.Exception: One or more errors occurred.
 ---> System.Exception: Unable to read beyond the end of the stream.
   at System.IO.BinaryReader.Read7BitEncodedInt()
   at System.IO.BinaryReader.ReadString()
   at Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.LengthPrefixCommunicationChannel.NotifyDataAvailable()
   at Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.TcpClientExtensions.MessageLoopAsync(TcpClient client, ICommunicationChannel channel, Action`1 errorHandler, CancellationToken cancellationToken)
   --- End of inner exception stack trace ---.

[pavelsavara]

I think that once you are given pointer to JNIEnv you could copy the pointers to functions and save one indirection on each call. I played with it here https://github.com/pavelsavara/simpleJNI/blob/main/JNI/JNIEnv.cs#L14

[jonpryor]

@pavelsavara while you theoretically can, that doesn't necessarily mean you should. JNI docs state that you should treat the JNIEnv* as a C++ object pointer + virtual member functions, and when using C++ you can (should!) use it as a C++ object pointer. From JNI Interface Functions and Pointers:

The JNI interface is organized like a C++ virtual function table or a COM interface. The advantage to using an interface table, rather than hard-wired function entries, is that the JNI name space becomes separate from the native code. A VM can easily provide multiple versions of JNI function tables:

• one performs thorough illegal argument checks, and is suitable for debugging;
• the other performs the minimal amount of checking required by the JNI specification, and is therefore more efficient.

By my reading, there is no requirement that all threads get the same "JNIEnv runtime type"; thread A could get the "debugging" version while thread B gets the "efficient" version while thread C is using a JNIEnv* that comes from a different JavaVM* entirely. (Allowed by the spec! However, creating more than one JVM in a process is not supported by any JVM that I know of. At the same time, it might be possible to create one JavaVM per separate JVM in the same process! A horrifying idea that had not occurred to me until just now…)

In short, I do not believe that this is a safe optimization to perform unless you "know" more things about the JVM you're using than the JNI spec requires. I think this optimization is best avoided.

[pavelsavara]

I think we need an instance of C# JNIEnv per thread anyway ...

[jonpryor]

On the whole "why does JI_LOADER_TYPE=native-library dotnet test … crash?" question, @akoeplinger suggested that as a test I try doing the JVM load within a child process, via a "Process indirection".

Behold!

diff --git a/samples/Hello-Core/Program.cs b/samples/Hello-Core/Program.cs
index 157aa80b..ee13f609 100644
--- a/samples/Hello-Core/Program.cs
+++ b/samples/Hello-Core/Program.cs
@@ -1,8 +1,13 @@
-using Java.Interop;
+using System.Diagnostics;
+
+using Java.Interop;
 
 using Mono.Options;
 
 bool showHelp = false;
+bool inner = false;
+
+Console.WriteLine ($"# jonp: args: {string.Join (" ", args)}");
 
 var jreOptions = new JreRuntimeOptions {
 };
@@ -24,6 +29,12 @@ var options = new OptionSet {
 	  "Show this message and exit.",
 	  v => showHelp = v != null },
 };
+options.Add (
+		prototype:      "inner",
+		description:    "directly use JNI, not via wrapper proc",
+		action:         v => inner = v != null,
+		hidden:         true
+);
 options.Parse (args);
 
 if (showHelp) {
@@ -36,6 +47,22 @@ if (string.IsNullOrEmpty (jreOptions.JvmLibraryPath) || !File.Exists (jreOptions
 	return;
 }
 
+if (!inner) {
+	var psi = new ProcessStartInfo () {
+		FileName = "dotnet",
+		CreateNoWindow = true,
+		UseShellExecute = false,
+	};
+	psi.ArgumentList.Add (System.Reflection.Assembly.GetExecutingAssembly ().Location);
+	psi.ArgumentList.Add ("--inner");
+	foreach (var a in args) {
+		psi.ArgumentList.Add (a);
+	}
+	var proc = Process.Start (psi);
+	proc?.WaitForExit ();
+	return;
+}
+
 var jre = jreOptions.CreateJreVM ();
 
 // We now have a JVM!

It doesn't fail:

% export JI_LOADER_TYPE=native-library
% dotnet run -- -jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib                              
# jonp: args: -jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib
# jonp: args: --inner -jvm /Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib
# jonp: LoadJvmLibrary(/Library/Java/JavaVirtualMachines/microsoft-11.jdk/Contents/Home/lib/jli/libjli.dylib)=8599356000
# jonp: JNI_CreateJavaVM=4308816205; JNI_GetCreatedJavaVMs=4308816274
# jonp: executing JNI_CreateJavaVM=100d3514d
# jonp: r=0 javavm=105979c80 jnienv=7fbd2f009348
Object_class=0x7fbd2e804d28/L
Object_val=0x7fbd2e804d38/L
Object_val.toString()=java.lang.Object@5cbc508c
Object_val.toString()=java.lang.Object@3419866c

I don't know why JI_LOADER_TYPE=native-library dotnet test … crashes, but it doesn't look like being within a child process is it.

[jpobst]

Given that the new Configurations will not be visible in Visual Studio, maybe there is no benefit to messing with Configurations in the first place, given that they complicate the build system. The Configuration name unfortunately drives a lot of things, which you had to add code to work around.

Maybe we would be better off with just a property? (Feel free to pick a better name!)

dotnet build src/Java.Interop/Java.Interop.csproj -p:InvokeType=Standalone

[jonpryor]

@jpobst: updated to not abuse $(Configuration), and instead use a $(Standalone)=True property.

I'm not sure if that name is any better, either, but at least the Configuration values are sane again.

[jonpryor]

@jpobst: Good suggestion. https://github.com/xamarin/xamarin-android/pull/7509

[jonpryor]

https://github.com/xamarin/xamarin-android/pull/7509 is "good enough". Merging.