Add docs; add Monitor, is_same_object; optimize Local::cast

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.

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<T>: 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<usize>) -> Vec<T> {
         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<T> {
         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<T: ReferenceType, E: ThrowableType>(core::convert::Infallible, PhantomData<(T, E)>);
 
@@ -187,6 +200,7 @@ unsafe impl<T: ReferenceType, E: ThrowableType> JniType for ObjectArray<T, E> {
 }
 
 impl<T: ReferenceType, E: ThrowableType> ObjectArray<T, E> {
+    /// 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<T: ReferenceType, E: ThrowableType> ObjectArray<T, E> {
         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<T: ReferenceType, E: ThrowableType> ObjectArray<T, E> {
         }
     }
 
+    /// 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<Item = impl AsArg<T>>,
@@ -224,12 +241,16 @@ impl<T: ReferenceType, E: ThrowableType> ObjectArray<T, E> {
         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<Option<Local<'env, T>>, 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<T: ReferenceType, E: ThrowableType> ObjectArray<T, E> {
         }
     }
 
-    /// 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<T>) -> 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<T: ReferenceType, E: ThrowableType> ObjectArray<T, E> {
     }
 }
 
+/// 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<T, E>>,
     index: usize,

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<T: ReferenceType> AsJValue for Ref<'_, T> {
     fn as_jvalue(&self) -> jvalue {

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<Object>,
+///     key_event: Arg<KeyEvent>,
 /// ) -> 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.

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,27 +66,38 @@ 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<T: ReferenceType>: ReferenceType {}
 
 /// A type is always assignable to itself.
 unsafe impl<T: ReferenceType> AssignableTo<T> 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<T>: Sized {
     fn as_arg(&self) -> jobject;
     fn as_arg_jvalue(&self) -> jvalue {
         jvalue { l: self.as_arg() }
     }
 }
 
+/// Represents a Java `null` value.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub struct Null;
 

java-spaghetti/src/refs/arg.rs

@@ -4,11 +4,11 @@ use jni_sys::*;
 
 use crate::{Env, Global, Local, Ref, ReferenceType};
 
-/// FFI: Use **Arg\<java::lang::Object\>** instead of jobject.  This represents a (null?) function argument.
+/// FFI: Use **Arg\<java::lang::Object\>** 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,21 +18,26 @@ pub struct Arg<T: ReferenceType> {
 }
 
 impl<T: ReferenceType> Arg<T> {
-    /// **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,
             _class: PhantomData,
         }
     }
 
+    /// 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<Ref<'env, T>> {
         if self.object.is_null() {
             None
@@ -41,9 +46,11 @@ impl<T: ReferenceType> Arg<T> {
         }
     }
 
-    /// **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<Local<'env, T>> {
         if self.object.is_null() {
             None
@@ -52,17 +59,12 @@ impl<T: ReferenceType> Arg<T> {
         }
     }
 
-    /// **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<Global<T>> {
-        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)
     }
 }