removed arc from keyexpr

Author: milyin

zenoh-flat/src/keyexpr.rs

@@ -11,39 +11,33 @@
 //   ZettaScale Zenoh Team, <zenoh@zettascale.tech>
 //
 
-use std::sync::Arc;
-
 use crate::{errors::ZResult, zerror};
 use prebindgen_proc_macro::prebindgen;
 use zenoh::key_expr::{KeyExpr as ZKeyExpr, SetIntersectionLevel};
 
 /// Universal key-expression handle shared across the flat layer.
 ///
 /// `ptr` is `None` for string-only expressions built via [`try_from`] /
-/// [`autocanonize`].  `Some(md)` holds an `Arc<ZKeyExpr<'static>>` wrapped
-/// in `ManuallyDrop` so it never auto-drops when the struct is destroyed:
-/// the caller is responsible for releasing the Arc either by calling
-/// `drop_key_expr` / `undeclare_key_expr` (session-declared) or simply
-/// letting the undeclared variant be discarded (no-op).
+/// [`autocanonize`]. `Some(md)` holds a session-declared zenoh
+/// `ZKeyExpr<'static>`.
 ///
 /// Field order (`ptr` first) matches the `KeyExpr(ptr: Long, string: String)`
 /// Kotlin constructor so positional call sites need no update.
 #[prebindgen_proc_macro::prebindgen]
 #[derive(Debug)]
 pub struct KeyExpr {
-    pub ptr: Option<Arc<ZKeyExpr<'static>>>,
+    pub ptr: Option<ZKeyExpr<'static>>,
     pub string: String,
 }
 
 impl KeyExpr {
     /// Materialize a borrowed zenoh `KeyExpr` for one-shot zenoh calls.
     ///
-    /// When `ptr != 0`, bumps the Arc strong count, clones the inner
-    /// `ZKeyExpr`, then drops the temporary Arc (net count unchanged).
+    /// When `ptr != 0`, clones the stored `ZKeyExpr`.
     /// When `ptr == 0`, constructs from the already-validated string.
     pub(crate) fn as_zenoh(&self) -> ZKeyExpr<'static> {
         if let Some(md) = &self.ptr {
-            md.as_ref().clone()
+            md.clone()
         } else {
             // SAFETY: every `KeyExpr` is built via the validating
             // constructors (`try_from`, `autocanonize`, join, concat).
@@ -105,7 +99,7 @@ pub fn join(a: &KeyExpr, other: String) -> ZResult<KeyExpr> {
         .map_err(|err| zerror!(err))?;
     let string = joined.to_string();
     let ptr = if a.ptr.is_some() {
-        Some(Arc::new(ZKeyExpr::from(joined)))
+        Some(ZKeyExpr::from(joined))
     } else {
         None
     };
@@ -123,22 +117,18 @@ pub fn concat(a: &KeyExpr, other: String) -> ZResult<KeyExpr> {
         .map_err(|err| zerror!(err))?;
     let string = concatenated.to_string();
     let ptr = if a.ptr.is_some() {
-        Some(Arc::new(ZKeyExpr::from(concatenated)))
+        Some(ZKeyExpr::from(concatenated))
     } else {
         None
     };
     Ok(KeyExpr { ptr, string })
 }
 
 /// Drop a [`KeyExpr`] handle obtained from a session-declared key
-/// expression. When `ptr != 0`, takes ownership of the `Arc<ZKeyExpr>`
-/// (decrementing the strong count). String-only expressions are a no-op.
+/// expression. With value-owned storage, this is always a no-op because the
+/// wrapper drops naturally when consumed.
 #[prebindgen]
 pub fn drop_key_expr(key_expr: KeyExpr) -> ZResult<()> {
-    if let Some(ptr) = key_expr.ptr {
-        // SAFETY: `ptr` was produced by `Arc::into_raw`; taking ownership
-        // here and letting the Arc drop releases the Java-side strong ref.
-        drop(ptr);
-    }
+    let _ = key_expr;
     Ok(())
 }

zenoh-flat/src/session.rs

@@ -13,7 +13,6 @@
 
 use crate::keyexpr::KeyExpr;
 use crate::{errors::ZResult, zerror};
-use std::sync::Arc;
 use std::time::Duration;
 use tracing::{error, trace};
 
@@ -110,8 +109,7 @@ pub fn declare_key_expr(session: &Session, key_expr: String) -> ZResult<KeyExpr>
         .map(|ke| {
             trace!("Declared key expression '{}'.", key_expr_clone);
             KeyExpr {
-                // SAFETY: transfers Arc ownership to the Java caller.
-                ptr: Some(Arc::new(ke)),
+                ptr: Some(ke),
                 string: key_expr,
             }
         })

zenoh-jni/build.rs

@@ -87,8 +87,7 @@ macro_rules! decode_option_arc_from_raw {
                 if #input != 0 {
                     Some(unsafe {
                         let raw = #input as *const $inner;
-                        std::sync::Arc::increment_strong_count(raw);
-                        std::sync::Arc::from_raw(raw)
+                        (*raw).clone()
                     })
                 } else {
                     None
@@ -122,6 +121,26 @@ macro_rules! encode_arc_into_raw {
     };
 }
 
+/// Encode an `Option<T>` (where `T: Clone`) into a `jlong` Arc-handle.
+///
+/// `Some(v)` becomes `Arc::into_raw(Arc::new(v.clone())) as i64`.
+/// `None` maps to `0`.
+macro_rules! encode_option_clone_into_arc_raw_jlong {
+    () => {
+        OutputFn::new(|output: Option<&syn::Ident>| -> TokenStream {
+            match output {
+                Some(output) => quote! {
+                    #output
+                        .as_ref()
+                        .map(|value| std::sync::Arc::into_raw(std::sync::Arc::new(value.clone())) as i64)
+                        .unwrap_or(0)
+                },
+                None => quote! { 0 },
+            }
+        })
+    };
+}
+
 /// Emit `<result> as <wire>` on success and `<on_err> as <wire>` on the
 /// throw-path. Used for primitive wire types that map straight from the
 /// Rust return value (e.g. `bool` → `jboolean`, `i32` → `jint`).
@@ -182,24 +201,15 @@ fn shared_bindings() -> TypeRegistry {
         // FlatKeyExpr (zenoh_flat::keyexpr::KeyExpr) — auto-generated as a
         // Kotlin data class; `ptr: Long` carries the raw Arc pointer (0 =
         // string-only). The flat Rust struct now stores
-        // `Option<Arc<ZKeyExpr<'static>>>`, so decode the primitive field
-        // into an owned `Arc` only when a non-zero pointer is present.
+        // `Option<ZKeyExpr<'static>>`, so decode the primitive field by
+        // temporarily materializing an Arc-handle (from raw), cloning the
+        // inner key expression, then dropping the temporary Arc.
         //
         // `"KeyExpr"` (by-value) is auto-registered by the struct_conv pass
         // below, so only the borrow and return variants need manual entries.
-        .type_pair("Option<Arc<ZKeyExpr<'static>>>", "jni::sys::jlong")
+        .type_pair("Option<ZKeyExpr<'static>>", "jni::sys::jlong")
         .input(decode_option_arc_from_raw!(zenoh::key_expr::KeyExpr<'static>))
-        .output(OutputFn::new(|output: Option<&syn::Ident>| -> TokenStream {
-            match output {
-                Some(output) => quote! {
-                    #output
-                        .as_ref()
-                        .map(|ptr| std::sync::Arc::into_raw(ptr.clone()) as i64)
-                        .unwrap_or(0)
-                },
-                None => quote! { 0 },
-            }
-        }))
+        .output(encode_option_clone_into_arc_raw_jlong!())
         .type_pair("&KeyExpr", "jni::objects::JObject")
         .input(decode_env_ref_mut!(decode_KeyExpr))
         .type_pair("ZResult<KeyExpr>", "jni::sys::jobject")
@@ -287,7 +297,7 @@ fn shared_kotlin_types() -> KotlinTypeMap {
         .add("QueryTarget", "Int")
         .add("ConsolidationMode", "Int")
         .add("ReplyKeyExpr", "Int")
-        .add("Option<Arc<ZKeyExpr<'static>>>", "Long")
+        .add("Option<ZKeyExpr<'static>>", "Long")
         .add("&KeyExpr", "KeyExpr")
         .add("ZResult<KeyExpr>", "KeyExpr")
         .add("ZResult<SetIntersectionLevel>", "Int")

zenoh-jni/src/key_expr.rs

@@ -22,13 +22,18 @@ use zenoh_flat::keyexpr::KeyExpr as FlatKeyExpr;
 
 use crate::errors::ZResult;
 
+unsafe fn clone_key_expr_from_arc_ptr(ptr: i64) -> ZResult<ZKeyExpr<'static>> {
+    let raw = ptr as *const ZKeyExpr<'static>;
+    Ok((*raw).clone())
+}
+
 /// Encode a [`FlatKeyExpr`] as a freshly-constructed
 /// `io.zenoh.jni.KeyExpr` Java object. When `ptr` is non-zero, the strong
 /// `Arc` reference (already owned by this value) is transferred to Java.
 pub(crate) fn encode_jni_keyexpr(env: &mut JNIEnv, k: FlatKeyExpr) -> ZResult<jobject> {
     let raw_ptr = k
         .ptr
-        .map(|ptr| Arc::into_raw(ptr) as i64)
+        .map(|key_expr| Arc::into_raw(Arc::new(key_expr)) as i64)
         .unwrap_or(0);
     let jstr = env
         .new_string(k.string)
@@ -58,10 +63,7 @@ pub(crate) unsafe fn decode_jni_key_expr(
         .and_then(|v| v.j())
         .map_err(|err| zerror!("KeyExpr.ptr: {}", err))?;
     if ptr != 0 {
-        let raw = ptr as *const ZKeyExpr<'static>;
-        Arc::increment_strong_count(raw);
-        let arc = Arc::from_raw(raw);
-        Ok((*arc).clone())
+        clone_key_expr_from_arc_ptr(ptr)
     } else {
         let str_obj = env
             .get_field(obj, "string", "Ljava/lang/String;")