You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
[Java.Interop] More sanely dispose of JNI Local References.
JNI Local References are thread-local values, which are cleaned up
upon return to the Java VM (assuming there IS a return to the VM).
As such, they are not usable across threads; see commit b2f0d6a.
A question thus arises [0]: how do we sanely dispose of JNI local
references without relying on the GC, as the GC may run on a dedicated
finalizer thread, which can't do anything with them (as it's a
different thread!).
In the ideal world, we could use some Thread-specific cleanup
mechanism, e.g. a pthread_key_create(3) destructor (have the
destructor call a JniEnvironment method which disposes of registered
JniLocalReference values).
Unfortunately, .NET doesn't appear to provide any such thread-local
cleanup mechanism.
We could rely on the Mono embedding API, but I'd rather not require
Mono at this time.
On the (mistaken) assumption that this was only an issue for newly
created threads, I had pondered a JavaVM.CreateThread() method...but
that complicates integration with existing Threading code.
This can instead be addressed by making four semantic changes to
JniEnvironment:
1. Remove JniEnvironment.CheckCurrent(), and replace with a new
JniEnvironment(IntPtr) constructor.
2. Alter JniEnvironment.Dispose() to clear all JniLocalReferences
associated with the current JniEnvironment.
3. Add a JniEnvironment.SetPendingException(Exception) method.
Dispose() will marshal and raise the exception after most other
work.
4. Treat JniEnvironment.Current as a "stack" of JniEnvironment
values, managed by the JniEnvironment constructor and Dispose()
calls. This allows creation of ~arbitrary "scopes", at the end
of which all current JNI local references will be cleared.
The above changes allow us to alter the pattern for methods registered
with Java from:
TReturnType (*)(IntPtr jnienv, IntPtr context /* ... args ....*/ )
{
JniEnvironment.CheckCurrent (jnienv);
try {
...
}
catch (Exception e) {
JniEnvironment.Errors.Throw (e);
}
}
and replace it with:
TReturnType (*)(IntPtr jnienv, IntPtr context /* ... args ....*/ )
{
var __envp = new JniEnvironment (jnienv);
try {
...
}
catch (Exception e) {
__envp.SetPendingException (e);
}
finally {
__envp.Dispose ();
}
}
The new pattern ensures that if we have a C#1 > Java > C#2 callstack,
any JNI Local References allocated within the C#2 frame will be
cleaned up before returning to the Java frame.
This allows conforming code to do a "best effort" at preventing leaks
of JNI local references, ensuring cleanup without relying on the GC.
Note: As part of removing JniEnvironment.CheckCurrent() and adding the
JniEnvironment(IntPtr) constructor, the underlying
JniEnvironmentSafeHandle value for the current thread must be
static, such that previous instances on the "environment stack" always
use the latest handle value:
var top = new JniEnvironment (HandleA);
// top.SafeHandle.DangerousGetHandle() == HandleA;
using (var nested = new JniEnvironment (HandleB)) {
// nested.SafeHandle.DangerousGetHandle() == HandleB;
}
// top.SafeHandle.DangerousGetHandle() == HandleB;
This semantic is required for the same reason that
JniEnvironment.CheckCurrent() was required: it's possible for the
JNIEnv* value for the current thread to change, and we must ALWAYS use
the latest version.
Note: Java.Interop.Export never needed to call
JniEnvironment.CheckCurrent(), because the methods it generates are
wrapped by JniType.RegisterNativeMethods() / JniMarshalMethod.Wrap().
It is thus safe to remove those checks.
Note: With the new "auto-cleanup" logic, JniReferenceSafeHandle.Null
can no longer be a JniLocalReference, as it will be disposed when the
thread-local JniEnvironment is disposed (which results in bizarre
ObjectDisposedException errors in the unit tests). JniInvocationHandle
should have been used instead.
[0]: A related question: should we care? Commit b2f0d6a will cause
JniLocalReference.Dispose() to be a no-op from outside the thread that
created them, so the only overhead is the finalizer queue.
0 commit comments