Add Array wrapper types with lifetimes

CHANGELOG.md

@@ -22,10 +22,14 @@ This release makes extensive breaking changes in order to improve safety. Most p
 - `JavaStr::from_raw()` which takes ownership of a raw string pointer to create a `JavaStr` ([#374](https://github.com/jni-rs/jni-rs/pull/374))
 - `JNIEnv::get_string_unchecked` is a cheaper, `unsafe` alternative to `get_string` that doesn't check the given object is a `java.lang.String` instance. ([#328](https://github.com/jni-rs/jni-rs/issues/328))
 - `WeakRef` and `JNIEnv#new_weak_ref`. ([#304](https://github.com/jni-rs/jni-rs/pull/304))
-- `define_class_bytearray` method that takes an `AutoArray<jbyte>` rather than a `&[u8]`
+- `define_class_bytearray` method that takes an `AutoElements<jbyte>` rather than a `&[u8]` ([#244](https://github.com/jni-rs/jni-rs/pull/244))
 - `JObject` now has an `as_raw` method that borrows the `JObject` instead of taking ownership like `into_raw`. Needed because `JObject` no longer has the `Copy` trait. ([#392](https://github.com/jni-rs/jni-rs/issues/392))
 - `JavaVM::destroy()` (unsafe) as a way to try and unload a `JavaVM` on supported platforms ([#391](https://github.com/jni-rs/jni-rs/issues/391))
 - `JavaVM::detach_current_thread()` (unsafe) as a way to explicitly detach a thread (normally this is automatic on thread exit). Needed to detach daemon threads manually if using `JavaVM::destroy()` ([#391](https://github.com/jni-rs/jni-rs/issues/391))
+- `JPrimitiveArray<T: TypeArray>` and type-specific aliases like `JByteArray`, `JIntArray` etc now provide safe, reference wrappers for the `sys` types `jarray` and `jbyteArray` etc with a lifetime like `JObject` ([#400](https://github.com/jni-rs/jni-rs/pull/400))
+- `JObjectArray` provides a reference wrapper for a `jobjectArray` with a lifetime like `JObject`. ([#400](https://github.com/jni-rs/jni-rs/pull/400))
+- `AutoElements` and `AutoElementsCritical` (previously `AutoArray`/`AutoPrimitiveArray`) implement `Deref<Target=[T]>` and `DerefMut` so array elements can be accessed via slices without needing additional `unsafe` code. ([#400](https://github.com/jni-rs/jni-rs/pull/400))
+- `AsJArrayRaw` trait which enables `JNIEnv::get_array_length()` to work with `JPrimitiveArray` or `JObjectArray` types ([#400](https://github.com/jni-rs/jni-rs/pull/400))
 
 ### Changed
 - `JNIEnv::get_string` checks that the given object is a `java.lang.String` instance to avoid undefined behaviour from the JNI implementation potentially aborting the program. ([#328](https://github.com/jni-rs/jni-rs/issues/328))
@@ -51,13 +55,21 @@ This release makes extensive breaking changes in order to improve safety. Most p
     - `JValueOwned` does not have the `Copy` trait.
     - The `to_jni` method is now named `as_jni`, and it borrows the `JValueGen` instead of taking ownership.
     - `JObject` can no longer be converted directly to `JValue`, which was commonly done when calling Java methods or constructors. Instead of `obj.into()`, use `(&obj).into()`.
+- All `JNIEnv` array APIs now work in terms of `JPrimitiveArray` and `JObjectArray` (reference wrappers with a lifetime) instead of `sys` types like `jarray` and `jbyteArray` ([#400](https://github.com/jni-rs/jni-rs/pull/400))
+- `AutoArray` and `AutoPrimitiveArray` have been renamed `AutoElements` and `AutoElementsCritical` to show their connection and differentiate from new `JPrimitiveArray` API ([#400](https://github.com/jni-rs/jni-rs/pull/400))
+- `get_primitive_array_critical` is now `unsafe` and has been renamed to `get_array_elements_critical` (consistent with the rename of `AutoPrimitiveArray`) with more detailed safety documentation ([#400](https://github.com/jni-rs/jni-rs/pull/400))
+- `get_array_elements` is now also `unsafe` (for many of the same reasons as `get_array_elements_critical`) and has detailed safety documentation ([#400](https://github.com/jni-rs/jni-rs/pull/400))
+- `AutoArray/AutoArrayCritical::size()` has been replaced with `.len()` which can't fail and returns a `usize` ([#400](https://github.com/jni-rs/jni-rs/pull/400))
+- The `TypeArray` trait is now a private / sealed trait, that is considered to be an implementation detail for the `AutoArray` API.
+
 
 ### Fixed
 - Trying to use an object reference after it has been deleted now causes a compile error instead of undefined behavior. As a result, it is now safe to use `AutoLocal`, `JNIEnv::delete_local_ref`, and `JNIEnv::with_local_frame`. (Most of the limitations added in #392, listed above, were needed to make this work.) ([#381](https://github.com/jni-rs/jni-rs/issues/381), [#392](https://github.com/jni-rs/jni-rs/issues/392))
 - Class lookups via the `Desc` trait now return `AutoLocal`s, which prevents them from leaking. ([#109](https://github.com/jni-rs/jni-rs/issues/109), [#392](https://github.com/jni-rs/jni-rs/issues/392))
 
 ### Removed
 - `get_string_utf_chars` and `release_string_utf_chars` from `JNIEnv` (See `JavaStr::into_raw()` and `JavaStr::from_raw()` instead) ([#372](https://github.com/jni-rs/jni-rs/pull/372))
+- All `JNIEnv::get_<type>_array_elements()` methods have been removed as redundant since they would all be equivalent to `get_array_elements()` with the introduction of `JPrimitiveArray` ([#400](https://github.com/jni-rs/jni-rs/pull/400))
 
 ## [0.20.0] — 2022-10-17
 

docs/0.21-MIGRATION.md

@@ -227,7 +227,6 @@ let object_ref: &JObject<'static> = global_ref.as_ref();
 ```
 
 
-
 ## `JNIEnv::get_superclass` now returns `Option`
 
 The `JNIEnv::get_superclass` method previously returned a `JClass`, which would be null if the class in question doesn't have a superclass. It now returns `Option<JClass>` instead, and when it's `Some`, the `JClass` inside is never null.
@@ -236,3 +235,92 @@ The `JNIEnv::get_superclass` method previously returned a `JClass`, which would
 ## `JNIEnv::{get,release}_string_utf_chars` removed
 
 The methods `JNIEnv::get_string_utf_chars` and `JNIEnv::release_string_utf_chars` have been removed. These methods have been replaced with `JavaStr::into_raw` and `JavaStr::from_raw`. To get a `JavaStr`, use `JNIEnv::get_string` or `JNIEnv::get_string_unchecked`. See [issue #372](https://github.com/jni-rs/jni-rs/pull/372) for discussion.
+
+
+## `JPrimitiveArray<T>` and `JObjectArray` provide safe reference wrappers (like `JObject`) for `jarray`
+
+This affects all `JNIEnv` array APIs including:
+- `new_<type>_array`
+- `get_array_length`
+- `get_object_array_element`
+- `set_object_array_element`
+- `get_<type>_array_region`
+- `set_<type>_array_region`
+- `get_array_elements`
+- `get_<type>_array_elements` (all removed)
+- `get_primitive_array_elements` (also renamed to `get_array_elements_critical`)
+
+With the exception of `get_array_length` these APIs now take or return a
+reference to a `JPrimitiveArray` or a `JObjectArray` instead of a `sys` type
+like `jarray` or `jbyteArray`. This improves safety since the sys types can be
+copied and easily read as invalid pointers after a reference has been deleted.
+
+`get_array_length` will accept a reference to a `JPrimitiveArray` or a `JObjectArray`
+since these both implement the `AsJArrayRaw` trait. You can also query the `.len()`
+of an array via the `AutoElements`/`AutoElementsCritical` APIs if you use those.
+
+There are the following type-specific aliases for `JPrimitiveArray<T>`:
+- `JBooleanArray`
+- `JByteArray`
+- `JCharArray`
+- `JShortArray`
+- `JIntArray`
+- `JLongArray`
+- `JFloatArray`
+- `JDoubleArray`
+
+
+## `AutoArray` and `AutoPrimitiveArray` renamed to `AutoElements` and `AutoElementsCritical` respectively
+
+This rename was done to:
+1. Clarify the connection between these APIs (they both provide temporary access to the elements of an array)
+2. Clarify their differences (`AutoElementsCritical` is an alternative that defines a restricted "critical" section that helps JNI avoid the need to copy the data for the array)
+3. Differentiate these from the new `JPrimitiveArray`/`JObjectArray` types and aliases like `JByteArray` that represent the array itself (not the elements)
+
+
+## `AutoElements<T>` and `AutoElementsCritical<T>` now implement `Deref<Target=[T]>` and `DerefMut`
+
+Previously accessing array elements required the use of `unsafe` code to access array elements via `AutoArray::as_ptr()` after acquiring an `AutoArray` guard.
+
+It's now possible to read and write elements via the `Deref` and `DerefMut` traits that will deref into a `[T]` slice like:
+
+```rust
+let byte_array = env.new_byte_array(100)?;
+
+{
+    let mut elements = unsafe { env.get_array_elements(&byte_array, ReleaseMode::CopyBack) }?;
+
+    elements[0] = 0xff;
+    assert_eq!(elements[0], 0xff);
+
+    // elements released (copied back to Java) here on Drop
+}
+```
+
+If you are accessing a `JPrimitiveArray<jbyte>`/`JByteArray` you may find it helpful
+to utilize the [bytemuck](https://crates.io/crates/bytemuck) crate in case you
+need to access the elements as `u8` unsigned bytes instead of `i8`.
+
+Although this hasn't changed, it seems worth highlighting that if you are
+accessing a `JPrimitiveArray<jboolean>`/`JBooleanArray` you are responsible for
+only storing values of `0` or `1`, since other values could lead to undefined
+behaviour for the JVM.
+
+
+## `AutoArray/AutoPrimitiveArray::size()` replace by `AutoElements/AutoElementsCritical::len()`
+
+Previously `AutoArray::size()` was a wrapper for calling `JNIEnv::get_array_length()` which could
+fail, where as `AutoElements` and `AutoElementsCritical` now need to know the array length to
+be constructed, and this constant is accessible via the `.len()` method.
+
+This change was required to support being able to `Deref` to a `[T]` slice.
+
+
+## `get_array_elements[_critical]` are `unsafe`
+
+Previously `get_array_elements` and `get_primitive_array_critical` could be called by safe code
+but there were multiple ways in which these APIs could quite easily lead to undefined behaviour.
+
+Since it is only possible to acquire an `AutoElements` and `AutoElementsCritical` instance via
+`get_array_elements` and `get_primitive_array_critical`, that's where we now document
+the safety rules that need to be followed to avoid any undefined behaviour.

example/mylib/Cargo.toml

@@ -2,7 +2,7 @@
 name = "mylib"
 version = "0.1.0"
 authors = ["Josh Chase <josh@prevoty.com>"]
-edition = "2018"
+edition = "2021"
 
 [dependencies]
 jni = { path = "../../" }

example/mylib/src/lib.rs

@@ -3,32 +3,47 @@
 use jni::JNIEnv;
 
 // These objects are what you should use as arguments to your native function.
-// They carry extra lifetime information to prevent them escaping this context
-// and getting used after being GC'd.
+// They carry extra lifetime information to prevent them escaping from the
+// current local frame (which is the scope within which local (temporary)
+// references to Java objects remain valid)
 use jni::objects::{GlobalRef, JClass, JObject, JString};
 
-// This is just a pointer. We'll be returning it from our function.
-// We can't return one of the objects with lifetime information because the
-// lifetime checker won't let us.
-use jni::sys::{jbyteArray, jint, jlong, jstring};
+use jni::objects::JByteArray;
+use jni::sys::{jint, jlong};
 
 use std::{sync::mpsc, thread, time::Duration};
 
-// This keeps rust from "mangling" the name and making it unique for this crate.
+// This `#[no_mangle]` keeps rust from "mangling" the name and making it unique
+// for this crate. The name follow a strict naming convention so that the
+// JNI implementation will be able to automatically find the implementation
+// of a native method based on its name.
+//
+// The `'local` lifetime here represents the local frame within which any local
+// (temporary) references to Java objects will remain valid.
+//
+// It's usually not necessary to explicitly name the `'local` input lifetimes but
+// in this case we want to return a reference and show the compiler what
+// local frame lifetime it is associated with.
+//
+// Alternatively we could instead return the `jni::sys::jstring` type instead
+// which would represent the same thing as a raw pointer, without any lifetime,
+// and at the end use `.into_raw()` to convert a local reference with a lifetime
+// into a raw pointer.
 #[no_mangle]
-pub extern "system" fn Java_HelloWorld_hello(
-    env: JNIEnv,
-    // this is the class that owns our
-    // static method. Not going to be
-    // used, but still needs to have
-    // an argument slot
-    _class: JClass,
-    input: JString,
-) -> jstring {
+pub extern "system" fn Java_HelloWorld_hello<'local>(
+    // Notice that this `env` argument is mutable. Any `JNIEnv` API that may
+    // allocate new object references will take a mutable reference to the
+    // environment.
+    mut env: JNIEnv<'local>,
+    // this is the class that owns our static method. Not going to be used, but
+    // still needs to have an argument slot
+    _class: JClass<'local>,
+    input: JString<'local>,
+) -> JString<'local> {
     // First, we have to get the string out of java. Check out the `strings`
     // module for more info on how this works.
     let input: String = env
-        .get_string(input)
+        .get_string(&input)
         .expect("Couldn't get java string!")
         .into();
 
@@ -37,29 +52,27 @@ pub extern "system" fn Java_HelloWorld_hello(
     let output = env
         .new_string(format!("Hello, {}!", input))
         .expect("Couldn't create java string!");
-    // Finally, extract the raw pointer to return.
-    output.into_raw()
+    output
 }
 
 #[no_mangle]
-pub extern "system" fn Java_HelloWorld_helloByte(
-    env: JNIEnv,
+pub extern "system" fn Java_HelloWorld_helloByte<'local>(
+    env: JNIEnv<'local>,
     _class: JClass,
-    input: jbyteArray,
-) -> jbyteArray {
+    input: JByteArray<'local>,
+) -> JByteArray<'local> {
     // First, we have to get the byte[] out of java.
-    let _input = env.convert_byte_array(input).unwrap();
+    let _input = env.convert_byte_array(&input).unwrap();
 
     // Then we have to create a new java byte[] to return.
     let buf = [1; 2000];
     let output = env.byte_array_from_slice(&buf).unwrap();
-    // Finally, extract the raw pointer to return.
     output
 }
 
 #[no_mangle]
 pub extern "system" fn Java_HelloWorld_factAndCallMeBack(
-    env: JNIEnv,
+    mut env: JNIEnv,
     _class: JClass,
     n: jint,
     callback: JObject,
@@ -84,7 +97,7 @@ impl Counter {
         }
     }
 
-    pub fn increment(&mut self, env: JNIEnv) {
+    pub fn increment(&mut self, env: &mut JNIEnv) {
         self.count = self.count + 1;
         env.call_method(
             &self.callback,
@@ -110,13 +123,13 @@ pub unsafe extern "system" fn Java_HelloWorld_counterNew(
 
 #[no_mangle]
 pub unsafe extern "system" fn Java_HelloWorld_counterIncrement(
-    env: JNIEnv,
+    mut env: JNIEnv,
     _class: JClass,
     counter_ptr: jlong,
 ) {
     let counter = &mut *(counter_ptr as *mut Counter);
 
-    counter.increment(env);
+    counter.increment(&mut env);
 }
 
 #[no_mangle]
@@ -152,7 +165,7 @@ pub extern "system" fn Java_HelloWorld_asyncComputation(
         tx.send(()).unwrap();
 
         // Use the `JavaVM` interface to attach a `JNIEnv` to the current thread.
-        let env = jvm.attach_current_thread().unwrap();
+        let mut env = jvm.attach_current_thread().unwrap();
 
         for i in 0..11 {
             let progress = (i * 10) as jint;