fix(remo-sdk): release capability handler context on unregister/replace (#60)

* fix(remo-sdk): release capability handler context on unregister/replace

Add an optional `destroy` callback to `remo_register_capability` so the FFI
side can free its retained context when the registration ends — by
unregister, by replacement, or when the registry is dropped. Swift wires it
to `Unmanaged.passRetained.release()` to balance the retain at registration,
fixing a HandlerBox leak that grew with every register/unregister cycle.

Covered by Rust unit tests on `CallbackHandle::drop` and Swift integration
tests in the example app exercising the public Remo.register/unregister API.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(example): drop unused try on non-throwing bridge.run calls

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: yjmeqt

.github/workflows/ci.yml

@@ -70,6 +70,17 @@ jobs:
             xcode-e2e-
       - name: Boot simulator
         run: xcrun simctl boot 'iPhone 17' 2>/dev/null || true
+      - name: Build iOS SDK (simulator)
+        run: ./build-ios.sh sim
+      - name: Run iOS unit tests (RemoExampleFeatureTests)
+        working-directory: examples/ios
+        run: |
+          xcodebuild \
+            -workspace RemoExample.xcworkspace \
+            -scheme RemoExample \
+            -destination 'platform=iOS Simulator,name=iPhone 17' \
+            -only-testing:RemoExampleFeatureTests \
+            test
       - name: Run E2E tests
         run: ./scripts/e2e-test.sh --screenshots --record
       - name: Upload artifacts

crates/remo-sdk/src/ffi.rs

@@ -192,24 +192,44 @@ pub extern "C" fn remo_get_port() -> u16 {
 pub type CapabilityCallback =
     unsafe extern "C" fn(context: *mut std::ffi::c_void, params_json: *const c_char) -> *mut c_char;
 
+/// Optional destroy callback invoked when the registration ends.
+///
+/// Called exactly once per `remo_register_capability` call: when the
+/// capability is unregistered, replaced by another registration of the same
+/// name, or the registry entry is otherwise dropped. Use this to balance any
+/// retain performed on `context` at registration time (e.g.
+/// `Unmanaged.passRetained`).
+///
+/// May be `NULL` if the context does not require cleanup.
+pub type CapabilityDestroy = unsafe extern "C" fn(context: *mut std::ffi::c_void);
+
 /// Register a capability handler from Swift.
 ///
+/// `destroy` (if non-null) is invoked exactly once when the registration ends —
+/// on unregister, on replacement by another registration of the same name, or
+/// when the handler is otherwise dropped from the registry. After that call the
+/// caller may release any resources owned by `context`.
+///
 /// # Safety
 /// - `name` must be a valid null-terminated C string.
-/// - `context` must remain valid for the lifetime of the registration.
+/// - `context` must remain valid until `destroy` is invoked (or, if `destroy`
+///   is null, for the lifetime of the process).
 /// - `callback` must be a valid, thread-safe function pointer.
+/// - `destroy`, if non-null, must be a valid, thread-safe function pointer.
 #[no_mangle]
 pub unsafe extern "C" fn remo_register_capability(
     name: *const c_char,
     context: *mut std::ffi::c_void,
     callback: CapabilityCallback,
+    destroy: Option<CapabilityDestroy>,
 ) {
     let name = CStr::from_ptr(name).to_string_lossy().into_owned();
 
     // Safety: Swift side guarantees context + callback are Send + Sync.
     let handle = CallbackHandle {
         ctx: SendPtr(context),
         cb: callback,
+        destroy,
     };
     // Prevent raw pointer parameters from being captured by the closure below.
     let _ = context;
@@ -294,9 +314,14 @@ unsafe impl Send for SendPtr {}
 unsafe impl Sync for SendPtr {}
 
 /// Wraps the FFI callback context so the closure is Send + Sync.
+///
+/// The optional `destroy` callback is invoked from `Drop` so the context is
+/// released exactly when the registry entry is removed — whether by an
+/// explicit unregister, by replacement, or by dropping the registry itself.
 struct CallbackHandle {
     ctx: SendPtr,
     cb: CapabilityCallback,
+    destroy: Option<CapabilityDestroy>,
 }
 // SAFETY: CallbackHandle's fields (SendPtr + extern "C" fn) are thread-safe per Swift contract.
 unsafe impl Send for CallbackHandle {}
@@ -313,7 +338,137 @@ impl CallbackHandle {
     }
 }
 
+impl Drop for CallbackHandle {
+    fn drop(&mut self) {
+        if let Some(destroy) = self.destroy {
+            // SAFETY: `destroy` was supplied by the FFI caller alongside `ctx`
+            // and is contracted to be safe to call exactly once with that ctx.
+            unsafe { destroy(self.ctx.0) };
+        }
+    }
+}
+
 extern "C" {
     #[link_name = "free"]
     fn libc_free(ptr: *mut std::ffi::c_void);
 }
+
+#[cfg(test)]
+mod tests {
+    //! Tests for the FFI capability lifecycle — specifically that the
+    //! `destroy` callback supplied alongside the context pointer is invoked
+    //! exactly once per registration, regardless of how the registration ends
+    //! (explicit unregister or replacement by another registration of the
+    //! same name).
+    //!
+    //! These tests exercise `CallbackHandle` through `CapabilityRegistry`
+    //! directly rather than the global FFI entry points, since the latter
+    //! share process-wide state.
+    use super::*;
+    use crate::registry::CapabilityRegistry;
+    use std::sync::atomic::{AtomicUsize, Ordering};
+    use std::sync::Arc;
+
+    /// `context` argument is the pointer to a leaked `Arc<AtomicUsize>` —
+    /// reclaim it and bump the destroy count.
+    unsafe extern "C" fn destroy_counter(context: *mut std::ffi::c_void) {
+        let counter = Arc::from_raw(context as *const AtomicUsize);
+        counter.fetch_add(1, Ordering::SeqCst);
+    }
+
+    unsafe extern "C" fn noop_callback(
+        _context: *mut std::ffi::c_void,
+        _params_json: *const c_char,
+    ) -> *mut c_char {
+        std::ptr::null_mut()
+    }
+
+    /// Register a capability whose `destroy` increments `counter` when fired.
+    /// Mirrors the bookkeeping `remo_register_capability` does internally so
+    /// these tests don't depend on the global FFI registry.
+    fn register_with_destroy(reg: &CapabilityRegistry, name: &str, counter: Arc<AtomicUsize>) {
+        let context = Arc::into_raw(counter) as *mut std::ffi::c_void;
+        let handle = CallbackHandle {
+            ctx: SendPtr(context),
+            cb: noop_callback,
+            destroy: Some(destroy_counter),
+        };
+        reg.register_sync(name.to_string(), move |_params| {
+            let _ = &handle;
+            Ok(Value::Null)
+        });
+    }
+
+    #[tokio::test]
+    async fn destroy_fires_on_unregister() {
+        let reg = CapabilityRegistry::new();
+        let counter = Arc::new(AtomicUsize::new(0));
+
+        register_with_destroy(&reg, "cap", Arc::clone(&counter));
+        assert_eq!(
+            counter.load(Ordering::SeqCst),
+            0,
+            "destroy must not fire on register"
+        );
+
+        assert!(reg.unregister("cap"));
+        assert_eq!(
+            counter.load(Ordering::SeqCst),
+            1,
+            "destroy must fire exactly once on unregister"
+        );
+    }
+
+    #[tokio::test]
+    async fn destroy_fires_on_replacement() {
+        let reg = CapabilityRegistry::new();
+        let first = Arc::new(AtomicUsize::new(0));
+        let second = Arc::new(AtomicUsize::new(0));
+
+        register_with_destroy(&reg, "cap", Arc::clone(&first));
+        register_with_destroy(&reg, "cap", Arc::clone(&second));
+
+        assert_eq!(
+            first.load(Ordering::SeqCst),
+            1,
+            "old context must be destroyed when replaced"
+        );
+        assert_eq!(
+            second.load(Ordering::SeqCst),
+            0,
+            "new context must still be alive"
+        );
+
+        assert!(reg.unregister("cap"));
+        assert_eq!(
+            second.load(Ordering::SeqCst),
+            1,
+            "new context must be destroyed on unregister"
+        );
+    }
+
+    #[tokio::test]
+    async fn destroy_fires_on_registry_drop() {
+        let counter = Arc::new(AtomicUsize::new(0));
+        {
+            let reg = CapabilityRegistry::new();
+            register_with_destroy(&reg, "cap", Arc::clone(&counter));
+            assert_eq!(counter.load(Ordering::SeqCst), 0);
+        }
+        assert_eq!(
+            counter.load(Ordering::SeqCst),
+            1,
+            "destroy must fire when registry is dropped"
+        );
+    }
+
+    #[tokio::test]
+    async fn null_destroy_is_safe() {
+        let handle = CallbackHandle {
+            ctx: SendPtr(std::ptr::null_mut()),
+            cb: noop_callback,
+            destroy: None,
+        };
+        drop(handle);
+    }
+}

examples/ios/RemoExamplePackage/Sources/RemoExampleFeature/UIKitDemo/UIKitDemoViewController+Remo.swift

@@ -411,7 +411,7 @@ extension UIKitDemoViewController {
 
                 #remoCap(GridFeedReset.self) { _ in
                     handleGridCapability {
-                        try bridge.run { controller in
+                        bridge.run { controller in
                             controller.handleReset()
                         }
                     }
@@ -435,7 +435,7 @@ extension UIKitDemoViewController {
 
                 #remoCap(GridVisible.self) { _ in
                     handleGridCapability {
-                        try bridge.run { controller in
+                        bridge.run { controller in
                             controller.handleVisible()
                         }
                     }

examples/ios/RemoExamplePackage/Tests/RemoExampleFeatureTests/CapabilityLifecycleTests.swift

@@ -0,0 +1,94 @@
+import Testing
+import RemoSwift
+import Foundation
+
+// End-to-end verification that registering a capability does not leak the
+// Swift HandlerBox or anything captured by the handler closure. Mirrors the
+// Rust-side unit tests for `CallbackHandle::drop` — those prove the destroy
+// callback fires; these prove `swiftCapabilityDestroy` correctly balances
+// `Unmanaged.passRetained`.
+//
+// Lives in the example's test target because RemoSwift's own SPM tests can't
+// run via `xcodebuild test -destination iOS` (the package's macro plugin
+// dependency is host-only). The example workspace handles macros correctly,
+// so we exercise the same Remo.register/unregister API here.
+
+@Suite struct CapabilityLifecycleTests {
+
+    /// Reference type whose `deinit` increments a shared counter. Captured
+    /// inside each registered handler closure so the counter reflects when
+    /// the underlying HandlerBox (and its closure) is released.
+    private final class DeallocSentinel: @unchecked Sendable {
+        let onDealloc: @Sendable () -> Void
+        init(onDealloc: @escaping @Sendable () -> Void) { self.onDealloc = onDealloc }
+        deinit { onDealloc() }
+    }
+
+    /// Register `name` with a handler that captures a fresh sentinel.
+    /// In a separate function so the local `sentinel` reference is gone from
+    /// the caller's stack frame on return — leaving the HandlerBox's retain
+    /// (via `passRetained`) as the only strong reference to it.
+    private func registerWithSentinel(
+        _ name: String,
+        onDealloc: @escaping @Sendable () -> Void
+    ) {
+        let sentinel = DeallocSentinel(onDealloc: onDealloc)
+        Remo.register(name) { _ in
+            _ = sentinel
+            return [:]
+        }
+    }
+
+    @Test
+    func unregisterReleasesHandlerBox() {
+        let counter = AtomicCounter()
+        registerWithSentinel("remo.test.lifecycle.unregister") { counter.increment() }
+
+        #expect(counter.value == 0, "sentinel must remain alive while registered")
+
+        #expect(Remo.unregister("remo.test.lifecycle.unregister"))
+        #expect(counter.value == 1, "sentinel must be released after unregister")
+    }
+
+    @Test
+    func replacementReleasesPreviousHandlerBox() {
+        let counter = AtomicCounter()
+        registerWithSentinel("remo.test.lifecycle.replace") { counter.increment() }
+        #expect(counter.value == 0)
+
+        // Re-registering the same name must release the previous HandlerBox.
+        Remo.register("remo.test.lifecycle.replace") { _ in [:] }
+        #expect(counter.value == 1, "previous registration's HandlerBox must be released on replacement")
+
+        Remo.unregister("remo.test.lifecycle.replace")
+    }
+
+    @Test
+    func repeatedCyclesDoNotAccumulate() {
+        let counter = AtomicCounter()
+        let cycles = 50
+        for i in 0..<cycles {
+            let name = "remo.test.lifecycle.cycle.\(i)"
+            registerWithSentinel(name) { counter.increment() }
+            #expect(Remo.unregister(name))
+        }
+        #expect(counter.value == cycles, "all sentinels must be released across register/unregister cycles")
+    }
+}
+
+/// Thread-safe counter — `deinit` callbacks may run on whichever thread
+/// drops the last reference.
+private final class AtomicCounter: @unchecked Sendable {
+    private let lock = NSLock()
+    private var _value = 0
+
+    var value: Int {
+        lock.lock(); defer { lock.unlock() }
+        return _value
+    }
+
+    func increment() {
+        lock.lock(); defer { lock.unlock() }
+        _value += 1
+    }
+}

swift/RemoSwift/Sources/RemoSwift/Remo.swift

@@ -110,7 +110,7 @@ public final class Remo {
         let context = Unmanaged.passRetained(handlerBox).toOpaque()
 
         name.withCString { namePtr in
-            remo_register_capability(namePtr, context, swiftCapabilityTrampoline)
+            remo_register_capability(namePtr, context, swiftCapabilityTrampoline, swiftCapabilityDestroy)
         }
         #else
         // Host-side package builds only need the macro-facing API surface to exist.
@@ -214,6 +214,15 @@ private final class HandlerBox {
     }
 }
 
+/// Balances the `passRetained` performed in `register(_:handler:)` — invoked
+/// by the Rust side when the registration ends (unregister, replacement, or
+/// registry drop). Without this, the `HandlerBox` would leak for the lifetime
+/// of the process.
+private let swiftCapabilityDestroy: remo_capability_destroy = { context in
+    guard let context = context else { return }
+    Unmanaged<HandlerBox>.fromOpaque(context).release()
+}
+
 /// C-compatible trampoline that bridges Rust -> Swift handler calls.
 private let swiftCapabilityTrampoline: remo_capability_callback = { context, paramsPtr in
     guard let context = context, let paramsPtr = paramsPtr else {

swift/RemoSwift/Sources/RemoSwift/include/remo.h

@@ -20,13 +20,23 @@ void remo_stop(void);
 /// Returns: a null-terminated JSON string allocated with strdup().
 typedef char* (*remo_capability_callback)(void* context, const char* params_json);
 
+/// Optional destroy callback paired with the context pointer.
+///
+/// Invoked exactly once when the registration ends — whether by unregister,
+/// by replacement (re-registering the same name), or by the registry itself
+/// being dropped. Use this to balance any retain performed on `context` at
+/// registration (e.g. Unmanaged.passRetained on Swift).
+typedef void (*remo_capability_destroy)(void* context);
+
 /// Register a capability handler.
 /// - name: null-terminated capability name
 /// - context: opaque pointer passed to callback
 /// - callback: function pointer invoked when the capability is called
+/// - destroy: optional cleanup invoked when the registration ends (may be NULL)
 void remo_register_capability(const char* name,
                               void* context,
-                              remo_capability_callback callback);
+                              remo_capability_callback callback,
+                              remo_capability_destroy destroy);
 
 /// Unregister a capability by name.
 /// Returns true if the capability was found and removed.