From a82d2be1b7f50bb799455cff75752e60259777cb Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Mon, 24 Mar 2025 08:56:10 +0800 Subject: [PATCH 01/10] Refactor `Env` API; Optimize casting and fix local reference leaks --- java-spaghetti-gen/src/emit_rust/fields.rs | 8 +- java-spaghetti-gen/src/emit_rust/methods.rs | 4 +- java-spaghetti-gen/src/emit_rust/structs.rs | 28 +- java-spaghetti/src/array.rs | 49 ++- java-spaghetti/src/env.rs | 392 +++++++++----------- java-spaghetti/src/id_cache.rs | 93 ++++- java-spaghetti/src/lib.rs | 6 +- java-spaghetti/src/refs/local.rs | 6 +- java-spaghetti/src/refs/ref_.rs | 7 +- 9 files changed, 326 insertions(+), 267 deletions(-) diff --git a/java-spaghetti-gen/src/emit_rust/fields.rs b/java-spaghetti-gen/src/emit_rust/fields.rs index 6ac782f..df5800d 100644 --- a/java-spaghetti-gen/src/emit_rust/fields.rs +++ b/java-spaghetti-gen/src/emit_rust/fields.rs @@ -173,9 +173,7 @@ impl<'a> Field<'a> { writeln!( out, "{indent} \ - let __jni_field = __FIELD.get_or_init(|| \ - ::java_spaghetti::JFieldID::from_raw(__jni_env.require_{}field(__jni_class, {}, {}))\ - ).as_raw();", + let __jni_field = *__FIELD.get_or_init(|| __jni_env.require_{}field(__jni_class, {}, {}));", if self.java.is_static() { "static_" } else { "" }, StrEmitter(self.java.name()), StrEmitter(FieldSigWriter(self.java.descriptor())) @@ -228,9 +226,7 @@ impl<'a> Field<'a> { writeln!( out, "{indent} \ - let __jni_field = __FIELD.get_or_init(|| \ - ::java_spaghetti::JFieldID::from_raw(__jni_env.require_{}field(__jni_class, {}, {}))\ - ).as_raw();", + let __jni_field = *__FIELD.get_or_init(|| __jni_env.require_{}field(__jni_class, {}, {}));", if self.java.is_static() { "static_" } else { "" }, StrEmitter(self.java.name()), StrEmitter(FieldSigWriter(self.java.descriptor())) diff --git a/java-spaghetti-gen/src/emit_rust/methods.rs b/java-spaghetti-gen/src/emit_rust/methods.rs index 4d7d6e6..99d3398 100644 --- a/java-spaghetti-gen/src/emit_rust/methods.rs +++ b/java-spaghetti-gen/src/emit_rust/methods.rs @@ -224,9 +224,7 @@ impl<'a> Method<'a> { writeln!( out, "{indent} \ - let __jni_method = __METHOD.get_or_init(|| \ - ::java_spaghetti::JMethodID::from_raw(__jni_env.require_{}method(__jni_class, {}, {}))\ - ).as_raw();", + let __jni_method = *__METHOD.get_or_init(|| __jni_env.require_{}method(__jni_class, {}, {}));", if self.java.is_static() { "static_" } else { "" }, StrEmitter(self.java.name()), StrEmitter(MethodSigWriter(self.java.descriptor())) diff --git a/java-spaghetti-gen/src/emit_rust/structs.rs b/java-spaghetti-gen/src/emit_rust/structs.rs index 37d0e11..f7542cf 100644 --- a/java-spaghetti-gen/src/emit_rust/structs.rs +++ b/java-spaghetti-gen/src/emit_rust/structs.rs @@ -119,15 +119,22 @@ impl Struct { let rust_name = &self.rust.struct_name; writeln!(out, "{attributes}{visibility} enum {rust_name}{{}}")?; if !self.java.is_static() { - writeln!(out, "unsafe impl ::java_spaghetti::ReferenceType for {rust_name} {{}}")?; + writeln!( + out, + "unsafe impl ::java_spaghetti::ReferenceType for {rust_name} {{\ + \n fn jni_get_class(__jni_env: ::java_spaghetti::Env) -> &'static ::java_spaghetti::JClass {{\ + \n Self::__class_global_ref(__jni_env)\ + \n }}\ + \n}}", + )?; } writeln!( out, "unsafe impl ::java_spaghetti::JniType for {rust_name} {{\ - \n fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R {{\ - \n callback({})\ - \n }}\ - \n}}", + \n fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R {{\ + \n callback({})\ + \n }}\ + \n}}", StrEmitter(self.java.path().as_str()), )?; @@ -156,15 +163,10 @@ impl Struct { writeln!( out, "\ - \nfn __class_global_ref(__jni_env: ::java_spaghetti::Env) -> ::java_spaghetti::sys::jobject {{\ - \n static __CLASS: ::std::sync::OnceLock<::java_spaghetti::Global<{}>> = ::std::sync::OnceLock::new();\ - \n __CLASS.get_or_init(|| unsafe {{\ - \n ::java_spaghetti::Local::from_raw(__jni_env, __jni_env.require_class({})).as_global()\ - \n }}).as_raw()\ + \nfn __class_global_ref(__jni_env: ::java_spaghetti::Env) -> &'static ::java_spaghetti::JClass {{\ + \n static _CLASS: ::std::sync::OnceLock<::java_spaghetti::JClass> = ::std::sync::OnceLock::new();\ + \n _CLASS.get_or_init(|| unsafe {{ __jni_env.require_class({}) }})\ \n}}", - context - .java_to_rust_path(Id("java/lang/Object"), &self.rust.mod_) - .unwrap(), StrEmitter(self.java.path().as_str()), )?; diff --git a/java-spaghetti/src/array.rs b/java-spaghetti/src/array.rs index 28975f0..02ad711 100644 --- a/java-spaghetti/src/array.rs +++ b/java-spaghetti/src/array.rs @@ -1,10 +1,11 @@ use std::marker::PhantomData; use std::ops::{Bound, RangeBounds}; use std::ptr::null_mut; +use std::sync::OnceLock; use jni_sys::*; -use crate::{AsArg, Env, JniType, Local, Ref, ReferenceType, ThrowableType}; +use crate::{AsArg, Env, JClass, JniType, Local, Ref, ReferenceType, ThrowableType}; /// A Java Array of some POD-like type such as bool, jbyte, jchar, jshort, jint, jlong, jfloat, or jdouble. /// @@ -43,6 +44,11 @@ where array } + /// Uses env.GetArrayLength to get the length of the java array, returns true if it is 0. + fn is_empty(self: &Ref<'_, Self>) -> bool { + self.len() == 0 + } + /// Uses env.GetArrayLength + env.Get{Type}ArrayRegion to read the contents of the java array from range into a new Vec. fn get_region_as_vec(self: &Ref<'_, Self>, range: impl RangeBounds) -> Vec { let len = self.len(); @@ -80,7 +86,12 @@ macro_rules! primitive_array { /// A [PrimitiveArray] implementation. pub enum $name {} - unsafe impl ReferenceType for $name {} + unsafe impl ReferenceType for $name { + fn jni_get_class(env: Env) -> &'static JClass { + static CLASS_CACHE: OnceLock = OnceLock::new(); + CLASS_CACHE.get_or_init(|| Self::static_with_jni_type(|t| unsafe { env.require_class(t) })) + } + } unsafe impl JniType for $name { fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { callback($type_str) @@ -89,7 +100,7 @@ macro_rules! primitive_array { impl PrimitiveArray<$type> for $name { fn new<'env>(env: Env<'env>, size: usize) -> Local<'env, Self> { - assert!(size <= std::i32::MAX as usize); // jsize == jint == i32 + assert!(size <= i32::MAX as usize); // jsize == jint == i32 let size = size as jsize; let jnienv = env.as_raw(); unsafe { @@ -116,8 +127,8 @@ macro_rules! primitive_array { } fn get_region(self: &Ref<'_, Self>, start: usize, elements: &mut [$type]) { - assert!(start <= std::i32::MAX as usize); // jsize == jint == i32 - assert!(elements.len() <= std::i32::MAX as usize); // jsize == jint == i32 + assert!(start <= i32::MAX as usize); // jsize == jint == i32 + assert!(elements.len() <= i32::MAX as usize); // jsize == jint == i32 let self_len = self.len() as jsize; let elements_len = elements.len() as jsize; @@ -139,8 +150,8 @@ macro_rules! primitive_array { } fn set_region(self: &Ref<'_, Self>, start: usize, elements: &[$type]) { - assert!(start <= std::i32::MAX as usize); // jsize == jint == i32 - assert!(elements.len() <= std::i32::MAX as usize); // jsize == jint == i32 + assert!(start <= i32::MAX as usize); // jsize == jint == i32 + assert!(elements.len() <= i32::MAX as usize); // jsize == jint == i32 let self_len = self.len() as jsize; let elements_len = elements.len() as jsize; @@ -178,7 +189,12 @@ primitive_array! { DoubleArray, "[D\0", jdouble { NewDoubleArray SetDoubleArra /// See also [PrimitiveArray] for arrays of reference types. pub struct ObjectArray(core::convert::Infallible, PhantomData<(T, E)>); -unsafe impl ReferenceType for ObjectArray {} +unsafe impl ReferenceType for ObjectArray { + fn jni_get_class(env: Env) -> &'static JClass { + static CLASS_CACHE: OnceLock = OnceLock::new(); + CLASS_CACHE.get_or_init(|| Self::static_with_jni_type(|t| unsafe { env.require_class(t) })) + } +} unsafe impl JniType for ObjectArray { fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { @@ -188,8 +204,8 @@ unsafe impl JniType for ObjectArray { impl ObjectArray { 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) }); + assert!(size <= i32::MAX as usize); // jsize == jint == i32 + let class = T::jni_get_class(env).as_raw(); let size = size as jsize; let object = unsafe { @@ -210,10 +226,7 @@ impl ObjectArray { } } - pub fn new_from<'env>( - env: Env<'env>, - elements: impl ExactSizeIterator + Iterator>, - ) -> Local<'env, Self> { + pub fn new_from<'env>(env: Env<'env>, elements: impl ExactSizeIterator>) -> Local<'env, Self> { let size = elements.len(); let array = Self::new(env, size); let env = array.env().as_raw(); @@ -229,9 +242,13 @@ impl ObjectArray { unsafe { ((**env).v1_2.GetArrayLength)(env, self.as_raw()) as usize } } + pub fn is_empty(self: &Ref<'_, Self>) -> bool { + self.len() == 0 + } + /// 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? + assert!(index <= i32::MAX as usize); // jsize == jint == i32 XXX: Should maybe be treated as an exception? let index = index as jsize; let env = self.env(); let result = unsafe { @@ -248,7 +265,7 @@ impl ObjectArray { /// 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? + assert!(index <= i32::MAX as usize); // jsize == jint == i32 XXX: Should maybe be treated as an exception? let index = index as jsize; let env = self.env(); unsafe { diff --git a/java-spaghetti/src/env.rs b/java-spaghetti/src/env.rs index d06301a..a5f9970 100644 --- a/java-spaghetti/src/env.rs +++ b/java-spaghetti/src/env.rs @@ -6,7 +6,7 @@ use std::sync::OnceLock; use jni_sys::*; -use crate::{AsArg, Local, Ref, ReferenceType, StringChars, ThrowableType, VM}; +use crate::{AsArg, JClass, JFieldID, JMethodID, Local, Ref, ReferenceType, StringChars, ThrowableType, VM}; /// FFI: Use **Env** instead of \*const JNIEnv. This represents a per-thread Java exection environment. /// @@ -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 @@ -39,15 +41,20 @@ 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; +/// 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 /// } @@ -148,17 +155,15 @@ impl<'env> Env<'env> { } unsafe fn exception_to_string(self, exception: jobject) -> String { - static METHOD_GET_MESSAGE: OnceLock = OnceLock::new(); + static METHOD_GET_MESSAGE: OnceLock = OnceLock::new(); let throwable_get_message = *METHOD_GET_MESSAGE.get_or_init(|| { // use JNI FindClass to avoid infinte recursion. - let throwable_class = self.require_class_jni("java/lang/Throwable\0"); - let method = self.require_method(throwable_class, "getMessage\0", "()Ljava/lang/String;\0"); - ((**self.env).v1_2.DeleteLocalRef)(self.env, throwable_class); - method.addr() - }) as jmethodID; // it is a global ID + let throwable_class = self.require_class_jni("java/lang/Throwable\0").unwrap(); + self.require_method(&throwable_class, "getMessage\0", "()Ljava/lang/String;\0") + }); let message = - ((**self.env).v1_2.CallObjectMethodA)(self.env, exception, throwable_get_message, ptr::null_mut()); + ((**self.env).v1_2.CallObjectMethodA)(self.env, exception, throwable_get_message.as_raw(), ptr::null_mut()); let e2: *mut _jobject = ((**self.env).v1_2.ExceptionOccurred)(self.env); if !e2.is_null() { ((**self.env).v1_2.ExceptionClear)(self.env); @@ -168,17 +173,10 @@ impl<'env> Env<'env> { StringChars::from_env_jstring(self, message).to_string_lossy() } - /// Note: the returned `jclass` is actually a new local reference of the class object. - pub unsafe fn require_class(self, class: &str) -> jclass { + pub unsafe fn require_class(self, class: &str) -> JClass { // First try with JNI FindClass. - debug_assert!(class.ends_with('\0')); - let c = ((**self.env).v1_2.FindClass)(self.env, class.as_ptr() as *const c_char); - let exception: *mut _jobject = ((**self.env).v1_2.ExceptionOccurred)(self.env); - if !exception.is_null() { - ((**self.env).v1_2.ExceptionClear)(self.env); - } - if !c.is_null() { - return c; + if let Some(class) = self.require_class_jni(class) { + return class; } // If class is not found and we have a classloader set, try that. @@ -191,19 +189,17 @@ impl<'env> Env<'env> { .collect::>(); let string = unsafe { self.new_string(chars.as_ptr(), chars.len() as jsize) }; - static CL_METHOD: OnceLock = OnceLock::new(); - let cl_method = *CL_METHOD.get_or_init(|| { + static CL_METHOD: OnceLock = OnceLock::new(); + let cl_method = CL_METHOD.get_or_init(|| { // We still use JNI FindClass for this, to avoid a chicken-and-egg situation. // If the system class loader cannot find java.lang.ClassLoader, things are pretty broken! - let cl_class = self.require_class_jni("java/lang/ClassLoader\0"); - let cl_method = self.require_method(cl_class, "loadClass\0", "(Ljava/lang/String;)Ljava/lang/Class;\0"); - ((**self.env).v1_2.DeleteLocalRef)(self.env, cl_class); - cl_method.addr() - }) as jmethodID; // it is a global ID + let cl_class = self.require_class_jni("java/lang/ClassLoader\0").unwrap(); + self.require_method(&cl_class, "loadClass\0", "(Ljava/lang/String;)Ljava/lang/Class;\0") + }); let args = [jvalue { l: string }]; let result: *mut _jobject = - ((**self.env).v1_2.CallObjectMethodA)(self.env, classloader, cl_method, args.as_ptr()); + ((**self.env).v1_2.CallObjectMethodA)(self.env, classloader, cl_method.as_raw(), args.as_ptr()); let exception: *mut _jobject = ((**self.env).v1_2.ExceptionOccurred)(self.env); if !exception.is_null() { ((**self.env).v1_2.ExceptionClear)(self.env); @@ -217,113 +213,88 @@ impl<'env> Env<'env> { ((**self.env).v1_2.DeleteLocalRef)(self.env, string); - return result as jclass; + return JClass::from_raw(self, result); } // If neither found the class, panic. panic!("couldn't load class {}", class); } - unsafe fn require_class_jni(self, class: &str) -> jclass { + unsafe fn require_class_jni(self, class: &str) -> Option { debug_assert!(class.ends_with('\0')); - let class = ((**self.env).v1_2.FindClass)(self.env, class.as_ptr() as *const c_char); - assert!(!class.is_null()); - class + let cls = ((**self.env).v1_2.FindClass)(self.env, class.as_ptr() as *const c_char); + let exception: *mut _jobject = ((**self.env).v1_2.ExceptionOccurred)(self.env); + if !exception.is_null() { + ((**self.env).v1_2.ExceptionClear)(self.env); + return None; + } + if cls.is_null() { + return None; + } + Some(JClass::from_raw(self, cls)) } - pub unsafe fn require_method(self, class: jclass, method: &str, descriptor: &str) -> jmethodID { + pub unsafe fn require_method(self, class: &JClass, method: &str, descriptor: &str) -> JMethodID { debug_assert!(method.ends_with('\0')); debug_assert!(descriptor.ends_with('\0')); let method = ((**self.env).v1_2.GetMethodID)( self.env, - class, + class.as_raw(), method.as_ptr() as *const c_char, descriptor.as_ptr() as *const c_char, ); - assert!(!method.is_null()); - method + JMethodID::from_raw(method) } - pub unsafe fn require_static_method(self, class: jclass, method: &str, descriptor: &str) -> jmethodID { + pub unsafe fn require_static_method(self, class: &JClass, method: &str, descriptor: &str) -> JMethodID { debug_assert!(method.ends_with('\0')); debug_assert!(descriptor.ends_with('\0')); let method = ((**self.env).v1_2.GetStaticMethodID)( self.env, - class, + class.as_raw(), method.as_ptr() as *const c_char, descriptor.as_ptr() as *const c_char, ); - assert!(!method.is_null()); - method + JMethodID::from_raw(method) } - pub unsafe fn require_field(self, class: jclass, field: &str, descriptor: &str) -> jfieldID { + pub unsafe fn require_field(self, class: &JClass, field: &str, descriptor: &str) -> JFieldID { debug_assert!(field.ends_with('\0')); debug_assert!(field.ends_with('\0')); let field = ((**self.env).v1_2.GetFieldID)( self.env, - class, + class.as_raw(), field.as_ptr() as *const c_char, descriptor.as_ptr() as *const c_char, ); - assert!(!field.is_null()); - field + JFieldID::from_raw(field) } - pub unsafe fn require_static_field(self, class: jclass, field: &str, descriptor: &str) -> jfieldID { + pub unsafe fn require_static_field(self, class: &JClass, field: &str, descriptor: &str) -> JFieldID { debug_assert!(field.ends_with('\0')); debug_assert!(field.ends_with('\0')); let field = ((**self.env).v1_2.GetStaticFieldID)( self.env, - class, + class.as_raw(), field.as_ptr() as *const c_char, descriptor.as_ptr() as *const c_char, ); - assert!(!field.is_null()); - field - } - - // Multi-Query Methods - // XXX: Remove these unused functions. - - pub unsafe fn require_class_method(self, class: &str, method: &str, descriptor: &str) -> (jclass, jmethodID) { - let class = self.require_class(class); - (class, self.require_method(class, method, descriptor)) - } - - pub unsafe fn require_class_static_method( - self, - class: &str, - method: &str, - descriptor: &str, - ) -> (jclass, jmethodID) { - let class = self.require_class(class); - (class, self.require_static_method(class, method, descriptor)) - } - - pub unsafe fn require_class_field(self, class: &str, method: &str, descriptor: &str) -> (jclass, jfieldID) { - let class = self.require_class(class); - (class, self.require_field(class, method, descriptor)) - } - - pub unsafe fn require_class_static_field(self, class: &str, method: &str, descriptor: &str) -> (jclass, jfieldID) { - let class = self.require_class(class); - (class, self.require_static_field(class, method, descriptor)) + JFieldID::from_raw(field) } // Constructor Methods pub unsafe fn new_object_a( self, - class: jclass, - method: jmethodID, + class: &JClass, + method: JMethodID, args: *const jvalue, ) -> Result, Local<'env, E>> { - let result = ((**self.env).v1_2.NewObjectA)(self.env, class, method, args); + let result = ((**self.env).v1_2.NewObjectA)(self.env, class.as_raw(), method.as_raw(), args); self.exception_check()?; assert!(!result.is_null()); Ok(Local::from_raw(self, result)) @@ -334,10 +305,10 @@ impl<'env> Env<'env> { pub unsafe fn call_object_method_a( self, this: jobject, - method: jmethodID, + method: JMethodID, args: *const jvalue, ) -> Result>, Local<'env, E>> { - let result = ((**self.env).v1_2.CallObjectMethodA)(self.env, this, method, args); + let result = ((**self.env).v1_2.CallObjectMethodA)(self.env, this, method.as_raw(), args); self.exception_check()?; if result.is_null() { Ok(None) @@ -349,10 +320,10 @@ impl<'env> Env<'env> { pub unsafe fn call_boolean_method_a( self, this: jobject, - method: jmethodID, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallBooleanMethodA)(self.env, this, method, args); + let result = ((**self.env).v1_2.CallBooleanMethodA)(self.env, this, method.as_raw(), args); self.exception_check()?; Ok(result != JNI_FALSE) } @@ -360,10 +331,10 @@ impl<'env> Env<'env> { pub unsafe fn call_byte_method_a( self, this: jobject, - method: jmethodID, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallByteMethodA)(self.env, this, method, args); + let result = ((**self.env).v1_2.CallByteMethodA)(self.env, this, method.as_raw(), args); self.exception_check()?; Ok(result) } @@ -371,10 +342,10 @@ impl<'env> Env<'env> { pub unsafe fn call_char_method_a( self, this: jobject, - method: jmethodID, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallCharMethodA)(self.env, this, method, args); + let result = ((**self.env).v1_2.CallCharMethodA)(self.env, this, method.as_raw(), args); self.exception_check()?; Ok(result) } @@ -382,10 +353,10 @@ impl<'env> Env<'env> { pub unsafe fn call_short_method_a( self, this: jobject, - method: jmethodID, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallShortMethodA)(self.env, this, method, args); + let result = ((**self.env).v1_2.CallShortMethodA)(self.env, this, method.as_raw(), args); self.exception_check()?; Ok(result) } @@ -393,10 +364,10 @@ impl<'env> Env<'env> { pub unsafe fn call_int_method_a( self, this: jobject, - method: jmethodID, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallIntMethodA)(self.env, this, method, args); + let result = ((**self.env).v1_2.CallIntMethodA)(self.env, this, method.as_raw(), args); self.exception_check()?; Ok(result) } @@ -404,10 +375,10 @@ impl<'env> Env<'env> { pub unsafe fn call_long_method_a( self, this: jobject, - method: jmethodID, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallLongMethodA)(self.env, this, method, args); + let result = ((**self.env).v1_2.CallLongMethodA)(self.env, this, method.as_raw(), args); self.exception_check()?; Ok(result) } @@ -415,10 +386,10 @@ impl<'env> Env<'env> { pub unsafe fn call_float_method_a( self, this: jobject, - method: jmethodID, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallFloatMethodA)(self.env, this, method, args); + let result = ((**self.env).v1_2.CallFloatMethodA)(self.env, this, method.as_raw(), args); self.exception_check()?; Ok(result) } @@ -426,10 +397,10 @@ impl<'env> Env<'env> { pub unsafe fn call_double_method_a( self, this: jobject, - method: jmethodID, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallDoubleMethodA)(self.env, this, method, args); + let result = ((**self.env).v1_2.CallDoubleMethodA)(self.env, this, method.as_raw(), args); self.exception_check()?; Ok(result) } @@ -437,10 +408,10 @@ impl<'env> Env<'env> { pub unsafe fn call_void_method_a( self, this: jobject, - method: jmethodID, + method: JMethodID, args: *const jvalue, ) -> Result<(), Local<'env, E>> { - ((**self.env).v1_2.CallVoidMethodA)(self.env, this, method, args); + ((**self.env).v1_2.CallVoidMethodA)(self.env, this, method.as_raw(), args); self.exception_check() } @@ -448,11 +419,11 @@ impl<'env> Env<'env> { pub unsafe fn call_static_object_method_a( self, - class: jclass, - method: jmethodID, + class: &JClass, + method: JMethodID, args: *const jvalue, ) -> Result>, Local<'env, E>> { - let result = ((**self.env).v1_2.CallStaticObjectMethodA)(self.env, class, method, args); + let result = ((**self.env).v1_2.CallStaticObjectMethodA)(self.env, class.as_raw(), method.as_raw(), args); self.exception_check()?; if result.is_null() { Ok(None) @@ -463,106 +434,106 @@ impl<'env> Env<'env> { pub unsafe fn call_static_boolean_method_a( self, - class: jclass, - method: jmethodID, + class: &JClass, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallStaticBooleanMethodA)(self.env, class, method, args); + let result = ((**self.env).v1_2.CallStaticBooleanMethodA)(self.env, class.as_raw(), method.as_raw(), args); self.exception_check()?; Ok(result != JNI_FALSE) } pub unsafe fn call_static_byte_method_a( self, - class: jclass, - method: jmethodID, + class: &JClass, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallStaticByteMethodA)(self.env, class, method, args); + let result = ((**self.env).v1_2.CallStaticByteMethodA)(self.env, class.as_raw(), method.as_raw(), args); self.exception_check()?; Ok(result) } pub unsafe fn call_static_char_method_a( self, - class: jclass, - method: jmethodID, + class: &JClass, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallStaticCharMethodA)(self.env, class, method, args); + let result = ((**self.env).v1_2.CallStaticCharMethodA)(self.env, class.as_raw(), method.as_raw(), args); self.exception_check()?; Ok(result) } pub unsafe fn call_static_short_method_a( self, - class: jclass, - method: jmethodID, + class: &JClass, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallStaticShortMethodA)(self.env, class, method, args); + let result = ((**self.env).v1_2.CallStaticShortMethodA)(self.env, class.as_raw(), method.as_raw(), args); self.exception_check()?; Ok(result) } pub unsafe fn call_static_int_method_a( self, - class: jclass, - method: jmethodID, + class: &JClass, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallStaticIntMethodA)(self.env, class, method, args); + let result = ((**self.env).v1_2.CallStaticIntMethodA)(self.env, class.as_raw(), method.as_raw(), args); self.exception_check()?; Ok(result) } pub unsafe fn call_static_long_method_a( self, - class: jclass, - method: jmethodID, + class: &JClass, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallStaticLongMethodA)(self.env, class, method, args); + let result = ((**self.env).v1_2.CallStaticLongMethodA)(self.env, class.as_raw(), method.as_raw(), args); self.exception_check()?; Ok(result) } pub unsafe fn call_static_float_method_a( self, - class: jclass, - method: jmethodID, + class: &JClass, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallStaticFloatMethodA)(self.env, class, method, args); + let result = ((**self.env).v1_2.CallStaticFloatMethodA)(self.env, class.as_raw(), method.as_raw(), args); self.exception_check()?; Ok(result) } pub unsafe fn call_static_double_method_a( self, - class: jclass, - method: jmethodID, + class: &JClass, + method: JMethodID, args: *const jvalue, ) -> Result> { - let result = ((**self.env).v1_2.CallStaticDoubleMethodA)(self.env, class, method, args); + let result = ((**self.env).v1_2.CallStaticDoubleMethodA)(self.env, class.as_raw(), method.as_raw(), args); self.exception_check()?; Ok(result) } pub unsafe fn call_static_void_method_a( self, - class: jclass, - method: jmethodID, + class: &JClass, + method: JMethodID, args: *const jvalue, ) -> Result<(), Local<'env, E>> { - ((**self.env).v1_2.CallStaticVoidMethodA)(self.env, class, method, args); + ((**self.env).v1_2.CallStaticVoidMethodA)(self.env, class.as_raw(), method.as_raw(), args); self.exception_check() } // Instance Fields - pub unsafe fn get_object_field(self, this: jobject, field: jfieldID) -> Option> { - let result = ((**self.env).v1_2.GetObjectField)(self.env, this, field); + pub unsafe fn get_object_field(self, this: jobject, field: JFieldID) -> Option> { + let result = ((**self.env).v1_2.GetObjectField)(self.env, this, field.as_raw()); if result.is_null() { None } else { @@ -570,83 +541,83 @@ impl<'env> Env<'env> { } } - pub unsafe fn get_boolean_field(self, this: jobject, field: jfieldID) -> bool { - let result = ((**self.env).v1_2.GetBooleanField)(self.env, this, field); + pub unsafe fn get_boolean_field(self, this: jobject, field: JFieldID) -> bool { + let result = ((**self.env).v1_2.GetBooleanField)(self.env, this, field.as_raw()); result != JNI_FALSE } - pub unsafe fn get_byte_field(self, this: jobject, field: jfieldID) -> jbyte { - ((**self.env).v1_2.GetByteField)(self.env, this, field) + pub unsafe fn get_byte_field(self, this: jobject, field: JFieldID) -> jbyte { + ((**self.env).v1_2.GetByteField)(self.env, this, field.as_raw()) } - pub unsafe fn get_char_field(self, this: jobject, field: jfieldID) -> jchar { - ((**self.env).v1_2.GetCharField)(self.env, this, field) + pub unsafe fn get_char_field(self, this: jobject, field: JFieldID) -> jchar { + ((**self.env).v1_2.GetCharField)(self.env, this, field.as_raw()) } - pub unsafe fn get_short_field(self, this: jobject, field: jfieldID) -> jshort { - ((**self.env).v1_2.GetShortField)(self.env, this, field) + pub unsafe fn get_short_field(self, this: jobject, field: JFieldID) -> jshort { + ((**self.env).v1_2.GetShortField)(self.env, this, field.as_raw()) } - pub unsafe fn get_int_field(self, this: jobject, field: jfieldID) -> jint { - ((**self.env).v1_2.GetIntField)(self.env, this, field) + pub unsafe fn get_int_field(self, this: jobject, field: JFieldID) -> jint { + ((**self.env).v1_2.GetIntField)(self.env, this, field.as_raw()) } - pub unsafe fn get_long_field(self, this: jobject, field: jfieldID) -> jlong { - ((**self.env).v1_2.GetLongField)(self.env, this, field) + pub unsafe fn get_long_field(self, this: jobject, field: JFieldID) -> jlong { + ((**self.env).v1_2.GetLongField)(self.env, this, field.as_raw()) } - pub unsafe fn get_float_field(self, this: jobject, field: jfieldID) -> jfloat { - ((**self.env).v1_2.GetFloatField)(self.env, this, field) + pub unsafe fn get_float_field(self, this: jobject, field: JFieldID) -> jfloat { + ((**self.env).v1_2.GetFloatField)(self.env, this, field.as_raw()) } - pub unsafe fn get_double_field(self, this: jobject, field: jfieldID) -> jdouble { - ((**self.env).v1_2.GetDoubleField)(self.env, this, field) + pub unsafe fn get_double_field(self, this: jobject, field: JFieldID) -> jdouble { + ((**self.env).v1_2.GetDoubleField)(self.env, this, field.as_raw()) } - pub unsafe fn set_object_field(self, this: jobject, field: jfieldID, value: impl AsArg) { - ((**self.env).v1_2.SetObjectField)(self.env, this, field, value.as_arg()); + pub unsafe fn set_object_field(self, this: jobject, field: JFieldID, value: impl AsArg) { + ((**self.env).v1_2.SetObjectField)(self.env, this, field.as_raw(), value.as_arg()); } - pub unsafe fn set_boolean_field(self, this: jobject, field: jfieldID, value: bool) { - ((**self.env).v1_2.SetBooleanField)(self.env, this, field, if value { JNI_TRUE } else { JNI_FALSE }); + pub unsafe fn set_boolean_field(self, this: jobject, field: JFieldID, value: bool) { + ((**self.env).v1_2.SetBooleanField)(self.env, this, field.as_raw(), if value { JNI_TRUE } else { JNI_FALSE }); } - pub unsafe fn set_byte_field(self, this: jobject, field: jfieldID, value: jbyte) { - ((**self.env).v1_2.SetByteField)(self.env, this, field, value); + pub unsafe fn set_byte_field(self, this: jobject, field: JFieldID, value: jbyte) { + ((**self.env).v1_2.SetByteField)(self.env, this, field.as_raw(), value); } - pub unsafe fn set_char_field(self, this: jobject, field: jfieldID, value: jchar) { - ((**self.env).v1_2.SetCharField)(self.env, this, field, value); + pub unsafe fn set_char_field(self, this: jobject, field: JFieldID, value: jchar) { + ((**self.env).v1_2.SetCharField)(self.env, this, field.as_raw(), value); } - pub unsafe fn set_short_field(self, this: jobject, field: jfieldID, value: jshort) { - ((**self.env).v1_2.SetShortField)(self.env, this, field, value); + pub unsafe fn set_short_field(self, this: jobject, field: JFieldID, value: jshort) { + ((**self.env).v1_2.SetShortField)(self.env, this, field.as_raw(), value); } - pub unsafe fn set_int_field(self, this: jobject, field: jfieldID, value: jint) { - ((**self.env).v1_2.SetIntField)(self.env, this, field, value); + pub unsafe fn set_int_field(self, this: jobject, field: JFieldID, value: jint) { + ((**self.env).v1_2.SetIntField)(self.env, this, field.as_raw(), value); } - pub unsafe fn set_long_field(self, this: jobject, field: jfieldID, value: jlong) { - ((**self.env).v1_2.SetLongField)(self.env, this, field, value); + pub unsafe fn set_long_field(self, this: jobject, field: JFieldID, value: jlong) { + ((**self.env).v1_2.SetLongField)(self.env, this, field.as_raw(), value); } - pub unsafe fn set_float_field(self, this: jobject, field: jfieldID, value: jfloat) { - ((**self.env).v1_2.SetFloatField)(self.env, this, field, value); + pub unsafe fn set_float_field(self, this: jobject, field: JFieldID, value: jfloat) { + ((**self.env).v1_2.SetFloatField)(self.env, this, field.as_raw(), value); } - pub unsafe fn set_double_field(self, this: jobject, field: jfieldID, value: jdouble) { - ((**self.env).v1_2.SetDoubleField)(self.env, this, field, value); + pub unsafe fn set_double_field(self, this: jobject, field: JFieldID, value: jdouble) { + ((**self.env).v1_2.SetDoubleField)(self.env, this, field.as_raw(), value); } // Static Fields pub unsafe fn get_static_object_field( self, - class: jclass, - field: jfieldID, + class: &JClass, + field: JFieldID, ) -> Option> { - let result = ((**self.env).v1_2.GetStaticObjectField)(self.env, class, field); + let result = ((**self.env).v1_2.GetStaticObjectField)(self.env, class.as_raw(), field.as_raw()); if result.is_null() { None } else { @@ -654,78 +625,83 @@ impl<'env> Env<'env> { } } - pub unsafe fn get_static_boolean_field(self, class: jclass, field: jfieldID) -> bool { - let result = ((**self.env).v1_2.GetStaticBooleanField)(self.env, class, field); + pub unsafe fn get_static_boolean_field(self, class: &JClass, field: JFieldID) -> bool { + let result = ((**self.env).v1_2.GetStaticBooleanField)(self.env, class.as_raw(), field.as_raw()); result != JNI_FALSE } - pub unsafe fn get_static_byte_field(self, class: jclass, field: jfieldID) -> jbyte { - ((**self.env).v1_2.GetStaticByteField)(self.env, class, field) + pub unsafe fn get_static_byte_field(self, class: &JClass, field: JFieldID) -> jbyte { + ((**self.env).v1_2.GetStaticByteField)(self.env, class.as_raw(), field.as_raw()) } - pub unsafe fn get_static_char_field(self, class: jclass, field: jfieldID) -> jchar { - ((**self.env).v1_2.GetStaticCharField)(self.env, class, field) + pub unsafe fn get_static_char_field(self, class: &JClass, field: JFieldID) -> jchar { + ((**self.env).v1_2.GetStaticCharField)(self.env, class.as_raw(), field.as_raw()) } - pub unsafe fn get_static_short_field(self, class: jclass, field: jfieldID) -> jshort { - ((**self.env).v1_2.GetStaticShortField)(self.env, class, field) + pub unsafe fn get_static_short_field(self, class: &JClass, field: JFieldID) -> jshort { + ((**self.env).v1_2.GetStaticShortField)(self.env, class.as_raw(), field.as_raw()) } - pub unsafe fn get_static_int_field(self, class: jclass, field: jfieldID) -> jint { - ((**self.env).v1_2.GetStaticIntField)(self.env, class, field) + pub unsafe fn get_static_int_field(self, class: &JClass, field: JFieldID) -> jint { + ((**self.env).v1_2.GetStaticIntField)(self.env, class.as_raw(), field.as_raw()) } - pub unsafe fn get_static_long_field(self, class: jclass, field: jfieldID) -> jlong { - ((**self.env).v1_2.GetStaticLongField)(self.env, class, field) + pub unsafe fn get_static_long_field(self, class: &JClass, field: JFieldID) -> jlong { + ((**self.env).v1_2.GetStaticLongField)(self.env, class.as_raw(), field.as_raw()) } - pub unsafe fn get_static_float_field(self, class: jclass, field: jfieldID) -> jfloat { - ((**self.env).v1_2.GetStaticFloatField)(self.env, class, field) + pub unsafe fn get_static_float_field(self, class: &JClass, field: JFieldID) -> jfloat { + ((**self.env).v1_2.GetStaticFloatField)(self.env, class.as_raw(), field.as_raw()) } - pub unsafe fn get_static_double_field(self, class: jclass, field: jfieldID) -> jdouble { - ((**self.env).v1_2.GetStaticDoubleField)(self.env, class, field) + pub unsafe fn get_static_double_field(self, class: &JClass, field: JFieldID) -> jdouble { + ((**self.env).v1_2.GetStaticDoubleField)(self.env, class.as_raw(), field.as_raw()) } pub unsafe fn set_static_object_field( self, - class: jclass, - field: jfieldID, + class: &JClass, + field: JFieldID, value: impl AsArg, ) { - ((**self.env).v1_2.SetStaticObjectField)(self.env, class, field, value.as_arg()); + ((**self.env).v1_2.SetStaticObjectField)(self.env, class.as_raw(), field.as_raw(), value.as_arg()); } - pub unsafe fn set_static_boolean_field(self, class: jclass, field: jfieldID, value: bool) { - ((**self.env).v1_2.SetStaticBooleanField)(self.env, class, field, if value { JNI_TRUE } else { JNI_FALSE }); + pub unsafe fn set_static_boolean_field(self, class: &JClass, field: JFieldID, value: bool) { + ((**self.env).v1_2.SetStaticBooleanField)( + self.env, + class.as_raw(), + field.as_raw(), + if value { JNI_TRUE } else { JNI_FALSE }, + ); } - pub unsafe fn set_static_byte_field(self, class: jclass, field: jfieldID, value: jbyte) { - ((**self.env).v1_2.SetStaticByteField)(self.env, class, field, value); + pub unsafe fn set_static_byte_field(self, class: &JClass, field: JFieldID, value: jbyte) { + ((**self.env).v1_2.SetStaticByteField)(self.env, class.as_raw(), field.as_raw(), value); } - pub unsafe fn set_static_char_field(self, class: jclass, field: jfieldID, value: jchar) { - ((**self.env).v1_2.SetStaticCharField)(self.env, class, field, value); + pub unsafe fn set_static_char_field(self, class: &JClass, field: JFieldID, value: jchar) { + ((**self.env).v1_2.SetStaticCharField)(self.env, class.as_raw(), field.as_raw(), value); } - pub unsafe fn set_static_short_field(self, class: jclass, field: jfieldID, value: jshort) { - ((**self.env).v1_2.SetStaticShortField)(self.env, class, field, value); + pub unsafe fn set_static_short_field(self, class: &JClass, field: JFieldID, value: jshort) { + ((**self.env).v1_2.SetStaticShortField)(self.env, class.as_raw(), field.as_raw(), value); } - pub unsafe fn set_static_int_field(self, class: jclass, field: jfieldID, value: jint) { - ((**self.env).v1_2.SetStaticIntField)(self.env, class, field, value); + pub unsafe fn set_static_int_field(self, class: &JClass, field: JFieldID, value: jint) { + ((**self.env).v1_2.SetStaticIntField)(self.env, class.as_raw(), field.as_raw(), value); } - pub unsafe fn set_static_long_field(self, class: jclass, field: jfieldID, value: jlong) { - ((**self.env).v1_2.SetStaticLongField)(self.env, class, field, value); + pub unsafe fn set_static_long_field(self, class: &JClass, field: JFieldID, value: jlong) { + ((**self.env).v1_2.SetStaticLongField)(self.env, class.as_raw(), field.as_raw(), value); } - pub unsafe fn set_static_float_field(self, class: jclass, field: jfieldID, value: jfloat) { - ((**self.env).v1_2.SetStaticFloatField)(self.env, class, field, value); + pub unsafe fn set_static_float_field(self, class: &JClass, field: JFieldID, value: jfloat) { + ((**self.env).v1_2.SetStaticFloatField)(self.env, class.as_raw(), field.as_raw(), value); } - pub unsafe fn set_static_double_field(self, class: jclass, field: jfieldID, value: jdouble) { - ((**self.env).v1_2.SetStaticDoubleField)(self.env, class, field, value); + pub unsafe fn set_static_double_field(self, class: &JClass, field: JFieldID, value: jdouble) { + ((**self.env).v1_2.SetStaticDoubleField)(self.env, class.as_raw(), field.as_raw(), value); } pub fn throw(self, throwable: Ref) { diff --git a/java-spaghetti/src/id_cache.rs b/java-spaghetti/src/id_cache.rs index 8253f23..dfe2687 100644 --- a/java-spaghetti/src/id_cache.rs +++ b/java-spaghetti/src/id_cache.rs @@ -1,16 +1,83 @@ -//! New types for `jfieldID` and `jmethodID` that implement `Send` and `Sync`. +//! New type for cached class objects as JNI global references; new types for `jfieldID` and `jmethodID` that +//! implement `Send` and `Sync`. //! //! Inspired by: . -//! -//! According to the JNI spec field IDs may be invalidated when the corresponding class is unloaded: -//! -//! -//! You should generally not be interacting with these types directly, but it must be public for codegen. -use crate::sys::{jfieldID, jmethodID}; +use crate::sys::{jclass, jfieldID, jmethodID, jobject}; +use crate::{Env, VM}; + +/// New type for cached class objects as JNI global references. +/// +/// Holding a `JClass` global reference prevents the corresponding Java class from being unloaded. +pub struct JClass { + class: jclass, + vm: VM, +} + +unsafe impl Send for JClass {} +unsafe impl Sync for JClass {} + +impl JClass { + /// Creates a `JClass` from a raw JNI local reference of a class object. + /// + /// # Safety + /// + /// `class` must be a valid JNI local reference to a `java.lang.Class` object. + /// It is safe to pass the returned value of JNI `FindClass` to it if no exeception occurred. + pub unsafe fn from_raw<'env>(env: Env<'env>, class: jclass) -> Self { + assert!(!class.is_null(), "from_raw jclass argument"); + let jnienv = env.as_raw(); + let class_global = unsafe { ((**jnienv).v1_2.NewGlobalRef)(jnienv, class) }; + Self::from_raw_global(env.vm(), class_global) + } + + /// Creates a `JClass` from a raw JNI global reference of a class object. + /// + /// # Safety + /// + /// `class` must be a valid JNI global reference to a `java.lang.Class` object. + pub unsafe fn from_raw_global(vm: VM, class: jobject) -> Self { + assert!(!class.is_null(), "from_raw_global jclass argument"); + Self { + class: class as jclass, + vm, + } + } + + pub fn as_raw(&self) -> jclass { + self.class + } +} + +impl Clone for JClass { + fn clone(&self) -> Self { + self.vm.with_env(|env| { + let env = env.as_raw(); + let class_global = unsafe { ((**env).v1_2.NewGlobalRef)(env, self.class) }; + assert!(!class_global.is_null()); + unsafe { Self::from_raw_global(self.vm, class_global) } + }) + } +} + +// XXX: Unfortunately, static items (e.g. `OnceLock`) do not call drop at the end of the Rust program: +// JNI global references may be leaked if `java-spaghetti`-based libraries are unloaded and reloaded by the VM. +// Store them in thread local keys may resolve this potential risk. +impl Drop for JClass { + fn drop(&mut self) { + self.vm.with_env(|env| { + let env = env.as_raw(); + unsafe { ((**env).v1_2.DeleteGlobalRef)(env, self.class) } + }); + } +} -#[doc(hidden)] +/// New type for `jfieldID`, implements `Send` and `Sync`. +/// +/// According to the JNI spec, field IDs may be invalidated when the corresponding class is unloaded: +/// . #[repr(transparent)] +#[derive(Clone, Copy)] pub struct JFieldID { internal: jfieldID, } @@ -26,7 +93,7 @@ impl JFieldID { /// /// Expects a valid, non-`null` ID. pub unsafe fn from_raw(raw: jfieldID) -> Self { - debug_assert!(!raw.is_null(), "from_raw fieldID argument"); + assert!(!raw.is_null(), "from_raw fieldID argument"); Self { internal: raw } } @@ -35,8 +102,12 @@ impl JFieldID { } } -#[doc(hidden)] +/// New type for `jmethodID`, implements `Send` and `Sync`. +/// +/// According to the JNI spec, method IDs may be invalidated when the corresponding class is unloaded: +/// . #[repr(transparent)] +#[derive(Clone, Copy)] pub struct JMethodID { internal: jmethodID, } @@ -52,7 +123,7 @@ impl JMethodID { /// /// Expects a valid, non-`null` ID. pub unsafe fn from_raw(raw: jmethodID) -> Self { - debug_assert!(!raw.is_null(), "from_raw methodID argument"); + assert!(!raw.is_null(), "from_raw methodID argument"); Self { internal: raw } } diff --git a/java-spaghetti/src/lib.rs b/java-spaghetti/src/lib.rs index 39ca593..cd9594f 100644 --- a/java-spaghetti/src/lib.rs +++ b/java-spaghetti/src/lib.rs @@ -59,7 +59,11 @@ pub trait ThrowableType: ReferenceType {} /// You should generally not be interacting with this type directly, but it must be public for codegen. #[doc(hidden)] -pub unsafe trait ReferenceType: JniType + Sized + 'static {} +pub unsafe trait ReferenceType: JniType + Sized + 'static { + // There couldn't be a default impl holding a static `OnceLock`: the compiler may not generate + // an independent static item for each generated struct that implements `ReferenceType`. + fn jni_get_class<'env>(env: Env<'env>) -> &'static JClass; +} /// Marker trait indicating `Self` can be assigned to `T`. /// diff --git a/java-spaghetti/src/refs/local.rs b/java-spaghetti/src/refs/local.rs index af5b56e..b615546 100644 --- a/java-spaghetti/src/refs/local.rs +++ b/java-spaghetti/src/refs/local.rs @@ -80,13 +80,9 @@ impl<'env, T: ReferenceType> Local<'env, T> { } pub fn cast(&self) -> Result, crate::CastError> { + self.as_ref().check_assignable::()?; 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) }) diff --git a/java-spaghetti/src/refs/ref_.rs b/java-spaghetti/src/refs/ref_.rs index 0e5448a..1b1a537 100644 --- a/java-spaghetti/src/refs/ref_.rs +++ b/java-spaghetti/src/refs/ref_.rs @@ -57,12 +57,11 @@ impl<'env, T: ReferenceType> Ref<'env, T> { unsafe { Local::from_raw(self.env(), object) } } - fn check_assignable(&self) -> Result<(), crate::CastError> { + 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::jni_get_class(env).as_raw(); + if !unsafe { ((**jnienv).v1_2.IsInstanceOf)(jnienv, self.as_raw(), class) } { return Err(crate::CastError); } Ok(()) From b67931380f567b139e22e293b0c75ab1f6825f2d Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Mon, 24 Mar 2025 09:33:54 +0800 Subject: [PATCH 02/10] Fix `Env` comment for cargo test --- java-spaghetti/src/env.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java-spaghetti/src/env.rs b/java-spaghetti/src/env.rs index a5f9970..d5cc149 100644 --- a/java-spaghetti/src/env.rs +++ b/java-spaghetti/src/env.rs @@ -40,7 +40,7 @@ use crate::{AsArg, JClass, JFieldID, JMethodID, Local, Ref, ReferenceType, Strin /// /// ### main_activity.rs /// -/// ```rust +/// ```ignore /// use java_spaghetti::{Env, Arg}; /// use java_spaghetti::sys::{jboolean, JNI_TRUE}; /// use bindings::java::lang::Object; From d4d7a4f2bf50a1b957eff1f7382471fe0714ec41 Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Mon, 24 Mar 2025 11:48:10 +0800 Subject: [PATCH 03/10] Refactor `Env` API: use `&Ref` and `&[jvalue]` --- java-spaghetti/src/env.rs | 218 +++++++++++++++++++++----------------- 1 file changed, 120 insertions(+), 98 deletions(-) diff --git a/java-spaghetti/src/env.rs b/java-spaghetti/src/env.rs index d5cc149..dc10495 100644 --- a/java-spaghetti/src/env.rs +++ b/java-spaghetti/src/env.rs @@ -292,9 +292,9 @@ impl<'env> Env<'env> { self, class: &JClass, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result, Local<'env, E>> { - let result = ((**self.env).v1_2.NewObjectA)(self.env, class.as_raw(), method.as_raw(), args); + let result = ((**self.env).v1_2.NewObjectA)(self.env, class.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; assert!(!result.is_null()); Ok(Local::from_raw(self, result)) @@ -302,13 +302,13 @@ impl<'env> Env<'env> { // Instance Methods - pub unsafe fn call_object_method_a( + pub unsafe fn call_object_method_a( self, - this: jobject, + this: &Ref<'env, T>, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result>, Local<'env, E>> { - let result = ((**self.env).v1_2.CallObjectMethodA)(self.env, this, method.as_raw(), args); + let result = ((**self.env).v1_2.CallObjectMethodA)(self.env, this.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; if result.is_null() { Ok(None) @@ -317,101 +317,101 @@ impl<'env> Env<'env> { } } - pub unsafe fn call_boolean_method_a( + pub unsafe fn call_boolean_method_a( self, - this: jobject, + this: &Ref<'env, T>, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallBooleanMethodA)(self.env, this, method.as_raw(), args); + let result = ((**self.env).v1_2.CallBooleanMethodA)(self.env, this.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result != JNI_FALSE) } - pub unsafe fn call_byte_method_a( + pub unsafe fn call_byte_method_a( self, - this: jobject, + this: &Ref<'env, T>, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallByteMethodA)(self.env, this, method.as_raw(), args); + let result = ((**self.env).v1_2.CallByteMethodA)(self.env, this.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } - pub unsafe fn call_char_method_a( + pub unsafe fn call_char_method_a( self, - this: jobject, + this: &Ref<'env, T>, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallCharMethodA)(self.env, this, method.as_raw(), args); + let result = ((**self.env).v1_2.CallCharMethodA)(self.env, this.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } - pub unsafe fn call_short_method_a( + pub unsafe fn call_short_method_a( self, - this: jobject, + this: &Ref<'env, T>, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallShortMethodA)(self.env, this, method.as_raw(), args); + let result = ((**self.env).v1_2.CallShortMethodA)(self.env, this.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } - pub unsafe fn call_int_method_a( + pub unsafe fn call_int_method_a( self, - this: jobject, + this: &Ref<'env, T>, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallIntMethodA)(self.env, this, method.as_raw(), args); + let result = ((**self.env).v1_2.CallIntMethodA)(self.env, this.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } - pub unsafe fn call_long_method_a( + pub unsafe fn call_long_method_a( self, - this: jobject, + this: &Ref<'env, T>, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallLongMethodA)(self.env, this, method.as_raw(), args); + let result = ((**self.env).v1_2.CallLongMethodA)(self.env, this.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } - pub unsafe fn call_float_method_a( + pub unsafe fn call_float_method_a( self, - this: jobject, + this: &Ref<'env, T>, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallFloatMethodA)(self.env, this, method.as_raw(), args); + let result = ((**self.env).v1_2.CallFloatMethodA)(self.env, this.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } - pub unsafe fn call_double_method_a( + pub unsafe fn call_double_method_a( self, - this: jobject, + this: &Ref<'env, T>, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallDoubleMethodA)(self.env, this, method.as_raw(), args); + let result = ((**self.env).v1_2.CallDoubleMethodA)(self.env, this.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } - pub unsafe fn call_void_method_a( + pub unsafe fn call_void_method_a( self, - this: jobject, + this: &Ref<'env, T>, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result<(), Local<'env, E>> { - ((**self.env).v1_2.CallVoidMethodA)(self.env, this, method.as_raw(), args); + ((**self.env).v1_2.CallVoidMethodA)(self.env, this.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check() } @@ -421,9 +421,10 @@ impl<'env> Env<'env> { self, class: &JClass, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result>, Local<'env, E>> { - let result = ((**self.env).v1_2.CallStaticObjectMethodA)(self.env, class.as_raw(), method.as_raw(), args); + let result = + ((**self.env).v1_2.CallStaticObjectMethodA)(self.env, class.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; if result.is_null() { Ok(None) @@ -436,9 +437,10 @@ impl<'env> Env<'env> { self, class: &JClass, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallStaticBooleanMethodA)(self.env, class.as_raw(), method.as_raw(), args); + let result = + ((**self.env).v1_2.CallStaticBooleanMethodA)(self.env, class.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result != JNI_FALSE) } @@ -447,9 +449,10 @@ impl<'env> Env<'env> { self, class: &JClass, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallStaticByteMethodA)(self.env, class.as_raw(), method.as_raw(), args); + let result = + ((**self.env).v1_2.CallStaticByteMethodA)(self.env, class.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } @@ -458,9 +461,10 @@ impl<'env> Env<'env> { self, class: &JClass, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallStaticCharMethodA)(self.env, class.as_raw(), method.as_raw(), args); + let result = + ((**self.env).v1_2.CallStaticCharMethodA)(self.env, class.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } @@ -469,9 +473,10 @@ impl<'env> Env<'env> { self, class: &JClass, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallStaticShortMethodA)(self.env, class.as_raw(), method.as_raw(), args); + let result = + ((**self.env).v1_2.CallStaticShortMethodA)(self.env, class.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } @@ -480,9 +485,9 @@ impl<'env> Env<'env> { self, class: &JClass, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallStaticIntMethodA)(self.env, class.as_raw(), method.as_raw(), args); + let result = ((**self.env).v1_2.CallStaticIntMethodA)(self.env, class.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } @@ -491,9 +496,10 @@ impl<'env> Env<'env> { self, class: &JClass, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallStaticLongMethodA)(self.env, class.as_raw(), method.as_raw(), args); + let result = + ((**self.env).v1_2.CallStaticLongMethodA)(self.env, class.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } @@ -502,9 +508,10 @@ impl<'env> Env<'env> { self, class: &JClass, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallStaticFloatMethodA)(self.env, class.as_raw(), method.as_raw(), args); + let result = + ((**self.env).v1_2.CallStaticFloatMethodA)(self.env, class.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } @@ -513,9 +520,10 @@ impl<'env> Env<'env> { self, class: &JClass, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result> { - let result = ((**self.env).v1_2.CallStaticDoubleMethodA)(self.env, class.as_raw(), method.as_raw(), args); + let result = + ((**self.env).v1_2.CallStaticDoubleMethodA)(self.env, class.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check()?; Ok(result) } @@ -524,16 +532,20 @@ impl<'env> Env<'env> { self, class: &JClass, method: JMethodID, - args: *const jvalue, + args: &[jvalue], ) -> Result<(), Local<'env, E>> { - ((**self.env).v1_2.CallStaticVoidMethodA)(self.env, class.as_raw(), method.as_raw(), args); + ((**self.env).v1_2.CallStaticVoidMethodA)(self.env, class.as_raw(), method.as_raw(), args.as_ptr()); self.exception_check() } // Instance Fields - pub unsafe fn get_object_field(self, this: jobject, field: JFieldID) -> Option> { - let result = ((**self.env).v1_2.GetObjectField)(self.env, this, field.as_raw()); + pub unsafe fn get_object_field( + self, + this: &Ref<'env, T>, + field: JFieldID, + ) -> Option> { + let result = ((**self.env).v1_2.GetObjectField)(self.env, this.as_raw(), field.as_raw()); if result.is_null() { None } else { @@ -541,73 +553,83 @@ impl<'env> Env<'env> { } } - pub unsafe fn get_boolean_field(self, this: jobject, field: JFieldID) -> bool { - let result = ((**self.env).v1_2.GetBooleanField)(self.env, this, field.as_raw()); + pub unsafe fn get_boolean_field(self, this: &Ref<'env, T>, field: JFieldID) -> bool { + let result = ((**self.env).v1_2.GetBooleanField)(self.env, this.as_raw(), field.as_raw()); result != JNI_FALSE } - pub unsafe fn get_byte_field(self, this: jobject, field: JFieldID) -> jbyte { - ((**self.env).v1_2.GetByteField)(self.env, this, field.as_raw()) + pub unsafe fn get_byte_field(self, this: &Ref<'env, T>, field: JFieldID) -> jbyte { + ((**self.env).v1_2.GetByteField)(self.env, this.as_raw(), field.as_raw()) } - pub unsafe fn get_char_field(self, this: jobject, field: JFieldID) -> jchar { - ((**self.env).v1_2.GetCharField)(self.env, this, field.as_raw()) + pub unsafe fn get_char_field(self, this: &Ref<'env, T>, field: JFieldID) -> jchar { + ((**self.env).v1_2.GetCharField)(self.env, this.as_raw(), field.as_raw()) } - pub unsafe fn get_short_field(self, this: jobject, field: JFieldID) -> jshort { - ((**self.env).v1_2.GetShortField)(self.env, this, field.as_raw()) + pub unsafe fn get_short_field(self, this: &Ref<'env, T>, field: JFieldID) -> jshort { + ((**self.env).v1_2.GetShortField)(self.env, this.as_raw(), field.as_raw()) } - pub unsafe fn get_int_field(self, this: jobject, field: JFieldID) -> jint { - ((**self.env).v1_2.GetIntField)(self.env, this, field.as_raw()) + pub unsafe fn get_int_field(self, this: &Ref<'env, T>, field: JFieldID) -> jint { + ((**self.env).v1_2.GetIntField)(self.env, this.as_raw(), field.as_raw()) } - pub unsafe fn get_long_field(self, this: jobject, field: JFieldID) -> jlong { - ((**self.env).v1_2.GetLongField)(self.env, this, field.as_raw()) + pub unsafe fn get_long_field(self, this: &Ref<'env, T>, field: JFieldID) -> jlong { + ((**self.env).v1_2.GetLongField)(self.env, this.as_raw(), field.as_raw()) } - pub unsafe fn get_float_field(self, this: jobject, field: JFieldID) -> jfloat { - ((**self.env).v1_2.GetFloatField)(self.env, this, field.as_raw()) + pub unsafe fn get_float_field(self, this: &Ref<'env, T>, field: JFieldID) -> jfloat { + ((**self.env).v1_2.GetFloatField)(self.env, this.as_raw(), field.as_raw()) } - pub unsafe fn get_double_field(self, this: jobject, field: JFieldID) -> jdouble { - ((**self.env).v1_2.GetDoubleField)(self.env, this, field.as_raw()) + pub unsafe fn get_double_field(self, this: &Ref<'env, T>, field: JFieldID) -> jdouble { + ((**self.env).v1_2.GetDoubleField)(self.env, this.as_raw(), field.as_raw()) } - pub unsafe fn set_object_field(self, this: jobject, field: JFieldID, value: impl AsArg) { - ((**self.env).v1_2.SetObjectField)(self.env, this, field.as_raw(), value.as_arg()); + pub unsafe fn set_object_field( + self, + this: &Ref<'env, T>, + field: JFieldID, + value: impl AsArg, + ) { + ((**self.env).v1_2.SetObjectField)(self.env, this.as_raw(), field.as_raw(), value.as_arg()); } - pub unsafe fn set_boolean_field(self, this: jobject, field: JFieldID, value: bool) { - ((**self.env).v1_2.SetBooleanField)(self.env, this, field.as_raw(), if value { JNI_TRUE } else { JNI_FALSE }); + pub unsafe fn set_boolean_field(self, this: &Ref<'env, T>, field: JFieldID, value: bool) { + ((**self.env).v1_2.SetBooleanField)( + self.env, + this.as_raw(), + field.as_raw(), + if value { JNI_TRUE } else { JNI_FALSE }, + ); } - pub unsafe fn set_byte_field(self, this: jobject, field: JFieldID, value: jbyte) { - ((**self.env).v1_2.SetByteField)(self.env, this, field.as_raw(), value); + pub unsafe fn set_byte_field(self, this: &Ref<'env, T>, field: JFieldID, value: jbyte) { + ((**self.env).v1_2.SetByteField)(self.env, this.as_raw(), field.as_raw(), value); } - pub unsafe fn set_char_field(self, this: jobject, field: JFieldID, value: jchar) { - ((**self.env).v1_2.SetCharField)(self.env, this, field.as_raw(), value); + pub unsafe fn set_char_field(self, this: &Ref<'env, T>, field: JFieldID, value: jchar) { + ((**self.env).v1_2.SetCharField)(self.env, this.as_raw(), field.as_raw(), value); } - pub unsafe fn set_short_field(self, this: jobject, field: JFieldID, value: jshort) { - ((**self.env).v1_2.SetShortField)(self.env, this, field.as_raw(), value); + pub unsafe fn set_short_field(self, this: &Ref<'env, T>, field: JFieldID, value: jshort) { + ((**self.env).v1_2.SetShortField)(self.env, this.as_raw(), field.as_raw(), value); } - pub unsafe fn set_int_field(self, this: jobject, field: JFieldID, value: jint) { - ((**self.env).v1_2.SetIntField)(self.env, this, field.as_raw(), value); + pub unsafe fn set_int_field(self, this: &Ref<'env, T>, field: JFieldID, value: jint) { + ((**self.env).v1_2.SetIntField)(self.env, this.as_raw(), field.as_raw(), value); } - pub unsafe fn set_long_field(self, this: jobject, field: JFieldID, value: jlong) { - ((**self.env).v1_2.SetLongField)(self.env, this, field.as_raw(), value); + pub unsafe fn set_long_field(self, this: &Ref<'env, T>, field: JFieldID, value: jlong) { + ((**self.env).v1_2.SetLongField)(self.env, this.as_raw(), field.as_raw(), value); } - pub unsafe fn set_float_field(self, this: jobject, field: JFieldID, value: jfloat) { - ((**self.env).v1_2.SetFloatField)(self.env, this, field.as_raw(), value); + pub unsafe fn set_float_field(self, this: &Ref<'env, T>, field: JFieldID, value: jfloat) { + ((**self.env).v1_2.SetFloatField)(self.env, this.as_raw(), field.as_raw(), value); } - pub unsafe fn set_double_field(self, this: jobject, field: JFieldID, value: jdouble) { - ((**self.env).v1_2.SetDoubleField)(self.env, this, field.as_raw(), value); + pub unsafe fn set_double_field(self, this: &Ref<'env, T>, field: JFieldID, value: jdouble) { + ((**self.env).v1_2.SetDoubleField)(self.env, this.as_raw(), field.as_raw(), value); } // Static Fields From 7b9a60b8dd2a7ec318b1102a8798c9d9420dc57c Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Mon, 24 Mar 2025 11:54:22 +0800 Subject: [PATCH 04/10] Adjust java-spaghetti-gen for changes in `Env` --- java-spaghetti-gen/src/emit_rust/fields.rs | 16 ++++++++-------- java-spaghetti-gen/src/emit_rust/methods.rs | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/java-spaghetti-gen/src/emit_rust/fields.rs b/java-spaghetti-gen/src/emit_rust/fields.rs index df5800d..92c5f95 100644 --- a/java-spaghetti-gen/src/emit_rust/fields.rs +++ b/java-spaghetti-gen/src/emit_rust/fields.rs @@ -186,7 +186,7 @@ impl<'a> Field<'a> { } else { writeln!( out, - "{indent} __jni_env.get_{field_fragment}_field(self.as_raw(), __jni_field)", + "{indent} __jni_env.get_{field_fragment}_field(self, __jni_field)", )?; } writeln!(out, "{indent} }}")?; @@ -241,7 +241,7 @@ impl<'a> Field<'a> { writeln!( out, "{indent} \ - __jni_env.set_{field_fragment}_field(self.as_raw(), __jni_field, value)", + __jni_env.set_{field_fragment}_field(self, __jni_field, value)", )?; } writeln!(out, "{indent} }}")?; @@ -279,17 +279,17 @@ impl std::fmt::Display for ConstantWriter<'_> { LiteralConstant::Long(value) => write!(f, "{}i64", value), LiteralConstant::Float(value) if value.is_infinite() && *value < 0.0 => { - write!(f, "::std::f32::NEG_INFINITY") + write!(f, "f32::NEG_INFINITY") } - LiteralConstant::Float(value) if value.is_infinite() => write!(f, "::std::f32::INFINITY"), - LiteralConstant::Float(value) if value.is_nan() => write!(f, "::std::f32::NAN"), + LiteralConstant::Float(value) if value.is_infinite() => write!(f, "f32::INFINITY"), + LiteralConstant::Float(value) if value.is_nan() => write!(f, "f32::NAN"), LiteralConstant::Float(value) => write!(f, "{}f32", value), LiteralConstant::Double(value) if value.is_infinite() && *value < 0.0 => { - write!(f, "::std::f64::NEG_INFINITY") + write!(f, "f64::NEG_INFINITY") } - LiteralConstant::Double(value) if value.is_infinite() => write!(f, "::std::f64::INFINITY"), - LiteralConstant::Double(value) if value.is_nan() => write!(f, "::std::f64::NAN"), + LiteralConstant::Double(value) if value.is_infinite() => write!(f, "f64::INFINITY"), + LiteralConstant::Double(value) if value.is_nan() => write!(f, "f64::NAN"), LiteralConstant::Double(value) => write!(f, "{}f64", value), LiteralConstant::String(value) => std::fmt::Debug::fmt(value, f), diff --git a/java-spaghetti-gen/src/emit_rust/methods.rs b/java-spaghetti-gen/src/emit_rust/methods.rs index 99d3398..5e38613 100644 --- a/java-spaghetti-gen/src/emit_rust/methods.rs +++ b/java-spaghetti-gen/src/emit_rust/methods.rs @@ -234,20 +234,20 @@ impl<'a> Method<'a> { writeln!( out, "{indent} \ - __jni_env.new_object_a(__jni_class, __jni_method, __jni_args.as_ptr())", + __jni_env.new_object_a(__jni_class, __jni_method, &__jni_args)", )?; } else if self.java.is_static() { writeln!( out, "{indent} \ - __jni_env.call_static_{}_method_a(__jni_class, __jni_method, __jni_args.as_ptr())", + __jni_env.call_static_{}_method_a(__jni_class, __jni_method, &__jni_args)", ret_method_fragment )?; } else { writeln!( out, "{indent} \ - __jni_env.call_{}_method_a(self.as_raw(), __jni_method, __jni_args.as_ptr())", + __jni_env.call_{}_method_a(self, __jni_method, &__jni_args)", ret_method_fragment )?; } From 231b0973de7c1626933b2c8f55fdc86c4433304d Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Mon, 24 Mar 2025 16:10:47 +0800 Subject: [PATCH 05/10] Fix `jni_get_class` for `ObjectArray` --- java-spaghetti/src/array.rs | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/java-spaghetti/src/array.rs b/java-spaghetti/src/array.rs index 02ad711..4ad9c4b 100644 --- a/java-spaghetti/src/array.rs +++ b/java-spaghetti/src/array.rs @@ -1,7 +1,8 @@ +use std::collections::HashMap; use std::marker::PhantomData; use std::ops::{Bound, RangeBounds}; use std::ptr::null_mut; -use std::sync::OnceLock; +use std::sync::{LazyLock, OnceLock, RwLock}; use jni_sys::*; @@ -189,10 +190,24 @@ primitive_array! { DoubleArray, "[D\0", jdouble { NewDoubleArray SetDoubleArra /// See also [PrimitiveArray] for arrays of reference types. pub struct ObjectArray(core::convert::Infallible, PhantomData<(T, E)>); +// NOTE: This is a performance compromise for returning `&'static JClass`, +// still faster than non-cached `require_class`. +static OBJ_ARR_CLASSES: LazyLock>> = + LazyLock::new(|| RwLock::new(HashMap::new())); + unsafe impl ReferenceType for ObjectArray { fn jni_get_class(env: Env) -> &'static JClass { - static CLASS_CACHE: OnceLock = OnceLock::new(); - CLASS_CACHE.get_or_init(|| Self::static_with_jni_type(|t| unsafe { env.require_class(t) })) + Self::static_with_jni_type(|t| { + let class_map_reader = OBJ_ARR_CLASSES.read().unwrap(); + if let Some(&class) = class_map_reader.get(t) { + class + } else { + drop(class_map_reader); + let class: &'static JClass = Box::leak(Box::new(unsafe { env.require_class(t) })); + let _ = OBJ_ARR_CLASSES.write().unwrap().insert(t.to_string(), class); + class + } + }) } } From ef5789a02e75fb6003a423851f2928a1e96c777c Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Tue, 25 Mar 2025 05:29:42 +0800 Subject: [PATCH 06/10] Fix local reference leaks in `id_cache` --- java-spaghetti/src/id_cache.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/java-spaghetti/src/id_cache.rs b/java-spaghetti/src/id_cache.rs index dfe2687..7c74f8d 100644 --- a/java-spaghetti/src/id_cache.rs +++ b/java-spaghetti/src/id_cache.rs @@ -18,26 +18,30 @@ unsafe impl Send for JClass {} unsafe impl Sync for JClass {} impl JClass { - /// Creates a `JClass` from a raw JNI local reference of a class object. + /// Creates a `JClass` from an owned JNI local reference of a class object and deletes the + /// local reference. /// /// # Safety /// /// `class` must be a valid JNI local reference to a `java.lang.Class` object. + /// Do not use the passed `class` local reference after calling this function. + /// /// It is safe to pass the returned value of JNI `FindClass` to it if no exeception occurred. pub unsafe fn from_raw<'env>(env: Env<'env>, class: jclass) -> Self { assert!(!class.is_null(), "from_raw jclass argument"); let jnienv = env.as_raw(); let class_global = unsafe { ((**jnienv).v1_2.NewGlobalRef)(jnienv, class) }; + unsafe { ((**jnienv).v1_2.DeleteLocalRef)(jnienv, class) } Self::from_raw_global(env.vm(), class_global) } - /// Creates a `JClass` from a raw JNI global reference of a class object. + /// Wraps an owned raw JNI global reference of a class object. /// /// # Safety /// /// `class` must be a valid JNI global reference to a `java.lang.Class` object. pub unsafe fn from_raw_global(vm: VM, class: jobject) -> Self { - assert!(!class.is_null(), "from_raw_global jclass argument"); + assert!(!class.is_null(), "from_raw_global class argument"); Self { class: class as jclass, vm, @@ -87,11 +91,11 @@ unsafe impl Send for JFieldID {} unsafe impl Sync for JFieldID {} impl JFieldID { - /// Creates a [`JFieldID`] that wraps the given `raw` [`jfieldID`]. + /// Creates a [`JFieldID`] that wraps the given raw [`jfieldID`]. /// /// # Safety /// - /// Expects a valid, non-`null` ID. + /// Expects a valid, non-null ID. pub unsafe fn from_raw(raw: jfieldID) -> Self { assert!(!raw.is_null(), "from_raw fieldID argument"); Self { internal: raw } @@ -121,7 +125,7 @@ impl JMethodID { /// /// # Safety /// - /// Expects a valid, non-`null` ID. + /// Expects a valid, non-null ID. pub unsafe fn from_raw(raw: jmethodID) -> Self { assert!(!raw.is_null(), "from_raw methodID argument"); Self { internal: raw } From 808e31603dd1ebe9399345cd6c5068ad243dd708 Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Tue, 25 Mar 2025 05:31:34 +0800 Subject: [PATCH 07/10] Fix all clippy warnings; improve doc comments --- java-spaghetti-gen/src/run/mod.rs | 1 + java-spaghetti/src/array.rs | 48 +++++++++++++++++++++++------- java-spaghetti/src/env.rs | 23 +++++++------- java-spaghetti/src/jni_type.rs | 10 ++++--- java-spaghetti/src/lib.rs | 20 ++++++++++--- java-spaghetti/src/refs/arg.rs | 10 ++++++- java-spaghetti/src/refs/global.rs | 15 +++++++--- java-spaghetti/src/refs/local.rs | 15 ++++++++-- java-spaghetti/src/refs/ref_.rs | 12 ++++++-- java-spaghetti/src/refs/return_.rs | 14 +++++++-- java-spaghetti/src/string_chars.rs | 13 ++++++-- java-spaghetti/src/vm.rs | 2 +- 12 files changed, 137 insertions(+), 46 deletions(-) diff --git a/java-spaghetti-gen/src/run/mod.rs b/java-spaghetti-gen/src/run/mod.rs index d9b537b..6f7467c 100644 --- a/java-spaghetti-gen/src/run/mod.rs +++ b/java-spaghetti-gen/src/run/mod.rs @@ -1,5 +1,6 @@ //! Core generation logic +#[allow(clippy::module_inception)] mod run; pub use run::run; diff --git a/java-spaghetti/src/array.rs b/java-spaghetti/src/array.rs index 4ad9c4b..8437d8e 100644 --- a/java-spaghetti/src/array.rs +++ b/java-spaghetti/src/array.rs @@ -8,7 +8,10 @@ use jni_sys::*; use crate::{AsArg, Env, JClass, 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. /// /// See also [ObjectArray] for arrays of reference types. /// @@ -26,31 +29,38 @@ 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 to get the length of the java array, returns true if it is 0. + /// Uses JNI `GetArrayLength` to get the length of the java array, returns `true` if it is 0. fn is_empty(self: &Ref<'_, Self>) -> bool { self.len() == 0 } - /// 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(); @@ -76,7 +86,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()) } @@ -187,6 +197,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)>); @@ -218,6 +231,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 <= i32::MAX as usize); // jsize == jint == i32 let class = T::jni_get_class(env).as_raw(); @@ -233,6 +247,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, @@ -241,6 +256,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>) -> Local<'env, Self> { let size = elements.len(); let array = Self::new(env, size); @@ -252,16 +269,21 @@ 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 } } + /// Uses JNI `GetArrayLength` to get the length of the java array, returns `true` if it is 0. pub fn is_empty(self: &Ref<'_, Self>) -> bool { self.len() == 0 } - /// 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 <= i32::MAX as usize); // jsize == jint == i32 XXX: Should maybe be treated as an exception? let index = index as jsize; @@ -278,7 +300,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 <= i32::MAX as usize); // jsize == jint == i32 XXX: Should maybe be treated as an exception? let index = index as jsize; @@ -291,6 +315,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/env.rs b/java-spaghetti/src/env.rs index dc10495..b25dc8d 100644 --- a/java-spaghetti/src/env.rs +++ b/java-spaghetti/src/env.rs @@ -8,9 +8,9 @@ use jni_sys::*; use crate::{AsArg, JClass, JFieldID, JMethodID, 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 @@ -68,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 { @@ -221,7 +222,7 @@ impl<'env> Env<'env> { } unsafe fn require_class_jni(self, class: &str) -> Option { - debug_assert!(class.ends_with('\0')); + assert!(class.ends_with('\0')); let cls = ((**self.env).v1_2.FindClass)(self.env, class.as_ptr() as *const c_char); let exception: *mut _jobject = ((**self.env).v1_2.ExceptionOccurred)(self.env); if !exception.is_null() { @@ -235,8 +236,8 @@ impl<'env> Env<'env> { } pub unsafe fn require_method(self, class: &JClass, method: &str, descriptor: &str) -> JMethodID { - debug_assert!(method.ends_with('\0')); - debug_assert!(descriptor.ends_with('\0')); + assert!(method.ends_with('\0')); + assert!(descriptor.ends_with('\0')); let method = ((**self.env).v1_2.GetMethodID)( self.env, @@ -248,8 +249,8 @@ impl<'env> Env<'env> { } pub unsafe fn require_static_method(self, class: &JClass, method: &str, descriptor: &str) -> JMethodID { - debug_assert!(method.ends_with('\0')); - debug_assert!(descriptor.ends_with('\0')); + assert!(method.ends_with('\0')); + assert!(descriptor.ends_with('\0')); let method = ((**self.env).v1_2.GetStaticMethodID)( self.env, @@ -261,8 +262,8 @@ impl<'env> Env<'env> { } pub unsafe fn require_field(self, class: &JClass, field: &str, descriptor: &str) -> JFieldID { - debug_assert!(field.ends_with('\0')); - debug_assert!(field.ends_with('\0')); + assert!(field.ends_with('\0')); + assert!(field.ends_with('\0')); let field = ((**self.env).v1_2.GetFieldID)( self.env, @@ -274,8 +275,8 @@ impl<'env> Env<'env> { } pub unsafe fn require_static_field(self, class: &JClass, field: &str, descriptor: &str) -> JFieldID { - debug_assert!(field.ends_with('\0')); - debug_assert!(field.ends_with('\0')); + assert!(field.ends_with('\0')); + assert!(field.ends_with('\0')); let field = ((**self.env).v1_2.GetStaticFieldID)( self.env, diff --git a/java-spaghetti/src/jni_type.rs b/java-spaghetti/src/jni_type.rs index d7e07b3..a7a3e1d 100644 --- a/java-spaghetti/src/jni_type.rs +++ b/java-spaghetti/src/jni_type.rs @@ -2,10 +2,6 @@ use jni_sys::*; /// JNI bindings rely on this type being accurate. /// -/// **unsafe**: static_with_jni_type must pass a string terminated by '\0'. Failing to do so is a soundness bug, as -/// the string is passed directly to JNI as a raw pointer! Additionally, passing the wrong type may be a soundness bug -/// as although the Android JVM will simply panic and abort, I've no idea if that's a guarantee or not. -/// /// Why the awkward callback style instead of returning `&'static str`? Arrays of arrays may need to dynamically /// construct their type strings, which would need to leak. Worse, we can't easily intern those strings via /// lazy_static without running into: @@ -13,6 +9,12 @@ use jni_sys::*; /// ```text /// error[E0401]: can't use generic parameters from outer function /// ``` +/// +/// # Safety +/// +/// **unsafe**: static_with_jni_type must pass a string terminated by '\0'. Failing to do so is a soundness bug, as +/// the string is passed directly to JNI as a raw pointer! Additionally, passing the wrong type may be a soundness bug +/// as although the Android JVM will simply panic and abort, I've no idea if that's a guarantee or not. pub unsafe trait JniType { fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R; } diff --git a/java-spaghetti/src/lib.rs b/java-spaghetti/src/lib.rs index cd9594f..969ae5d 100644 --- a/java-spaghetti/src/lib.rs +++ b/java-spaghetti/src/lib.rs @@ -1,4 +1,4 @@ -//! 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). @@ -43,7 +43,7 @@ pub use refs::*; pub use string_chars::*; pub use vm::*; -/// Error returned on failed `.cast()`.` +/// Error returned on failed `.cast()`. #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] pub struct CastError; @@ -54,7 +54,8 @@ impl fmt::Display for CastError { } } -/// A marker type indicating this is a valid exception type that all exceptions thrown by java should be compatible with +/// A marker trait indicating this is a valid exception type that all exceptions thrown by Java +/// should be compatible with. pub trait ThrowableType: ReferenceType {} /// You should generally not be interacting with this type directly, but it must be public for codegen. @@ -67,20 +68,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 { @@ -88,6 +99,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..3e7858e 100644 --- a/java-spaghetti/src/refs/arg.rs +++ b/java-spaghetti/src/refs/arg.rs @@ -4,7 +4,7 @@ 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. /// @@ -18,6 +18,8 @@ pub struct Arg { } impl Arg { + /// # 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 { @@ -30,6 +32,8 @@ impl Arg { self.object } + /// # Safety + /// /// **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. @@ -41,6 +45,8 @@ impl Arg { } } + /// # Safety + /// /// **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. @@ -52,6 +58,8 @@ impl Arg { } } + /// # Safety + /// /// **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. diff --git a/java-spaghetti/src/refs/global.rs b/java-spaghetti/src/refs/global.rs index 4aed8a5..110fa3f 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,11 @@ unsafe impl Send for Global {} unsafe impl Sync for Global {} impl Global { + /// Wraps an owned raw JNI global reference. + /// + /// # Safety + /// + /// `object` must be an owned non-null JNI global reference that references an instance of type `T`. pub unsafe fn from_raw(vm: VM, object: jobject) -> Self { Self { object, @@ -41,9 +46,11 @@ impl Global { 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. + std::mem::forget(self); object } diff --git a/java-spaghetti/src/refs/local.rs b/java-spaghetti/src/refs/local.rs index b615546..21d7fe3 100644 --- a/java-spaghetti/src/refs/local.rs +++ b/java-spaghetti/src/refs/local.rs @@ -23,15 +23,21 @@ 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*... pub struct Local<'env, T: ReferenceType> { ref_: Ref<'env, T>, } impl<'env, T: ReferenceType> Local<'env, T> { + /// Wraps an owned raw JNI local reference. + /// + /// # Safety + /// + /// - `object` must be an owned non-null JNI local reference that belongs to `env`; + /// - `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), @@ -46,9 +52,11 @@ 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 local reference; prevents `DeleteLocalRef` from being called on dropping. 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); object } @@ -64,6 +72,7 @@ impl<'env, T: ReferenceType> Local<'env, T> { unsafe { Global::from_raw(env.vm(), object) } } + #[allow(clippy::should_implement_trait)] pub fn as_ref(&self) -> &Ref<'_, T> { &self.ref_ } diff --git a/java-spaghetti/src/refs/ref_.rs b/java-spaghetti/src/refs/ref_.rs index 1b1a537..1be5ba9 100644 --- a/java-spaghetti/src/refs/ref_.rs +++ b/java-spaghetti/src/refs/ref_.rs @@ -9,9 +9,9 @@ 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 +/// **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 +25,12 @@ 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 for `'env` lifetime; + /// - `object` references an instance of type `T`. pub unsafe fn from_raw(env: Env<'env>, object: jobject) -> Self { Self { object, @@ -67,6 +73,7 @@ impl<'env, T: ReferenceType> Ref<'env, T> { Ok(()) } + #[allow(clippy::missing_safety_doc)] pub unsafe fn cast_unchecked(self) -> Ref<'env, U> { transmute(self) } @@ -83,6 +90,7 @@ impl<'env, T: ReferenceType> Ref<'env, T> { unsafe { self.cast_unchecked() } } + #[allow(clippy::missing_safety_doc)] pub unsafe fn cast_ref_unchecked(&self) -> &Ref<'env, U> { transmute(self) } diff --git a/java-spaghetti/src/refs/return_.rs b/java-spaghetti/src/refs/return_.rs index b84916b..161b97b 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(), @@ -37,6 +46,7 @@ impl<'env, T: ReferenceType> Return<'env, T> { } 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..b07529f 100644 --- a/java-spaghetti/src/vm.rs +++ b/java-spaghetti/src/vm.rs @@ -5,7 +5,7 @@ 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. From 2d6510ca725d27dec6bee6e16c27e7ebcb290495 Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Wed, 26 Mar 2025 20:24:23 +0800 Subject: [PATCH 08/10] Add `Monitor`, add doc comments, add `is_same_object`, fix `ReferenceType` generation --- java-spaghetti-gen/src/emit_rust/structs.rs | 20 ++--- java-spaghetti/src/array.rs | 80 +++++++++--------- java-spaghetti/src/env.rs | 33 ++++---- java-spaghetti/src/jni_type.rs | 75 +++++++++-------- java-spaghetti/src/lib.rs | 22 ++++- java-spaghetti/src/refs/arg.rs | 1 + java-spaghetti/src/refs/global.rs | 4 + java-spaghetti/src/refs/local.rs | 17 ++++ java-spaghetti/src/refs/ref_.rs | 93 ++++++++++++++++++++- java-spaghetti/src/refs/return_.rs | 1 + 10 files changed, 233 insertions(+), 113 deletions(-) diff --git a/java-spaghetti-gen/src/emit_rust/structs.rs b/java-spaghetti-gen/src/emit_rust/structs.rs index f7542cf..1946507 100644 --- a/java-spaghetti-gen/src/emit_rust/structs.rs +++ b/java-spaghetti-gen/src/emit_rust/structs.rs @@ -118,21 +118,15 @@ impl Struct { let rust_name = &self.rust.struct_name; writeln!(out, "{attributes}{visibility} enum {rust_name}{{}}")?; - if !self.java.is_static() { - writeln!( - out, - "unsafe impl ::java_spaghetti::ReferenceType for {rust_name} {{\ - \n fn jni_get_class(__jni_env: ::java_spaghetti::Env) -> &'static ::java_spaghetti::JClass {{\ - \n Self::__class_global_ref(__jni_env)\ - \n }}\ - \n}}", - )?; - } + writeln!( out, - "unsafe impl ::java_spaghetti::JniType for {rust_name} {{\ - \n fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R {{\ - \n callback({})\ + "unsafe impl ::java_spaghetti::ReferenceType for {rust_name} {{\ + \n fn jni_reference_type_name() -> ::std::borrow::Cow<'static, str> {{\ + \n ::std::borrow::Cow::Borrowed({})\ + \n }}\ + \n fn jni_get_class(__jni_env: ::java_spaghetti::Env) -> &'static ::java_spaghetti::JClass {{\ + \n Self::__class_global_ref(__jni_env)\ \n }}\ \n}}", StrEmitter(self.java.path().as_str()), diff --git a/java-spaghetti/src/array.rs b/java-spaghetti/src/array.rs index 8437d8e..8418ac4 100644 --- a/java-spaghetti/src/array.rs +++ b/java-spaghetti/src/array.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::collections::HashMap; use std::marker::PhantomData; use std::ops::{Bound, RangeBounds}; @@ -6,12 +7,12 @@ use std::sync::{LazyLock, OnceLock, RwLock}; use jni_sys::*; -use crate::{AsArg, Env, JClass, JniType, Local, Ref, ReferenceType, ThrowableType}; +use crate::{AsArg, Env, JClass, Local, Ref, ReferenceType, ThrowableType}; /// 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. +/// 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. /// @@ -29,35 +30,35 @@ pub trait PrimitiveArray: Sized + ReferenceType where T: Clone + Default, { - /// Uses JNI `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 JNI `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 JNI `Get{Type}ArrayRegion` to read the contents of the java array within `[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 JNI `Set{Type}ArrayRegion` to set the contents of the java array within `[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 JNI `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 JNI `GetArrayLength` to get the length of the java array, returns `true` if it is 0. + /// Uses JNI `GetArrayLength` to get the length of the Java array, returns `true` if it is 0. fn is_empty(self: &Ref<'_, Self>) -> bool { self.len() == 0 } - /// Uses JNI `GetArrayLength` + `Get{Type}ArrayRegion` to read the contents of the java array within given range + /// 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. @@ -86,7 +87,7 @@ where vec } - /// Uses JNI `GetArrayLength` + `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()) } @@ -98,14 +99,12 @@ macro_rules! primitive_array { pub enum $name {} unsafe impl ReferenceType for $name { + fn jni_reference_type_name() -> Cow<'static, str> { + Cow::Borrowed($type_str) + } fn jni_get_class(env: Env) -> &'static JClass { static CLASS_CACHE: OnceLock = OnceLock::new(); - CLASS_CACHE.get_or_init(|| Self::static_with_jni_type(|t| unsafe { env.require_class(t) })) - } - } - unsafe impl JniType for $name { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { - callback($type_str) + CLASS_CACHE.get_or_init(|| unsafe { env.require_class(&Self::jni_reference_type_name()) }) } } @@ -116,8 +115,8 @@ macro_rules! primitive_array { let jnienv = env.as_raw(); unsafe { let object = ((**jnienv).v1_2.$new_array)(jnienv, size); - let exception = ((**jnienv).v1_2.ExceptionOccurred)(jnienv); - assert!(exception.is_null()); // Only sane exception here is an OOM exception + assert!(!object.is_null(), "OOM"); + env.exception_check_raw().expect("OOM"); // Only sane exception here is an OOM exception Local::from_raw(env, object) } } @@ -203,35 +202,32 @@ primitive_array! { DoubleArray, "[D\0", jdouble { NewDoubleArray SetDoubleArra /// See also [PrimitiveArray] for arrays of reference types. pub struct ObjectArray(core::convert::Infallible, PhantomData<(T, E)>); -// NOTE: This is a performance compromise for returning `&'static JClass`, -// still faster than non-cached `require_class`. +// NOTE: This is a performance compromise for returning `&'static JClass`, still faster than non-cached `FindClass`. static OBJ_ARR_CLASSES: LazyLock>> = LazyLock::new(|| RwLock::new(HashMap::new())); unsafe impl ReferenceType for ObjectArray { - fn jni_get_class(env: Env) -> &'static JClass { - Self::static_with_jni_type(|t| { - let class_map_reader = OBJ_ARR_CLASSES.read().unwrap(); - if let Some(&class) = class_map_reader.get(t) { - class - } else { - drop(class_map_reader); - let class: &'static JClass = Box::leak(Box::new(unsafe { env.require_class(t) })); - let _ = OBJ_ARR_CLASSES.write().unwrap().insert(t.to_string(), class); - class - } - }) + fn jni_reference_type_name() -> Cow<'static, str> { + let inner = T::jni_reference_type_name(); + Cow::Owned(format!("[L{};\0", inner.trim_end_matches("\0"))) } -} - -unsafe impl JniType for ObjectArray { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { - T::static_with_jni_type(|inner| callback(format!("[L{};\0", inner.trim_end_matches("\0")).as_str())) + fn jni_get_class(env: Env) -> &'static JClass { + let t = Self::jni_reference_type_name(); + let class_map_reader = OBJ_ARR_CLASSES.read().unwrap(); + if let Some(&class) = class_map_reader.get(t.as_ref()) { + class + } else { + drop(class_map_reader); + let class = unsafe { env.require_class(&t) }; + let class_leaked: &'static JClass = Box::leak(Box::new(class)); + let _ = OBJ_ARR_CLASSES.write().unwrap().insert(t.to_string(), class_leaked); + class_leaked + } } } impl ObjectArray { - /// Uses JNI `NewObjectArray` to create a new java object array. + /// Uses JNI `NewObjectArray` to create a new Java object array. pub fn new<'env>(env: Env<'env>, size: usize) -> Local<'env, Self> { assert!(size <= i32::MAX as usize); // jsize == jint == i32 let class = T::jni_get_class(env).as_raw(); @@ -242,7 +238,7 @@ impl ObjectArray { let fill = null_mut(); ((**env).v1_2.NewObjectArray)(env, size, class, fill) }; - // Only sane exception here is an OOM exception + assert!(!object.is_null(), "OOM"); env.exception_check::().map_err(|_| "OOM").unwrap(); unsafe { Local::from_raw(env, object) } } @@ -256,7 +252,7 @@ impl ObjectArray { } } - /// Uses JNI `NewObjectArray` to create a new java object array of the exact size, then sets its items + /// 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>) -> Local<'env, Self> { let size = elements.len(); @@ -269,13 +265,13 @@ impl ObjectArray { array } - /// Uses JNI `GetArrayLength` to get the length of the java 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 } } - /// Uses JNI `GetArrayLength` to get the length of the java array, returns `true` if it is 0. + /// Uses JNI `GetArrayLength` to get the length of the Java array, returns `true` if it is 0. pub fn is_empty(self: &Ref<'_, Self>) -> bool { self.len() == 0 } diff --git a/java-spaghetti/src/env.rs b/java-spaghetti/src/env.rs index b25dc8d..315b46a 100644 --- a/java-spaghetti/src/env.rs +++ b/java-spaghetti/src/env.rs @@ -140,22 +140,31 @@ 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. pub(crate) fn exception_check(self) -> Result<(), Local<'env, E>> { + self.exception_check_raw() + .map_err(|throwable| unsafe { Local::from_raw(self, throwable) }) + } + + /// The same as `exception_check`, except that it may return a raw local reference of the exception. + pub(crate) fn exception_check_raw(self) -> Result<(), jthrowable> { unsafe { let exception = ((**self.env).v1_2.ExceptionOccurred)(self.env); if exception.is_null() { Ok(()) } else { ((**self.env).v1_2.ExceptionClear)(self.env); - Err(Local::from_raw(self, exception)) + Err(exception as jthrowable) } } } - unsafe fn exception_to_string(self, exception: jobject) -> String { + pub(crate) unsafe fn raw_exception_to_string(self, exception: jthrowable) -> String { static METHOD_GET_MESSAGE: OnceLock = OnceLock::new(); let throwable_get_message = *METHOD_GET_MESSAGE.get_or_init(|| { // use JNI FindClass to avoid infinte recursion. @@ -165,12 +174,8 @@ impl<'env> Env<'env> { let message = ((**self.env).v1_2.CallObjectMethodA)(self.env, exception, throwable_get_message.as_raw(), ptr::null_mut()); - let e2: *mut _jobject = ((**self.env).v1_2.ExceptionOccurred)(self.env); - if !e2.is_null() { - ((**self.env).v1_2.ExceptionClear)(self.env); - panic!("exception happened calling Throwable.getMessage()"); - } - + self.exception_check_raw() + .expect("exception happened calling Throwable.getMessage()"); StringChars::from_env_jstring(self, message).to_string_lossy() } @@ -201,12 +206,10 @@ impl<'env> Env<'env> { let args = [jvalue { l: string }]; let result: *mut _jobject = ((**self.env).v1_2.CallObjectMethodA)(self.env, classloader, cl_method.as_raw(), args.as_ptr()); - let exception: *mut _jobject = ((**self.env).v1_2.ExceptionOccurred)(self.env); - if !exception.is_null() { - ((**self.env).v1_2.ExceptionClear)(self.env); + if let Err(exception) = self.exception_check_raw() { panic!( "exception happened calling loadClass(): {}", - self.exception_to_string(exception) + self.raw_exception_to_string(exception) ); } else if result.is_null() { panic!("loadClass() returned null"); @@ -224,11 +227,7 @@ impl<'env> Env<'env> { unsafe fn require_class_jni(self, class: &str) -> Option { assert!(class.ends_with('\0')); let cls = ((**self.env).v1_2.FindClass)(self.env, class.as_ptr() as *const c_char); - let exception: *mut _jobject = ((**self.env).v1_2.ExceptionOccurred)(self.env); - if !exception.is_null() { - ((**self.env).v1_2.ExceptionClear)(self.env); - return None; - } + self.exception_check_raw().ok()?; if cls.is_null() { return None; } diff --git a/java-spaghetti/src/jni_type.rs b/java-spaghetti/src/jni_type.rs index a7a3e1d..4a60b97 100644 --- a/java-spaghetti/src/jni_type.rs +++ b/java-spaghetti/src/jni_type.rs @@ -1,71 +1,74 @@ +//! XXX: This type came from the original [jni-glue](https://docs.rs/jni-glue/0.0.10/src/jni_glue/jni_type.rs.html), +//! I'm not sure of its possible funcationality in the future, but it's currently preserved. +//! +//! NOTE: While primitive array type signatures like "[I\0" can be passed to the JNI `FindClass`, a primitive "class" +//! like `int.class` cannot be obtained by passing "I\0" to `FindClass`. Primitive "classes" may be obtained from +//! [java.lang.reflect.Method](https://docs.oracle.com/javase/8/docs/api/java/lang/reflect/Method.html#getParameterTypes). + +use std::borrow::Cow; + use jni_sys::*; -/// JNI bindings rely on this type being accurate. -/// -/// Why the awkward callback style instead of returning `&'static str`? Arrays of arrays may need to dynamically -/// construct their type strings, which would need to leak. Worse, we can't easily intern those strings via -/// lazy_static without running into: -/// -/// ```text -/// error[E0401]: can't use generic parameters from outer function -/// ``` -/// -/// # Safety -/// -/// **unsafe**: static_with_jni_type must pass a string terminated by '\0'. Failing to do so is a soundness bug, as -/// the string is passed directly to JNI as a raw pointer! Additionally, passing the wrong type may be a soundness bug -/// as although the Android JVM will simply panic and abort, I've no idea if that's a guarantee or not. +use crate::ReferenceType; + +#[doc(hidden)] pub unsafe trait JniType { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R; + fn jni_type_name() -> Cow<'static, str>; } unsafe impl JniType for () { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { - callback("V\0") + fn jni_type_name() -> Cow<'static, str> { + Cow::Borrowed("V\0") } } unsafe impl JniType for bool { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { - callback("Z\0") + fn jni_type_name() -> Cow<'static, str> { + Cow::Borrowed("Z\0") } } unsafe impl JniType for jbyte { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { - callback("B\0") + fn jni_type_name() -> Cow<'static, str> { + Cow::Borrowed("B\0") } } unsafe impl JniType for jchar { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { - callback("C\0") + fn jni_type_name() -> Cow<'static, str> { + Cow::Borrowed("C\0") } } unsafe impl JniType for jshort { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { - callback("S\0") + fn jni_type_name() -> Cow<'static, str> { + Cow::Borrowed("S\0") } } unsafe impl JniType for jint { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { - callback("I\0") + fn jni_type_name() -> Cow<'static, str> { + Cow::Borrowed("I\0") } } unsafe impl JniType for jlong { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { - callback("J\0") + fn jni_type_name() -> Cow<'static, str> { + Cow::Borrowed("J\0") } } unsafe impl JniType for jfloat { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { - callback("F\0") + fn jni_type_name() -> Cow<'static, str> { + Cow::Borrowed("F\0") } } unsafe impl JniType for jdouble { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { - callback("D\0") + fn jni_type_name() -> Cow<'static, str> { + Cow::Borrowed("D\0") } } unsafe impl JniType for &str { - fn static_with_jni_type(callback: impl FnOnce(&str) -> R) -> R { - callback("Ljava/lang/String;\0") + fn jni_type_name() -> Cow<'static, str> { + Cow::Borrowed("Ljava/lang/String;\0") + } +} + +unsafe impl JniType for T { + fn jni_type_name() -> Cow<'static, str> { + Self::jni_reference_type_name() } } diff --git a/java-spaghetti/src/lib.rs b/java-spaghetti/src/lib.rs index 969ae5d..d3d8c14 100644 --- a/java-spaghetti/src/lib.rs +++ b/java-spaghetti/src/lib.rs @@ -2,9 +2,13 @@ //! //! 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)] +use std::borrow::Cow; use std::fmt; use std::ptr::null_mut; @@ -58,11 +62,25 @@ impl fmt::Display for CastError { /// should be compatible with. pub trait ThrowableType: ReferenceType {} +/// JNI bindings rely on this type being accurate. +/// /// You should generally not be interacting with this type directly, but it must be public for codegen. +/// +/// # Safety +/// +/// **unsafe**: `jni_reference_type_name` must pass a string terminated by '\0'. Failing to do so is a soundness bug, as +/// the string is passed directly to JNI as a raw pointer! Additionally, passing the wrong type may be a soundness bug +/// as although the Android JVM will simply panic and abort, I have no idea if that is a guarantee or not. #[doc(hidden)] pub unsafe trait ReferenceType: JniType + Sized + 'static { - // There couldn't be a default impl holding a static `OnceLock`: the compiler may not generate - // an independent static item for each generated struct that implements `ReferenceType`. + /// Returns a string value compatible with JNI + /// [FindClass](https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#FindClass). + fn jni_reference_type_name() -> Cow<'static, str>; + + /// Returns a cached `JClass` of the class object for this reference type. + /// + /// There could not be a default implementation holding a static `OnceLock`: the compiler may not + /// generate an independent static item for each generated struct that implements `ReferenceType`. fn jni_get_class<'env>(env: Env<'env>) -> &'static JClass; } diff --git a/java-spaghetti/src/refs/arg.rs b/java-spaghetti/src/refs/arg.rs index 3e7858e..556d3d2 100644 --- a/java-spaghetti/src/refs/arg.rs +++ b/java-spaghetti/src/refs/arg.rs @@ -28,6 +28,7 @@ impl Arg { } } + /// Returns the raw JNI reference pointer. pub fn as_raw(&self) -> jobject { self.object } diff --git a/java-spaghetti/src/refs/global.rs b/java-spaghetti/src/refs/global.rs index 110fa3f..f7b78d0 100644 --- a/java-spaghetti/src/refs/global.rs +++ b/java-spaghetti/src/refs/global.rs @@ -38,10 +38,12 @@ 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 } @@ -54,6 +56,7 @@ impl Global { 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()) }; @@ -61,6 +64,7 @@ impl Global { unsafe { Local::from_raw(env, object) } } + /// Returns a borrowed [Ref], with which Java methods from generated bindings can be used. 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 21d7fe3..1912dbf 100644 --- a/java-spaghetti/src/refs/local.rs +++ b/java-spaghetti/src/refs/local.rs @@ -44,10 +44,12 @@ impl<'env, T: ReferenceType> Local<'env, T> { } } + /// 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() } @@ -60,10 +62,18 @@ impl<'env, T: ReferenceType> Local<'env, T> { 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(); @@ -72,11 +82,14 @@ impl<'env, T: ReferenceType> Local<'env, T> { unsafe { Global::from_raw(env.vm(), object) } } + /// Returns a borrowed [Ref], with which Java methods from generated bindings can be used. #[allow(clippy::should_implement_trait)] pub fn as_ref(&self) -> &Ref<'_, T> { &self.ref_ } + /// 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()) }; @@ -84,10 +97,13 @@ impl<'env, T: ReferenceType> Local<'env, T> { unsafe { Return::from_raw(object) } } + /// 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()) } } + /// Tries to cast itself to a JNI reference of type `U`. pub fn cast(&self) -> Result, crate::CastError> { self.as_ref().check_assignable::()?; let env = self.env(); @@ -97,6 +113,7 @@ impl<'env, T: ReferenceType> Local<'env, T> { Ok(unsafe { Local::from_raw(env, object) }) } + /// Casts itself towards a super class type, without the cost of runtime checking. pub fn upcast(&self) -> Local<'env, U> where Self: AssignableTo, diff --git a/java-spaghetti/src/refs/ref_.rs b/java-spaghetti/src/refs/ref_.rs index 1be5ba9..4fb0167 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; @@ -39,14 +40,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(); @@ -55,14 +59,22 @@ 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(); let object = unsafe { ((**jnienv).v1_2.NewLocalRef)(jnienv, self.as_raw()) }; assert!(!object.is_null()); - unsafe { Local::from_raw(self.env(), object) } + unsafe { Local::from_raw(env, object) } } + /// 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()) } + } + + /// 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(); @@ -73,16 +85,22 @@ impl<'env, T: ReferenceType> Ref<'env, T> { Ok(()) } - #[allow(clippy::missing_safety_doc)] + /// 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, @@ -90,22 +108,33 @@ impl<'env, T: ReferenceType> Ref<'env, T> { unsafe { self.cast_unchecked() } } - #[allow(clippy::missing_safety_doc)] + /// 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, { unsafe { self.cast_ref_unchecked() } } + + /// Enters monitored mode for the corresponding object in this thread. See [Monitor]. + pub fn monitor<'r>(&'r self) -> Monitor<'env, 'r, T> { + Monitor::new(self) + } } impl<'env, T: JavaDebug> Debug for Ref<'env, T> { @@ -119,3 +148,61 @@ impl<'env, T: JavaDisplay> Display for Ref<'env, T> { T::fmt(self, f) } } + +/// A non-null reference of a Java object locked with the JNI monitor mechanism, providing *limited* thread safety. +/// +/// 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, 'r, T: ReferenceType> { + inner: Option<&'r Ref<'env, T>>, +} + +impl<'env, 'r, T: ReferenceType> Monitor<'env, 'r, T> { + fn new(reference: &'r 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: Some(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(mut self) -> &'r Ref<'env, T> { + self.unlock_inner(); + self.inner.take().unwrap() + } + + fn unlock_inner(&mut self) { + if let Some(inner) = self.inner.as_ref() { + let env = inner.env; + let jnienv = env.as_raw(); + let result = unsafe { ((**jnienv).v1_2.MonitorExit)(jnienv, 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) } + ); + } + } + } +} + +impl<'env, 'r, T: ReferenceType> Deref for Monitor<'env, 'r, T> { + type Target = Ref<'env, T>; + fn deref(&self) -> &Self::Target { + self.inner.as_ref().unwrap() + } +} + +impl<'env, 'r, T: ReferenceType> Drop for Monitor<'env, 'r, T> { + fn drop(&mut self) { + self.unlock_inner(); + } +} diff --git a/java-spaghetti/src/refs/return_.rs b/java-spaghetti/src/refs/return_.rs index 161b97b..f17d9b4 100644 --- a/java-spaghetti/src/refs/return_.rs +++ b/java-spaghetti/src/refs/return_.rs @@ -40,6 +40,7 @@ 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 } From 3f9cf92aac564336c2685f74b7aac09ecd6b17c1 Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Wed, 26 Mar 2025 20:51:23 +0800 Subject: [PATCH 09/10] Add clippy in CI workflow --- .github/workflows/rust_nightly.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/rust_nightly.yml b/.github/workflows/rust_nightly.yml index 2759c43..df3f660 100644 --- a/.github/workflows/rust_nightly.yml +++ b/.github/workflows/rust_nightly.yml @@ -17,7 +17,10 @@ jobs: - uses: actions/checkout@v4 - run: rustup update nightly && rustup default nightly - run: rustup component add rustfmt + - run: rustup component add clippy - name: Check fmt run: cargo fmt -- --check + - name: Clippy + run: cargo clippy -- -Dwarnings - name: Test run: cargo test From 4e8bd08dde866f831de1c6bde2e981821d8af603 Mon Sep 17 00:00:00 2001 From: wuwbobo2021 Date: Thu, 27 Mar 2025 18:58:52 +0800 Subject: [PATCH 10/10] Fix docs for reference types; optimize `Local::cast` and `Monitor`; impl `AsRef` for `Local` --- java-spaghetti/src/refs/arg.rs | 31 +++++-------- java-spaghetti/src/refs/global.rs | 14 +++--- java-spaghetti/src/refs/local.rs | 58 +++++++++++------------ java-spaghetti/src/refs/ref_.rs | 77 +++++++++++++++---------------- 4 files changed, 84 insertions(+), 96 deletions(-) diff --git a/java-spaghetti/src/refs/arg.rs b/java-spaghetti/src/refs/arg.rs index 556d3d2..7571f58 100644 --- a/java-spaghetti/src/refs/arg.rs +++ b/java-spaghetti/src/refs/arg.rs @@ -8,7 +8,7 @@ use crate::{Env, Global, Local, Ref, ReferenceType}; /// /// 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)] @@ -20,7 +20,7 @@ pub struct Arg { impl Arg { /// # Safety /// - /// **unsafe**: There's no guarantee the jobject being passed is valid or null, nor any means of checking it. + /// **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, @@ -35,9 +35,9 @@ impl Arg { /// # Safety /// - /// **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. + /// **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 @@ -48,9 +48,9 @@ impl Arg { /// # Safety /// - /// **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. + /// **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 @@ -59,19 +59,12 @@ impl Arg { } } + /// This equals [Arg::into_ref] + [Ref::as_global]. + /// /// # Safety /// - /// **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. + /// **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 f7b78d0..de2c784 100644 --- a/java-spaghetti/src/refs/global.rs +++ b/java-spaghetti/src/refs/global.rs @@ -29,7 +29,8 @@ impl Global { /// /// # Safety /// - /// `object` must be an owned non-null JNI global reference that references an instance of type `T`. + /// `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, @@ -58,13 +59,14 @@ impl Global { /// 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 borrowed [Ref], with which Java methods from generated bindings can be used. + /// 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 1912dbf..b799c23 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 /// 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::*; @@ -36,7 +37,8 @@ impl<'env, T: ReferenceType> Local<'env, T> { /// /// # Safety /// - /// - `object` must be an owned non-null JNI local reference that belongs to `env`; + /// - `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 { @@ -67,7 +69,7 @@ impl<'env, T: ReferenceType> Local<'env, T> { /// 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, + /// 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()) } @@ -75,26 +77,13 @@ impl<'env, T: ReferenceType> Local<'env, T> { /// 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) } - } - - /// Returns a borrowed [Ref], with which Java methods from generated bindings can be used. - #[allow(clippy::should_implement_trait)] - 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. @@ -104,25 +93,19 @@ impl<'env, T: ReferenceType> Local<'env, T> { } /// Tries to cast itself to a JNI reference of type `U`. - pub fn cast(&self) -> Result, crate::CastError> { + pub fn cast(self) -> Result, crate::CastError> { self.as_ref().check_assignable::()?; - let env = self.env(); - let jnienv = env.as_raw(); - let object = unsafe { ((**jnienv).v1_2.NewLocalRef)(jnienv, self.as_raw()) }; - assert!(!object.is_null()); - Ok(unsafe { Local::from_raw(env, object) }) + // Memory layout of the inner `Ref<'env, U>` is the same as `Ref<'env, T>`. + Ok(unsafe { transmute::, Local<'_, U>>(self) }) } /// Casts itself towards a super class type, without the cost of runtime checking. - pub fn upcast(&self) -> Local<'env, U> + 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) } } } @@ -144,6 +127,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 4fb0167..ca95cec 100644 --- a/java-spaghetti/src/refs/ref_.rs +++ b/java-spaghetti/src/refs/ref_.rs @@ -10,6 +10,8 @@ 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. /// +/// `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 [Ref]. Of course, there's no @@ -30,7 +32,8 @@ impl<'env, T: ReferenceType> Ref<'env, T> { /// /// # Safety /// - /// - `object` is a non-null JNI reference, and it must keep valid for `'env` lifetime; + /// - `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 { @@ -68,6 +71,12 @@ impl<'env, T: ReferenceType> Ref<'env, T> { unsafe { Local::from_raw(env, object) } } + /// Enters monitored mode for the corresponding object in this thread. 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(); @@ -130,11 +139,6 @@ impl<'env, T: ReferenceType> Ref<'env, T> { { unsafe { self.cast_ref_unchecked() } } - - /// Enters monitored mode for the corresponding object in this thread. See [Monitor]. - pub fn monitor<'r>(&'r self) -> Monitor<'env, 'r, T> { - Monitor::new(self) - } } impl<'env, T: JavaDebug> Debug for Ref<'env, T> { @@ -149,9 +153,9 @@ impl<'env, T: JavaDisplay> Display for Ref<'env, T> { } } -/// A non-null reference of a Java object locked with the JNI monitor mechanism, providing *limited* thread safety. +/// A borrowed [Ref] of a Java object locked with the JNI monitor mechanism, providing *limited* thread safety. /// -/// It is important to drop the monitor or call [Monitor::unlock()] when appropriate. +/// **It is imposible to be FFI safe.** It is important to drop the monitor or call [Monitor::unlock()] when appropriate. /// /// Limitations: /// @@ -159,50 +163,45 @@ impl<'env, T: JavaDisplay> Display for Ref<'env, T> { /// - 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, 'r, T: ReferenceType> { - inner: Option<&'r Ref<'env, T>>, +pub struct Monitor<'env, T: ReferenceType> { + inner: &'env Ref<'env, T>, } -impl<'env, 'r, T: ReferenceType> Monitor<'env, 'r, T> { - fn new(reference: &'r Ref<'env, T>) -> Self { +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: Some(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(mut self) -> &'r Ref<'env, T> { - self.unlock_inner(); - self.inner.take().unwrap() - } - - fn unlock_inner(&mut self) { - if let Some(inner) = self.inner.as_ref() { - let env = inner.env; - let jnienv = env.as_raw(); - let result = unsafe { ((**jnienv).v1_2.MonitorExit)(jnienv, 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) } - ); - } - } + 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, 'r, T: ReferenceType> Deref for Monitor<'env, 'r, T> { +impl<'env, T: ReferenceType> Deref for Monitor<'env, T> { type Target = Ref<'env, T>; fn deref(&self) -> &Self::Target { - self.inner.as_ref().unwrap() + self.inner } } -impl<'env, 'r, T: ReferenceType> Drop for Monitor<'env, 'r, T> { +impl<'env, T: ReferenceType> Drop for Monitor<'env, T> { fn drop(&mut self) { - self.unlock_inner(); + 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) } + ); + } } }