-
Notifications
You must be signed in to change notification settings - Fork 52
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
[Java.Interop] Allow JniRuntime init from JavaVM* and JNIEnv* #1158
Conversation
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
var dir = string.IsNullOrEmpty (loc) ? null : Path.GetDirectoryName (loc); | ||
var jij = string.IsNullOrEmpty (dir) ? null : Path.Combine (dir, "java-interop.jar"); | ||
if (!File.Exists (jij)) { | ||
throw new InvalidOperationException ($"`java-interop.jar` is required. Please add to `JreRuntimeOptions.ClassPath`. Tried to find it in `{jij}`."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth doing FileNotFoundException
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
Path.GetDirectoryName (typeof (JreRuntimeOptions).Assembly.Location) ?? throw new NotSupportedException (), | ||
"java-interop.jar"), | ||
}; | ||
ClassPath = new Collection<string> (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is Collection<string>
? I've never heard of this? It would be an API break to change it, so probably can't be a List<string>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Collection<T>
. It's existed since .NET Framework 2.0, and is the recommended collection type in the .NET Framework Design Guidelines:
✔️ DO use Collection or a subclass of
Collection<T>
for properties or return values representing read/write collections.
This isn't an API break because the type hasn't changed at all, and -- again -- Collection<T>
is in fact the preferred collection type here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL, List
vs Collection
: https://stackoverflow.com/a/271842
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, we've never "shipped" Java.Runtime.Environment.dll
, so I'm fine witih making API breaks here if warranted.
[Test] | ||
public void UseInvocationPointerOnNewThread () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is old, but could we start adding new tests with the pattern:
public async Task UseInvocationPointerOnNewThread ()
{
await Task.Run(() => ...);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonathanpeppers: what's the benefit to using the Task
-based version? Other than ability to use async
/await
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cringe when I see Thread.Join()
that is the only reason, lol.
Throw FileNotFoundException.
Context: #1153
JNI supports two modes of operation:
Native code creates the JVM, e.g. via
JNI_CreateJavaVM()
The JVM already exists, and when Java code calls
System.loadLibrary()
, the JVM calls theJNI_OnLoad()
function on the specified library.Java.Interop samples and unit tests rely on the first approach, e.g.
TestJVM
subclassesJreRuntime
, which is responsible for callingJNI_CreateJavaVM()
so that Java code can be used.PR #1153 is exploring the use of .NET Native AOT 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
andJreRuntime
around existing JVM-provided pointers:The
JavaVM*
provided toJNI_OnLoad()
, which can be used to setJniRuntime.CreationOptions.InvocationPointer
:The
JNIEnv*
value provided to Javanative
methods when they are invoked, which can be used to setJniRuntime.CreationOptions.EnvironmentPointer
:Update
JniRuntime
andJreRuntime
to support these Java-originated initialization strategies. In particular, don't require thatJreRuntimeOptions.JvmLibraryPath
be set, avoiding: