From 1660f064c57a23bd642857c4e022c82fbf42fd32 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 17 Feb 2025 16:25:19 +0100 Subject: [PATCH 1/5] Merge pull request #5 from wuwbobo2021/main Port java-spaghetti-gen to cafebabe; cache method/field IDs; fix jclass memory leaks From 22e5a5b764a22925db05f1094e80cd0099810971 Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Mon, 24 Mar 2025 01:05:11 +0000 Subject: [PATCH 2/5] Merge branch 'Dirbaio:main' into main From 0900f12f87a6cfa69a928eb10888bce94e8ab62f Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Fri, 28 Mar 2025 10:48:34 +0800 Subject: [PATCH 3/5] Add docs; add `Monitor`, `is_same_object`, optimize `cast` --- README.md | 2 +- java-spaghetti/src/array.rs | 45 ++++++++++--- java-spaghetti/src/as_jvalue.rs | 4 +- java-spaghetti/src/env.rs | 29 +++++--- java-spaghetti/src/lib.rs | 18 ++++- java-spaghetti/src/refs/arg.rs | 42 ++++++------ java-spaghetti/src/refs/global.rs | 27 ++++++-- java-spaghetti/src/refs/local.rs | 91 +++++++++++++++---------- java-spaghetti/src/refs/ref_.rs | 105 +++++++++++++++++++++++++++-- java-spaghetti/src/refs/return_.rs | 15 ++++- java-spaghetti/src/string_chars.rs | 13 +++- java-spaghetti/src/vm.rs | 8 +-- 12 files changed, 297 insertions(+), 102 deletions(-) diff --git a/README.md b/README.md index 2762171..eb55bbc 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,7 @@ The full list of differences are: - You can filter which classes are generated in the TOML config. - Generated code uses relative paths (`super::...`) instead of absolute paths (`crate::...`), so it works if you place it in a submodule not at the crate root. - Generated code is a single `.rs` file, there's no support for spltting it in one file per class. You can still run the output through [form](https://github.com/djmcgill/form), if you want. -- Generated code uses cached method IDs and field IDs stored in `OnceLock` to speed up invocations by several times. Used classes are also stored as JNI global references in order to keep the validity of cached IDs. This may not ensure memory safety when class redefinition features (e.g. `java.lang.instrument` which is unavailable on Android) of the JVM is being used. +- Generated code uses cached method IDs and field IDs stored in `OnceLock` to speed up invocations by several times. Used classes are also stored as JNI global references in order to keep the validity of cached IDs. This may not ensure memory safety when class redefinition features (e.g. `java.lang.instrument` which is unavailable on Android) of the JVM are being used. - Generated code doesn't use macros. - No support for generating Cargo features per class. - Modernized rust, updated dependencies. diff --git a/java-spaghetti/src/array.rs b/java-spaghetti/src/array.rs index 28975f0..64a9ec2 100644 --- a/java-spaghetti/src/array.rs +++ b/java-spaghetti/src/array.rs @@ -6,7 +6,10 @@ use jni_sys::*; use crate::{AsArg, Env, JniType, Local, Ref, ReferenceType, ThrowableType}; -/// A Java Array of some POD-like type such as bool, jbyte, jchar, jshort, jint, jlong, jfloat, or jdouble. +/// A Java Array of some POD-like type such as `bool`, `jbyte`, `jchar`, `jshort`, `jint`, `jlong`, `jfloat`, or `jdouble`. +/// +/// Thread safety of avoiding [race conditions](https://www.ibm.com/docs/en/sdk-java-technology/8?topic=jni-synchronization) +/// is not guaranteed. JNI `GetPrimitiveArrayCritical` cannot ensure exclusive access to the array, so it is not used here. /// /// See also [ObjectArray] for arrays of reference types. /// @@ -24,26 +27,33 @@ pub trait PrimitiveArray: Sized + ReferenceType where T: Clone + Default, { - /// Uses env.New{Type}Array to create a new java array containing "size" elements. + /// Uses JNI `New{Type}Array` to create a new Java array containing "size" elements. fn new<'env>(env: Env<'env>, size: usize) -> Local<'env, Self>; - /// Uses env.GetArrayLength to get the length of the java array. + /// Uses JNI `GetArrayLength` to get the length of the Java array. fn len(self: &Ref<'_, Self>) -> usize; - /// Uses env.Get{Type}ArrayRegion to read the contents of the java array from \[start .. start + elements.len()) + /// Uses JNI `Get{Type}ArrayRegion` to read the contents of the Java array within `[start .. start + elements.len()]`. + /// + /// Panics if the index is out of bound. fn get_region(self: &Ref<'_, Self>, start: usize, elements: &mut [T]); - /// Uses env.Set{Type}ArrayRegion to set the contents of the java array from \[start .. start + elements.len()) + /// Uses JNI `Set{Type}ArrayRegion` to set the contents of the Java array within `[start .. start + elements.len()]`. + /// + /// Panics if the index is out of bound. fn set_region(self: &Ref<'_, Self>, start: usize, elements: &[T]); - /// Uses env.New{Type}Array + Set{Type}ArrayRegion to create a new java array containing a copy of "elements". + /// Uses JNI `New{Type}Array` + `Set{Type}ArrayRegion` to create a new Java array containing a copy of "elements". fn new_from<'env>(env: Env<'env>, elements: &[T]) -> Local<'env, Self> { let array = Self::new(env, elements.len()); array.set_region(0, elements); array } - /// Uses env.GetArrayLength + env.Get{Type}ArrayRegion to read the contents of the java array from range into a new Vec. + /// Uses JNI `GetArrayLength` + `Get{Type}ArrayRegion` to read the contents of the Java array within given range + /// into a new `Vec`. + /// + /// Panics if the index is out of bound. fn get_region_as_vec(self: &Ref<'_, Self>, range: impl RangeBounds) -> Vec { let len = self.len(); @@ -69,7 +79,7 @@ where vec } - /// Uses env.GetArrayLength + env.Get{Type}ArrayRegion to read the contents of the entire java array into a new Vec. + /// Uses JNI `GetArrayLength` + `Get{Type}ArrayRegion` to read the contents of the entire Java array into a new `Vec`. fn as_vec(self: &Ref<'_, Self>) -> Vec { self.get_region_as_vec(0..self.len()) } @@ -175,6 +185,9 @@ primitive_array! { DoubleArray, "[D\0", jdouble { NewDoubleArray SetDoubleArra /// A Java Array of reference types (classes, interfaces, other arrays, etc.) /// +/// Thread safety of avoiding [race conditions](https://www.ibm.com/docs/en/sdk-java-technology/8?topic=jni-synchronization) +/// is not guaranteed. +/// /// See also [PrimitiveArray] for arrays of reference types. pub struct ObjectArray(core::convert::Infallible, PhantomData<(T, E)>); @@ -187,6 +200,7 @@ unsafe impl JniType for ObjectArray { } impl ObjectArray { + /// Uses JNI `NewObjectArray` to create a new Java object array. pub fn new<'env>(env: Env<'env>, size: usize) -> Local<'env, Self> { assert!(size <= std::i32::MAX as usize); // jsize == jint == i32 let class = T::static_with_jni_type(|t| unsafe { env.require_class(t) }); @@ -202,6 +216,7 @@ impl ObjectArray { unsafe { Local::from_raw(env, object) } } + /// Iterates through object items of the array. See [ObjectArrayIter]. pub fn iter<'a, 'env>(self: &'a Ref<'env, Self>) -> ObjectArrayIter<'a, 'env, T, E> { ObjectArrayIter { array: self, @@ -210,6 +225,8 @@ impl ObjectArray { } } + /// Uses JNI `NewObjectArray` to create a new Java object array of the exact size, then sets its items + /// with the iterator of JNI (null?) references. pub fn new_from<'env>( env: Env<'env>, elements: impl ExactSizeIterator + Iterator>, @@ -224,12 +241,16 @@ impl ObjectArray { array } + /// Uses JNI `GetArrayLength` to get the length of the Java array. pub fn len(self: &Ref<'_, Self>) -> usize { let env = self.env().as_raw(); unsafe { ((**env).v1_2.GetArrayLength)(env, self.as_raw()) as usize } } - /// XXX: Expose this via std::ops::Index + /// Gets a local reference of the object item at given `index` in the array. + /// Returns `None` if it is null; returns an exception if the index is invalid. + /// + /// XXX: Expose this via `std::ops::Index`. pub fn get<'env>(self: &Ref<'env, Self>, index: usize) -> Result>, Local<'env, E>> { assert!(index <= std::i32::MAX as usize); // jsize == jint == i32 XXX: Should maybe be treated as an exception? let index = index as jsize; @@ -246,7 +267,9 @@ impl ObjectArray { } } - /// XXX: I don't think there's a way to expose this via std::ops::IndexMut sadly? + /// Sets an element at the given `index` in the array. Returns an exception if the index is invalid. + /// + /// XXX: I don't think there's a way to expose this via `std::ops::IndexMut` sadly? pub fn set<'env>(self: &Ref<'env, Self>, index: usize, value: impl AsArg) -> Result<(), Local<'env, E>> { assert!(index <= std::i32::MAX as usize); // jsize == jint == i32 XXX: Should maybe be treated as an exception? let index = index as jsize; @@ -259,6 +282,8 @@ impl ObjectArray { } } +/// An iterator over object items of an [ObjectArray]. Local references of object items +/// will be created automatically. pub struct ObjectArrayIter<'a, 'env, T: ReferenceType, E: ThrowableType> { array: &'a Ref<'env, ObjectArray>, index: usize, diff --git a/java-spaghetti/src/as_jvalue.rs b/java-spaghetti/src/as_jvalue.rs index a942d44..a352352 100644 --- a/java-spaghetti/src/as_jvalue.rs +++ b/java-spaghetti/src/as_jvalue.rs @@ -53,7 +53,9 @@ unsafe impl AsJValue for jdouble { jvalue { d: *self } } } -//unsafe impl AsJValue for jobject { fn as_jvalue(&self) -> jvalue { jvalue { l: *self } } } // do NOT implement, no guarantee any given jobject is actually safe! + +// do NOT implement, no guarantee any given jobject is actually safe! +// unsafe impl AsJValue for jobject { fn as_jvalue(&self) -> jvalue { jvalue { l: *self } } } unsafe impl AsJValue for Ref<'_, T> { fn as_jvalue(&self) -> jvalue { diff --git a/java-spaghetti/src/env.rs b/java-spaghetti/src/env.rs index d06301a..d4c08d9 100644 --- a/java-spaghetti/src/env.rs +++ b/java-spaghetti/src/env.rs @@ -8,9 +8,9 @@ use jni_sys::*; use crate::{AsArg, Local, Ref, ReferenceType, StringChars, ThrowableType, VM}; -/// FFI: Use **Env** instead of \*const JNIEnv. This represents a per-thread Java exection environment. +/// FFI: Use **Env** instead of `*const JNIEnv`. This represents a per-thread Java exection environment. /// -/// A "safe" alternative to jni_sys::JNIEnv raw pointers, with the following caveats: +/// A "safe" alternative to `jni_sys::JNIEnv` raw pointers, with the following caveats: /// /// 1) A null env will result in **undefined behavior**. Java should not be invoking your native functions with a null /// *mut JNIEnv, however, so I don't believe this is a problem in practice unless you've bindgened the C header @@ -21,6 +21,8 @@ use crate::{AsArg, Local, Ref, ReferenceType, StringChars, ThrowableType, VM}; /// modify the JNIEnv, so as long as you're not accepting a *mut JNIEnv elsewhere, using unsafe to dereference it, /// and mucking with the methods on it yourself, I believe this "should" be fine. /// +/// Most methods of `Env` are supposed to be used by generated bindings. +/// /// # Example /// /// ### MainActivity.java @@ -38,16 +40,21 @@ use crate::{AsArg, Local, Ref, ReferenceType, StringChars, ThrowableType, VM}; /// /// ### main_activity.rs /// -/// ```rust -/// use jni_sys::{jboolean, jobject, JNI_TRUE}; // TODO: Replace with safer equivalent -/// use java_spaghetti::Env; +/// ```ignore +/// use java_spaghetti::{Env, Arg}; +/// use java_spaghetti::sys::{jboolean, JNI_TRUE}; +/// use bindings::java::lang::Object; +/// use bindings::android::view::KeyEvent; +/// +/// mod bindings; // Generated by `java-spaghetti-gen` /// -/// #[no_mangle] pub extern "system" +/// #[unsafe(no_mangle)] pub extern "system" /// fn Java_com_maulingmonkey_example_MainActivity_dispatchKeyEvent<'env>( -/// _env: Env<'env>, -/// _this: jobject, // TODO: Replace with safer equivalent -/// _key_event: jobject // TODO: Replace with safer equivalent +/// env: Env<'env>, +/// _this: Arg, +/// key_event: Arg, /// ) -> jboolean { +/// let key_event = unsafe { key_event.into_ref(env) }; /// // ... /// JNI_TRUE /// } @@ -61,6 +68,7 @@ pub struct Env<'env> { static CLASS_LOADER: AtomicPtr<_jobject> = AtomicPtr::new(null_mut()); +#[allow(clippy::missing_safety_doc)] impl<'env> Env<'env> { pub unsafe fn from_raw(ptr: *mut JNIEnv) -> Self { Self { @@ -132,6 +140,9 @@ impl<'env> Env<'env> { CLASS_LOADER.store(classloader, Ordering::Relaxed); } + /// Checks if an exception has occurred; if occurred, it clears the exception to make the next + /// JNI call possible, then it returns the exception as an `Err`. + /// /// XXX: Make this method public after making sure that it has a proper name. /// Note that there is `ExceptionCheck` in JNI functions, which does not create a /// local reference to the exception object. diff --git a/java-spaghetti/src/lib.rs b/java-spaghetti/src/lib.rs index 39ca593..bbe849a 100644 --- a/java-spaghetti/src/lib.rs +++ b/java-spaghetti/src/lib.rs @@ -1,7 +1,10 @@ -//! Common glue code between Rust and JNI, used in autogenerated `java-spaghetti` glue code. +//! Common glue code between Rust and JNI, used in auto-generated `java-spaghetti` glue code. //! //! See also the [Android JNI tips](https://developer.android.com/training/articles/perf-jni) documentation as well as the //! [Java Native Interface Specification](https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/jniTOC.html). +//! +//! Just like [jni-rs](https://docs.rs/jni/latest/jni/), thread safety of accessing Java objects are not guaranteed, unless +//! they are thread-safe by themselves. #![feature(arbitrary_self_types)] @@ -63,20 +66,30 @@ pub unsafe trait ReferenceType: JniType + Sized + 'static {} /// Marker trait indicating `Self` can be assigned to `T`. /// -/// This is true when `T` is a superclass or superinterface of `Self`. +/// # Safety +/// +/// `T` is a superclass or superinterface of `Self`. pub unsafe trait AssignableTo: ReferenceType {} /// A type is always assignable to itself. unsafe impl AssignableTo for T {} +/// A trait similar to `Display`. pub trait JavaDisplay: ReferenceType { fn fmt(self: &Ref<'_, Self>, f: &mut fmt::Formatter<'_>) -> fmt::Result; } +/// A trait similar to `Debug`. Currently it is implemented by `Throwable` in generated bindings. pub trait JavaDebug: ReferenceType { fn fmt(self: &Ref<'_, Self>, f: &mut fmt::Formatter<'_>) -> fmt::Result; } +/// A marker trait indicating this is a valid JNI reference type for Java method argument +/// type `T`, this can be null. +/// +/// # Safety +/// +/// It should be implemented automatically by `java_spaghetti`. pub unsafe trait AsArg: Sized { fn as_arg(&self) -> jobject; fn as_arg_jvalue(&self) -> jvalue { @@ -84,6 +97,7 @@ pub unsafe trait AsArg: Sized { } } +/// Represents a Java `null` value. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub struct Null; diff --git a/java-spaghetti/src/refs/arg.rs b/java-spaghetti/src/refs/arg.rs index 630aa66..7571f58 100644 --- a/java-spaghetti/src/refs/arg.rs +++ b/java-spaghetti/src/refs/arg.rs @@ -4,11 +4,11 @@ use jni_sys::*; use crate::{Env, Global, Local, Ref, ReferenceType}; -/// FFI: Use **Arg\** instead of jobject. This represents a (null?) function argument. +/// FFI: Use **Arg\** instead of `jobject`. This represents a (null?) function argument. /// /// Unlike most Java reference types from this library, this *can* be null. /// -/// FFI safe where a jobject is safe, assuming you match your types correctly. Using the wrong type may result in +/// FFI safe where a `jobject` is safe, assuming you match your types correctly. Using the wrong type may result in /// soundness issues, but at least on Android mostly seems to just result in JNI aborting execution for the current /// process when calling methods on an instance of the wrong type. #[repr(transparent)] @@ -18,7 +18,9 @@ pub struct Arg { } impl Arg { - /// **unsafe**: There's no guarantee the jobject being passed is valid or null, nor any means of checking it. + /// # Safety + /// + /// **unsafe**: There's no guarantee the `jobject` being passed is valid or null, nor any means of checking it. pub unsafe fn from_raw(object: jobject) -> Self { Self { object, @@ -26,13 +28,16 @@ impl Arg { } } + /// Returns the raw JNI reference pointer. pub fn as_raw(&self) -> jobject { self.object } - /// **unsafe**: This assumes the argument belongs to the given Env/VM, which is technically unsound. However, the - /// intended use case of immediately converting any Arg s into ArgRef s at the start of a JNI callback, - /// where Java directly invoked your function with an Env + arguments, is sound. + /// # Safety + /// + /// **unsafe**: This assumes the argument belongs to the given [Env], which is technically unsound. However, + /// the intended use case of immediately converting any [Arg] into [Ref] at the start of a JNI callback, + /// where Java directly invoked your function with an [Env] + arguments, is sound. pub unsafe fn into_ref<'env>(self, env: Env<'env>) -> Option> { if self.object.is_null() { None @@ -41,9 +46,11 @@ impl Arg { } } - /// **unsafe**: This assumes the argument belongs to the given Env/VM, which is technically unsound. However, the - /// intended use case of immediately converting any Arg s into ArgRef s at the start of a JNI callback, - /// where Java directly invoked your function with an Env + arguments, is sound. + /// # Safety + /// + /// **unsafe**: This assumes the argument belongs to the given [Env], which is technically unsound. However, + /// the intended use case of immediately converting any [Arg] into [Local] at the start of a JNI callback, + /// where Java directly invoked your function with an [Env] + arguments, is sound. pub unsafe fn into_local<'env>(self, env: Env<'env>) -> Option> { if self.object.is_null() { None @@ -52,17 +59,12 @@ impl Arg { } } - /// **unsafe**: This assumes the argument belongs to the given Env/VM, which is technically unsound. However, the - /// intended use case of immediately converting any Arg s into ArgRef s at the start of a JNI callback, - /// where Java directly invoked your function with an Env + arguments, is sound. + /// This equals [Arg::into_ref] + [Ref::as_global]. + /// + /// # Safety + /// + /// **unsafe**: The same as [Arg::into_ref]. pub unsafe fn into_global(self, env: Env) -> Option> { - if self.object.is_null() { - None - } else { - let jnienv = env.as_raw(); - let object = ((**jnienv).v1_2.NewGlobalRef)(jnienv, self.object); - assert!(!object.is_null()); - Some(Global::from_raw(env.vm(), object)) - } + self.into_ref(env).as_ref().map(Ref::as_global) } } diff --git a/java-spaghetti/src/refs/global.rs b/java-spaghetti/src/refs/global.rs index 4aed8a5..97a29c4 100644 --- a/java-spaghetti/src/refs/global.rs +++ b/java-spaghetti/src/refs/global.rs @@ -11,9 +11,9 @@ use crate::{Env, Local, Ref, ReferenceType, VM}; /// * You must create a [Ref] before use. /// * The [Global] can be invalidated if the [VM] is unloaded. /// -/// **Not FFI Safe:** #\[repr(rust)\], and exact layout is likely to change - depending on exact features used - in the -/// future. Specifically, on Android, since we're guaranteed to only have a single ambient [VM], we can likely store the -/// *const JavaVM in static and/or thread local storage instead of lugging it around in every [Local]. Of course, there's +/// **Not FFI Safe:** `#[repr(rust)]`, and exact layout is likely to change - depending on exact features used - in the +/// future. Specifically, on Android, since we're guaranteed to only have a single ambient VM, we can likely store the +/// `*const JavaVM` in static and/or thread local storage instead of lugging it around in every [Global]. Of course, there's /// no guarantee that's actually an *optimization*... pub struct Global { object: jobject, @@ -25,6 +25,12 @@ unsafe impl Send for Global {} unsafe impl Sync for Global {} impl Global { + /// Wraps an owned raw JNI global reference, taking the ownership. + /// + /// # Safety + /// + /// `object` must be an owned non-null JNI global reference to an object of type `T`, + /// not to be deleted by another wrapper. pub unsafe fn from_raw(vm: VM, object: jobject) -> Self { Self { object, @@ -33,27 +39,34 @@ impl Global { } } + /// Gets the [VM] under which the JNI reference is created. pub fn vm(&self) -> VM { self.vm } + /// Returns the raw JNI reference pointer. pub fn as_raw(&self) -> jobject { self.object } + /// Leaks the `Global` and turns it into a raw pointer, preserving the ownership of + /// one JNI global reference; prevents `DeleteGlobalRef` from being called on dropping. pub fn into_raw(self) -> jobject { let object = self.object; std::mem::forget(self); // Don't delete the object. object } + /// Returns a new JNI local reference of the same Java object. pub fn as_local<'env>(&self, env: Env<'env>) -> Local<'env, T> { - let jnienv = env.as_raw(); - let object = unsafe { ((**jnienv).v1_2.NewLocalRef)(jnienv, self.as_raw()) }; - assert!(!object.is_null()); - unsafe { Local::from_raw(env, object) } + // Safety: this `Ref<'env, T>` isn't available outside, and it does nothing on dropping. + let temp_ref = unsafe { Ref::from_raw(env, self.object) }; + temp_ref.as_local() // creates a new `Local` } + /// Returns a [Ref], with which Java methods from generated bindings can be used. + /// The lifetime of the returned [Ref] can be the intersection of this `Global` + /// and a supposed local reference under `env`. pub fn as_ref<'env>(&'env self, env: Env<'env>) -> Ref<'env, T> { unsafe { Ref::from_raw(env, self.object) } } diff --git a/java-spaghetti/src/refs/local.rs b/java-spaghetti/src/refs/local.rs index af5b56e..a433c01 100644 --- a/java-spaghetti/src/refs/local.rs +++ b/java-spaghetti/src/refs/local.rs @@ -1,4 +1,5 @@ use std::fmt::{self, Debug, Display, Formatter}; +use std::mem::transmute; use std::ops::Deref; use jni_sys::*; @@ -8,13 +9,13 @@ use crate::{AssignableTo, Env, Global, JavaDebug, JavaDisplay, Ref, ReferenceTyp /// A [Local](https://www.ibm.com/docs/en/sdk-java-technology/8?topic=collector-overview-jni-object-references), /// non-null, reference to a Java object (+ [Env]) limited to the current thread/stack. /// -/// Including the env allows for the convenient execution of methods without having to individually pass the env as an -/// argument to each and every one. Since this is limited to the current thread/stack, these cannot be sanely stored +/// Including the `env` allows for the convenient execution of methods without having to individually pass the `env` as +/// an argument to each and every one. Since this is limited to the current thread/stack, these cannot be sanely stored /// in any kind of static storage, nor shared between threads - instead use a [Global] if you need to do either. /// -/// Will `DeleteLocalRef` when dropped, invalidating the jobject but ensuring threads that rarely or never return to +/// Will `DeleteLocalRef` when dropped, invalidating the `jobject` but ensuring threads that rarely or never return to /// Java may run without being guaranteed to eventually exhaust their local reference limit. If this is not desired, -/// convert to a plain Ref with: +/// convert to a plain [Ref] with: /// /// ```rust,no_run /// # use java_spaghetti::*; @@ -23,84 +24,89 @@ use crate::{AssignableTo, Env, Global, JavaDebug, JavaDisplay, Ref, ReferenceTyp /// # } /// ``` /// -/// **Not FFI Safe:** #\[repr(rust)\], and exact layout is likely to change - depending on exact features used - in the +/// **Not FFI Safe:** `#[repr(rust)]`, and exact layout is likely to change - depending on exact features used - in the /// future. Specifically, on Android, since we're guaranteed to only have a single ambient VM, we can likely store the -/// \*const JNIEnv in thread local storage instead of lugging it around in every Local. Of course, there's no +/// `*const JNIEnv` in thread local storage instead of lugging it around in every [Local]. Of course, there's no /// guarantee that's actually an *optimization*... +#[repr(C)] // this is NOT for FFI-safety, this is to ensure `cast` methods are sound. pub struct Local<'env, T: ReferenceType> { ref_: Ref<'env, T>, } impl<'env, T: ReferenceType> Local<'env, T> { + /// Wraps an owned raw JNI local reference, taking the ownership. + /// + /// # Safety + /// + /// - `object` must be an owned non-null JNI local reference that belongs to `env`, + /// not to be deleted by another wrapper. + /// - `object` references an instance of type `T`. pub unsafe fn from_raw(env: Env<'env>, object: jobject) -> Self { Self { ref_: Ref::from_raw(env, object), } } + /// Gets the [Env] under which the JNI reference is valid. pub fn env(&self) -> Env<'env> { self.ref_.env() } + /// Returns the raw JNI reference pointer. pub fn as_raw(&self) -> jobject { self.ref_.as_raw() } + /// Leaks the `Local` and turns it into a raw pointer, preserving the ownership of one JNI + /// local reference; prevents `DeleteLocalRef` from being called on dropping. See [Local::leak]. pub fn into_raw(self) -> jobject { let object = self.ref_.as_raw(); - std::mem::forget(self); // Don't allow local to DeleteLocalRef the jobject + std::mem::forget(self); // Don't allow `Local` to delete it object } + /// Leaks the `Local`, prevents `DeleteLocalRef` from being called on dropping. + /// + /// If the current thread is a Java thread, it will be freed when the control flow returns + /// to Java; otherwise it will be freed when the native thread exits. + /// + /// NOTE: some JVM implementations have a strict limitation of local reference capacity, + /// an uncatchable error will be thrown if the capacity is full. pub fn leak(self) -> Ref<'env, T> { unsafe { Ref::from_raw(self.env(), self.into_raw()) } } + /// Returns a new JNI global reference of the same Java object. pub fn as_global(&self) -> Global { - let env = self.env(); - let jnienv = env.as_raw(); - let object = unsafe { ((**jnienv).v1_2.NewGlobalRef)(jnienv, self.as_raw()) }; - assert!(!object.is_null()); - unsafe { Global::from_raw(env.vm(), object) } - } - - pub fn as_ref(&self) -> &Ref<'_, T> { - &self.ref_ + self.as_ref().as_global() } + /// Creates and leaks a new local reference to be returned from the JNI `extern` callback function. + /// It will be freed as soon as the control flow returns to Java. pub fn as_return(&self) -> Return<'env, T> { - let env: *mut *const JNINativeInterface_ = self.env().as_raw(); - let object = unsafe { ((**env).v1_2.NewLocalRef)(env, self.as_raw()) }; - assert!(!object.is_null()); - unsafe { Return::from_raw(object) } + self.clone().into_return() } + /// Leaks the local reference to be returned from the JNI `extern` callback function. + /// It will be freed as soon as the control flow returns to Java. pub fn into_return(self) -> Return<'env, T> { unsafe { Return::from_raw(self.into_raw()) } } - pub fn cast(&self) -> Result, crate::CastError> { - let env = self.env(); - let jnienv = env.as_raw(); - let class1 = unsafe { ((**jnienv).v1_2.GetObjectClass)(jnienv, self.as_raw()) }; - let class2 = U::static_with_jni_type(|t| unsafe { env.require_class(t) }); - if !unsafe { ((**jnienv).v1_2.IsAssignableFrom)(jnienv, class1, class2) } { - return Err(crate::CastError); - } - let object = unsafe { ((**jnienv).v1_2.NewLocalRef)(jnienv, self.as_raw()) }; - assert!(!object.is_null()); - Ok(unsafe { Local::from_raw(env, object) }) + /// Tries to cast itself to a JNI reference of type `U`. + pub fn cast(self) -> Result, crate::CastError> { + self.as_ref().check_assignable::()?; + // Memory layout of the inner `Ref<'env, U>` is the same as `Ref<'env, T>`. + Ok(unsafe { transmute::, Local<'_, U>>(self) }) } - pub fn upcast(&self) -> Local<'env, U> + /// Casts itself towards a super class type, without the cost of runtime checking. + pub fn upcast(self) -> Local<'env, U> where Self: AssignableTo, { - let env = self.env(); - let jnienv = env.as_raw(); - let object = unsafe { ((**jnienv).v1_2.NewLocalRef)(jnienv, self.as_raw()) }; - assert!(!object.is_null()); - unsafe { Local::from_raw(env, object) } + // Memory layout of the inner `Ref<'env, U>` is the same as `Ref<'env, T>`. + unsafe { transmute(self) } } } @@ -122,6 +128,17 @@ impl<'env, T: ReferenceType> From<&Ref<'env, T>> for Local<'env, T> { } } +// NOTE: `AsRef` would become **unsound** if `Ref` should implement `Copy` or `Clone`. +// +// It is possible to have a safe `pub fn as_ref(&'env self) -> Ref<'env, T>` outside of +// `AsRef` trait, however a borrowed `Ref` is returned for the convenience of use. +impl<'env, T: ReferenceType> AsRef> for Local<'env, T> { + fn as_ref(&self) -> &Ref<'env, T> { + &self.ref_ + } +} + +// NOTE: `Deref` would become **unsound** if `Ref` should implement `Copy` or `Clone`. impl<'env, T: ReferenceType> Deref for Local<'env, T> { type Target = Ref<'env, T>; fn deref(&self) -> &Self::Target { diff --git a/java-spaghetti/src/refs/ref_.rs b/java-spaghetti/src/refs/ref_.rs index 0e5448a..b47d10a 100644 --- a/java-spaghetti/src/refs/ref_.rs +++ b/java-spaghetti/src/refs/ref_.rs @@ -1,6 +1,7 @@ use std::fmt::{self, Debug, Display, Formatter}; use std::marker::PhantomData; use std::mem::transmute; +use std::ops::Deref; use jni_sys::jobject; @@ -9,9 +10,11 @@ use crate::{AssignableTo, Env, Global, JavaDebug, JavaDisplay, Local, ReferenceT /// A non-null, [reference](https://www.ibm.com/docs/en/sdk-java-technology/8?topic=collector-overview-jni-object-references) /// to a Java object (+ [Env]). This may refer to a [Local](crate::Local), [Global](crate::Global), local [Arg](crate::Arg), etc. /// -/// **Not FFI Safe:** #\[repr(rust)\], and exact layout is likely to change - depending on exact features used - in the +/// `Ref` guarantees not to have a `Drop` implementation. It does not own the JNI reference. +/// +/// **Not FFI Safe:** `#[repr(rust)]`, and exact layout is likely to change - depending on exact features used - in the /// future. Specifically, on Android, since we're guaranteed to only have a single ambient VM, we can likely store the -/// \*const JNIEnv in thread local storage instead of lugging it around in every Local. Of course, there's no +/// `*const JNIEnv` in thread local storage instead of lugging it around in every [Ref]. Of course, there's no /// guarantee that's actually an *optimization*... #[repr(C)] // this is NOT for FFI-safety, this is to ensure `cast` methods are sound. pub struct Ref<'env, T: ReferenceType> { @@ -25,6 +28,13 @@ impl<'env, T: ReferenceType> std::ops::Receiver for Ref<'env, T> { } impl<'env, T: ReferenceType> Ref<'env, T> { + /// Wraps an raw JNI reference. + /// + /// # Safety + /// + /// - `object` is a non-null JNI reference, and it must keep valid under `env` before the created `Ref` goes out of scope. + /// This means it should not be a raw pointer managed by [Local] or any other wrapper that deletes it on dropping. + /// - `object` references an instance of type `T`. pub unsafe fn from_raw(env: Env<'env>, object: jobject) -> Self { Self { object, @@ -33,14 +43,17 @@ impl<'env, T: ReferenceType> Ref<'env, T> { } } + /// Gets the [Env] under which the JNI reference is valid. pub fn env(&self) -> Env<'env> { self.env } + /// Returns the raw JNI reference pointer. pub fn as_raw(&self) -> jobject { self.object } + /// Returns a new JNI global reference of the same Java object. pub fn as_global(&self) -> Global { let env = self.env(); let jnienv = env.as_raw(); @@ -49,6 +62,7 @@ impl<'env, T: ReferenceType> Ref<'env, T> { unsafe { Global::from_raw(env.vm(), object) } } + /// Returns a new JNI local reference of the same Java object. pub fn as_local(&self) -> Local<'env, T> { let env = self.env(); let jnienv = env.as_raw(); @@ -56,27 +70,46 @@ impl<'env, T: ReferenceType> Ref<'env, T> { assert!(!object.is_null()); unsafe { Local::from_raw(self.env(), object) } } + + /// Enters monitored mode or increments the JNI monitor counter in this thread for the referenced Java object. + /// See [Monitor] for the limited locking functionality. + pub fn as_monitor(&'env self) -> Monitor<'env, T> { + Monitor::new(self) + } + + /// Tests whether two JNI references refer to the same Java object. + pub fn is_same_object(&self, other: &Ref<'_, O>) -> bool { + let jnienv = self.env.as_raw(); + unsafe { ((**jnienv).v1_2.IsSameObject)(jnienv, self.as_raw(), other.as_raw()) } + } - fn check_assignable(&self) -> Result<(), crate::CastError> { + /// Checks if the Java object can be safely casted to type `U`. + pub(crate) fn check_assignable(&self) -> Result<(), crate::CastError> { let env = self.env(); let jnienv = env.as_raw(); - let class1 = unsafe { ((**jnienv).v1_2.GetObjectClass)(jnienv, self.as_raw()) }; - let class2 = U::static_with_jni_type(|t| unsafe { env.require_class(t) }); - if !unsafe { ((**jnienv).v1_2.IsAssignableFrom)(jnienv, class1, class2) } { + let class = U::static_with_jni_type(|t| unsafe { env.require_class(t) }); + if !unsafe { ((**jnienv).v1_2.IsInstanceOf)(jnienv, self.as_raw(), class) } { return Err(crate::CastError); } Ok(()) } + /// Casts itself to a JNI reference of type `U` forcefully, without the cost of runtime checking. + /// + /// # Safety + /// + /// - `self` references an instance of type `U`. pub unsafe fn cast_unchecked(self) -> Ref<'env, U> { transmute(self) } + /// Tries to cast itself to a JNI reference of type `U`. pub fn cast(self) -> Result, crate::CastError> { self.check_assignable::()?; Ok(unsafe { self.cast_unchecked() }) } + /// Casts itself towards a super class type, without the cost of runtime checking. pub fn upcast(self) -> Ref<'env, U> where Self: AssignableTo, @@ -84,15 +117,22 @@ impl<'env, T: ReferenceType> Ref<'env, T> { unsafe { self.cast_unchecked() } } + /// Casts the borrowed `Ref` to a JNI reference of type `U` forcefully, without the cost of runtime checking. + /// + /// # Safety + /// + /// - `self` references an instance of type `U`. pub unsafe fn cast_ref_unchecked(&self) -> &Ref<'env, U> { transmute(self) } + /// Tries to cast the borrowed `Ref` to a JNI reference of type `U`. pub fn cast_ref(&self) -> Result<&Ref<'env, U>, crate::CastError> { self.check_assignable::()?; Ok(unsafe { self.cast_ref_unchecked() }) } + /// Casts the borrowed `Ref` towards a super class type, without the cost of runtime checking. pub fn upcast_ref(&self) -> &Ref<'env, U> where Self: AssignableTo, @@ -112,3 +152,56 @@ impl<'env, T: JavaDisplay> Display for Ref<'env, T> { T::fmt(self, f) } } + +/// A borrowed [Ref] of a Java object locked with the JNI monitor mechanism, providing *limited* thread safety. +/// +/// **It is imposible to be FFI safe.** It is important to drop the monitor or call [Monitor::unlock()] when appropriate. +/// +/// Limitations: +/// +/// - It merely blocks other native functions from owning the JNI monitor of the same object before it drops. +/// - It may not block other native functions from using the corresponding object without entering monitored mode. +/// - It may not prevent any Java method or block from using this object, even if it is marked as `synchronized`. +/// - While it is a reentrant lock for the current thread, dead lock is still possible under multi-thread conditions. +pub struct Monitor<'env, T: ReferenceType> { + inner: &'env Ref<'env, T>, +} + +impl<'env, T: ReferenceType> Monitor<'env, T> { + fn new(reference: &'env Ref<'env, T>) -> Self { + let jnienv = reference.env.as_raw(); + let result = unsafe { ((**jnienv).v1_2.MonitorEnter)(jnienv, reference.as_raw()) }; + assert!(result == jni_sys::JNI_OK); + Self { inner: reference } + } + + /// Decrements the JNI monitor counter indicating the number of times it has entered this monitor. + /// If the value of the counter becomes zero, the current thread releases the monitor. + pub fn unlock(self) -> &'env Ref<'env, T> { + let inner = self.inner; + drop(self); // this looks clearer than dropping implicitly + inner + } +} + +impl<'env, T: ReferenceType> Deref for Monitor<'env, T> { + type Target = Ref<'env, T>; + fn deref(&self) -> &Self::Target { + self.inner + } +} + +impl<'env, T: ReferenceType> Drop for Monitor<'env, T> { + fn drop(&mut self) { + let env = self.inner.env; + let jnienv = env.as_raw(); + let result = unsafe { ((**jnienv).v1_2.MonitorExit)(jnienv, self.inner.as_raw()) }; + assert!(result == jni_sys::JNI_OK); + if let Err(exception) = env.exception_check_raw() { + panic!( + "exception happened calling JNI MonitorExit, the monitor is probably broken previously: {}", + unsafe { env.raw_exception_to_string(exception) } + ); + } + } +} diff --git a/java-spaghetti/src/refs/return_.rs b/java-spaghetti/src/refs/return_.rs index b84916b..f17d9b4 100644 --- a/java-spaghetti/src/refs/return_.rs +++ b/java-spaghetti/src/refs/return_.rs @@ -5,9 +5,10 @@ use jni_sys::jobject; use crate::ReferenceType; -/// FFI: Use **Return\** instead of jobject. This represents a (null?) JNI function call return value. +/// FFI: Use **Return\** instead of `jobject`. This represents a (null?) JNI function call return value. /// -/// Unlike most Java reference types from this library, this *can* be null. +/// Unlike most Java reference types from this library, this *can* be null. Recommended constructors are +/// [crate::Local::into_return] and [Return::null]. /// /// FFI safe where a jobject is safe, assuming you match your types correctly. #[repr(transparent)] @@ -17,6 +18,13 @@ pub struct Return<'env, T: ReferenceType> { } impl<'env, T: ReferenceType> Return<'env, T> { + /// Wraps a raw JNI reference. + /// + /// # Safety + /// + /// - If `object` is non-null, it must be a JNI local(?) reference to an instance of type `T`; + /// - `object` must keep valid for `'env` lifetime; it is not owned by `Local` or any other wrapper + /// that deletes the reference on `Drop` before the JNI native method call returns. pub unsafe fn from_raw(object: jobject) -> Self { Self { object, @@ -24,6 +32,7 @@ impl<'env, T: ReferenceType> Return<'env, T> { } } + /// Creates a null value to be returned from the JNI native method. pub fn null() -> Self { Self { object: null_mut(), @@ -31,12 +40,14 @@ impl<'env, T: ReferenceType> Return<'env, T> { } } + /// Returns the raw JNI reference pointer. Generally it should not be used. pub fn as_raw(&self) -> jobject { self.object } } impl<'env, T: ReferenceType> Default for Return<'env, T> { + /// This is a null value. fn default() -> Self { Self::null() } diff --git a/java-spaghetti/src/string_chars.rs b/java-spaghetti/src/string_chars.rs index 9d88358..380f9c2 100644 --- a/java-spaghetti/src/string_chars.rs +++ b/java-spaghetti/src/string_chars.rs @@ -4,8 +4,8 @@ use jni_sys::*; use crate::Env; -/// Represents an env.GetStringChars + env.GetStringLength query. -/// Will automatically env.ReleaseStringChars when dropped. +/// Represents a JNI `GetStringChars` + `GetStringLength` query. +/// It will call `ReleaseStringChars` automatically when dropped. pub struct StringChars<'env> { env: Env<'env>, string: jstring, @@ -14,7 +14,14 @@ pub struct StringChars<'env> { } impl<'env> StringChars<'env> { - /// Construct a StringChars from an Env + jstring. + /// Construct a `StringChars` from an [Env] + [jstring]. + /// + /// # Safety + /// + /// The Java string object referenced by `string` must remain available before the created + /// `StringChars` is dropped. This should be true if the JNI reference `string` is not deleted. + /// + /// This function is supposed to be used in generated bindings. pub unsafe fn from_env_jstring(env: Env<'env>, string: jstring) -> Self { debug_assert!(!string.is_null()); diff --git a/java-spaghetti/src/vm.rs b/java-spaghetti/src/vm.rs index 931d145..7e35413 100644 --- a/java-spaghetti/src/vm.rs +++ b/java-spaghetti/src/vm.rs @@ -5,20 +5,20 @@ use jni_sys::*; use crate::Env; -/// FFI: Use **&VM** instead of *const JavaVM. This represents a global, process-wide Java exection environment. +/// FFI: Use **&VM** instead of `*const JavaVM`. This represents a global, process-wide Java exection environment. /// /// On Android, there is only one VM per-process, although on desktop it's possible (if rare) to have multiple VMs /// within the same process. This library does not support having multiple VMs active simultaniously. /// -/// This is a "safe" alternative to jni_sys::JavaVM raw pointers, with the following caveats: +/// This is a "safe" alternative to `jni_sys::JavaVM` raw pointers, with the following caveats: /// /// 1) A null vm will result in **undefined behavior**. Java should not be invoking your native functions with a null -/// *mut JavaVM, however, so I don't believe this is a problem in practice unless you've bindgened the C header +/// `*mut JavaVM`, however, so I don't believe this is a problem in practice unless you've bindgened the C header /// definitions elsewhere, calling them (requiring `unsafe`), and passing null pointers (generally UB for JNI /// functions anyways, so can be seen as a caller soundness issue.) /// /// 2) Allowing the underlying JavaVM to be modified is **undefined behavior**. I don't believe the JNI libraries -/// modify the JavaVM, so as long as you're not accepting a *mut JavaVM elsewhere, using unsafe to dereference it, +/// modify the JavaVM, so as long as you're not accepting a `*mut JavaVM` elsewhere, using unsafe to dereference it, /// and mucking with the methods on it yourself, I believe this "should" be fine. #[repr(transparent)] #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] From d79db4e77c6a9201c90d1587d732c898b4d89491 Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Fri, 28 Mar 2025 11:09:50 +0800 Subject: [PATCH 4/5] Execute `cargo fmt` --- java-spaghetti/src/refs/local.rs | 2 +- java-spaghetti/src/refs/ref_.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/java-spaghetti/src/refs/local.rs b/java-spaghetti/src/refs/local.rs index a433c01..3c266b0 100644 --- a/java-spaghetti/src/refs/local.rs +++ b/java-spaghetti/src/refs/local.rs @@ -57,7 +57,7 @@ impl<'env, T: ReferenceType> Local<'env, T> { self.ref_.as_raw() } - /// Leaks the `Local` and turns it into a raw pointer, preserving the ownership of one JNI + /// Leaks the `Local` and turns it into a raw pointer, preserving the ownership of one JNI /// local reference; prevents `DeleteLocalRef` from being called on dropping. See [Local::leak]. pub fn into_raw(self) -> jobject { let object = self.ref_.as_raw(); diff --git a/java-spaghetti/src/refs/ref_.rs b/java-spaghetti/src/refs/ref_.rs index b47d10a..cb547a9 100644 --- a/java-spaghetti/src/refs/ref_.rs +++ b/java-spaghetti/src/refs/ref_.rs @@ -70,13 +70,13 @@ impl<'env, T: ReferenceType> Ref<'env, T> { assert!(!object.is_null()); unsafe { Local::from_raw(self.env(), object) } } - + /// Enters monitored mode or increments the JNI monitor counter in this thread for the referenced Java object. /// See [Monitor] for the limited locking functionality. pub fn as_monitor(&'env self) -> Monitor<'env, T> { Monitor::new(self) } - + /// Tests whether two JNI references refer to the same Java object. pub fn is_same_object(&self, other: &Ref<'_, O>) -> bool { let jnienv = self.env.as_raw(); From 98ed3dc25412ccdb4f6a5960653d6115edfe82f1 Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Fri, 28 Mar 2025 11:22:17 +0800 Subject: [PATCH 5/5] Fix `Monitor` under current branch --- java-spaghetti/src/refs/ref_.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/java-spaghetti/src/refs/ref_.rs b/java-spaghetti/src/refs/ref_.rs index cb547a9..bbfff2f 100644 --- a/java-spaghetti/src/refs/ref_.rs +++ b/java-spaghetti/src/refs/ref_.rs @@ -197,11 +197,10 @@ impl<'env, T: ReferenceType> Drop for Monitor<'env, T> { let jnienv = env.as_raw(); let result = unsafe { ((**jnienv).v1_2.MonitorExit)(jnienv, self.inner.as_raw()) }; assert!(result == jni_sys::JNI_OK); - if let Err(exception) = env.exception_check_raw() { - panic!( - "exception happened calling JNI MonitorExit, the monitor is probably broken previously: {}", - unsafe { env.raw_exception_to_string(exception) } - ); - } + let exception = unsafe { ((**jnienv).v1_2.ExceptionOccurred)(jnienv) }; + assert!( + exception.is_null(), + "exception happened calling JNI MonitorExit, the monitor is probably broken previously" + ); } }