Repository: leptos-rs/leptos (rust)

Issue:
Need a way to compare whether two callbacks are the same
**Is your feature request related to a problem? Please describe.**
I need a way to compare two callbacks to see if they reference the same underlying function. Currently, there's no public method for verifying equality, and the internal `.0` field of `Callback` and `UnsyncCallback` is private.

**Describe the solution you'd like**
A `matches` method on `Callback` and `UnsyncCallback` that returns a boolean indicating whether two callbacks wrap the same function pointer. Since `.0` is private, this new public method is needed to perform that comparison without exposing the internals.

**Describe alternatives you've considered**
- Exposing `.0` directly, but that would break encapsulation.
- Storing and comparing function pointers manually in user code, which is cumbersome and error-prone.

**Additional context**
A built-in `matches` method would help in scenarios like deduplicating callbacks, ensuring no duplicate references are stored, and making certain state updates more straightforward.


Pull request:
feat(callback): implement `matches` method for Callback and UnsyncCallback
### Description
This PR adds a new `matches` method to both `Callback` (uses `Arc`) and `UnsyncCallback` (uses `Rc`). This method checks if two callbacks refer to the same underlying function pointer by using `Arc::ptr_eq` or `Rc::ptr_eq`.

### Motivation and Context
- Simplifies equality checks on callbacks for deduplication or state updates.
- Matches can now be tested with an idiomatic `.matches(other_callback)` call.

### What was done
- Added `matches(&self, other: &Self) -> bool` for `Callback` using `try_with_value`.
- Added `matches(&self, other: &Self) -> bool` for `UnsyncCallback` using `with_value`.
- Included new unit tests confirming correct behaviour for both same and different callbacks.

### How to Test
1. Run `cargo test` to see new test cases:
   - `callback_matches_same`
   - `callback_matches_different`
   - `unsync_callback_matches_same`
   - `unsync_callback_matches_different`
2. Confirm all existing tests continue to pass.

### Screenshots / Proof of Testing
```rust
zsh ❮ cargo test --color=always --lib callback::tests
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.34s
     Running unittests src/lib.rs (/Users/geoffrey/Projects/leptos/target/debug/deps/leptos-63e467dbc97e4815)

running 8 tests
test callback::tests::clone_unsync_callback ... ok
test callback::tests::clone_callback ... ok
test callback::tests::runback_from ... ok
test callback::tests::unsync_callback_matches_different ... ok
test callback::tests::unsync_callback_matches_same ... ok
test callback::tests::callback_matches_same ... ok
test callback::tests::callback_matches_different ... ok
test callback::tests::sync_callback_from ... ok

test result: ok. 8 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
```

### Related Issues
Closes #3519.

### Types of changes
- [x] New feature (non-breaking change which adds functionality)

### Checklist
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.


Patch:
diff --git a/leptos/src/callback.rs b/leptos/src/callback.rs
index f4e43bc9..3db4d24c 100644
--- a/leptos/src/callback.rs
+++ b/leptos/src/callback.rs
@@ -80,6 +80,16 @@ impl<In, Out> UnsyncCallback<In, Out> {
     {
         Self(StoredValue::new_local(Rc::new(f)))
     }
+
+    /// Returns `true` if both callbacks wrap the same underlying function pointer.
+    #[inline]
+    pub fn matches(&self, other: &Self) -> bool {
+        self.0.with_value(|self_value| {
+            other
+                .0
+                .with_value(|other_value| Rc::ptr_eq(self_value, other_value))
+        })
+    }
 }
 
 impl<In: 'static, Out: 'static> Callable<In, Out> for UnsyncCallback<In, Out> {
@@ -212,6 +222,19 @@ impl<In: 'static, Out: 'static> Callback<In, Out> {
     {
         Self(StoredValue::new(Arc::new(fun)))
     }
+
+    /// Returns `true` if both callbacks wrap the same underlying function pointer.
+    #[inline]
+    pub fn matches(&self, other: &Self) -> bool {
+        self.0
+            .try_with_value(|self_value| {
+                other.0.try_with_value(|other_value| {
+                    Arc::ptr_eq(self_value, other_value)
+                })
+            })
+            .flatten()
+            .unwrap_or(false)
+    }
 }
 
 #[cfg(test)]
@@ -246,4 +269,32 @@ mod tests {
         let _callback: UnsyncCallback<(i32, String), String> =
             (|num, s| format!("{num} {s}")).into();
     }
+
+    #[test]
+    fn callback_matches_same() {
+        let callback1 = Callback::new(|x: i32| x * 2);
+        let callback2 = callback1.clone();
+        assert!(callback1.matches(&callback2));
+    }
+
+    #[test]
+    fn callback_matches_different() {
+        let callback1 = Callback::new(|x: i32| x * 2);
+        let callback2 = Callback::new(|x: i32| x + 1);
+        assert!(!callback1.matches(&callback2));
+    }
+
+    #[test]
+    fn unsync_callback_matches_same() {
+        let callback1 = UnsyncCallback::new(|x: i32| x * 2);
+        let callback2 = callback1.clone();
+        assert!(callback1.matches(&callback2));
+    }
+
+    #[test]
+    fn unsync_callback_matches_different() {
+        let callback1 = UnsyncCallback::new(|x: i32| x * 2);
+        let callback2 = UnsyncCallback::new(|x: i32| x + 1);
+        assert!(!callback1.matches(&callback2));
+    }
 }
diff --git a/reactive_graph/src/computed.rs b/reactive_graph/src/computed.rs
index f1d48774..aba2cdb9 100644
--- a/reactive_graph/src/computed.rs
+++ b/reactive_graph/src/computed.rs
@@ -15,7 +15,6 @@ use crate::{
 };
 pub use arc_memo::*;
 pub use async_derived::*;
-pub(crate) use inner::MemoInner;
 pub use memo::*;
 pub use selector::*;
 
diff --git a/reactive_graph/src/computed/arc_memo.rs b/reactive_graph/src/computed/arc_memo.rs
index 490f0975..007b06de 100644
--- a/reactive_graph/src/computed/arc_memo.rs
+++ b/reactive_graph/src/computed/arc_memo.rs
@@ -12,11 +12,10 @@ use crate::{
     traits::{DefinedAt, Get, IsDisposed, ReadUntracked},
 };
 use core::fmt::Debug;
-use or_poisoned::OrPoisoned;
 use std::{
     hash::Hash,
     panic::Location,
-    sync::{Arc, RwLock, Weak},
+    sync::{Arc, Weak},
 };
 
 /// An efficient derived reactive value based on other reactive values.
@@ -95,7 +94,7 @@ where
 {
     #[cfg(any(debug_assertions, leptos_debuginfo))]
     defined_at: &'static Location<'static>,
-    inner: Arc<RwLock<MemoInner<T, S>>>,
+    inner: Arc<MemoInner<T, S>>,
 }
 
 impl<T: 'static> ArcMemo<T, SyncStorage>
@@ -161,7 +160,7 @@ where
                 Weak::clone(weak) as Weak<dyn Subscriber + Send + Sync>,
             );
 
-            RwLock::new(MemoInner::new(Arc::new(fun), subscriber))
+            MemoInner::new(Arc::new(fun), subscriber)
         });
         Self {
             #[cfg(any(debug_assertions, leptos_debuginfo))]
@@ -312,15 +311,11 @@ where
     S: Storage<T>,
 {
     fn add_source(&self, source: AnySource) {
-        self.inner.write().or_poisoned().sources.insert(source);
+        self.inner.add_source(source);
     }
 
     fn clear_sources(&self, subscriber: &AnySubscriber) {
-        self.inner
-            .write()
-            .or_poisoned()
-            .sources
-            .clear_sources(subscriber);
+        self.inner.clear_sources(subscriber);
     }
 }
 
@@ -328,15 +323,15 @@ impl<T: 'static, S> ReadUntracked for ArcMemo<T, S>
 where
     S: Storage<T>,
 {
-    type Value = ReadGuard<T, Mapped<Plain<MemoInner<T, S>>, T>>;
+    type Value = ReadGuard<T, Mapped<Plain<Option<S::Wrapped>>, T>>;
 
     fn try_read_untracked(&self) -> Option<Self::Value> {
         self.update_if_necessary();
 
-        Mapped::try_new(Arc::clone(&self.inner), |t| {
+        Mapped::try_new(Arc::clone(&self.inner.value), |t| {
             // safe to unwrap here because update_if_necessary
             // guarantees the value is Some
-            t.value.as_ref().unwrap().as_borrowed()
+            t.as_ref().unwrap().as_borrowed()
         })
         .map(ReadGuard::new)
     }
diff --git a/reactive_graph/src/computed/inner.rs b/reactive_graph/src/computed/inner.rs
index 4f6bb3c5..9b451859 100644
--- a/reactive_graph/src/computed/inner.rs
+++ b/reactive_graph/src/computed/inner.rs
@@ -15,10 +15,15 @@ pub struct MemoInner<T, S>
 where
     S: Storage<T>,
 {
-    pub(crate) value: Option<S::Wrapped>,
+    /// Must always be aquired *after* the reactivity lock
+    pub(crate) value: Arc<RwLock<Option<S::Wrapped>>>,
     #[allow(clippy::type_complexity)]
     pub(crate) fun: Arc<dyn Fn(Option<T>) -> (T, bool) + Send + Sync>,
     pub(crate) owner: Owner,
+    pub(crate) reactivity: RwLock<MemoInnerReactivity>,
+}
+
+pub(crate) struct MemoInnerReactivity {
     pub(crate) state: ReactiveNodeState,
     pub(crate) sources: SourceSet,
     pub(crate) subscribers: SubscriberSet,
@@ -44,40 +49,44 @@ where
         any_subscriber: AnySubscriber,
     ) -> Self {
         Self {
-            value: None,
+            value: Arc::new(RwLock::new(None)),
             fun,
             owner: Owner::new(),
-            state: ReactiveNodeState::Dirty,
-            sources: Default::default(),
-            subscribers: SubscriberSet::new(),
-            any_subscriber,
+            reactivity: RwLock::new(MemoInnerReactivity {
+                state: ReactiveNodeState::Dirty,
+                sources: Default::default(),
+                subscribers: SubscriberSet::new(),
+                any_subscriber,
+            }),
         }
     }
 }
 
-impl<T: 'static, S> ReactiveNode for RwLock<MemoInner<T, S>>
+impl<T: 'static, S> ReactiveNode for MemoInner<T, S>
 where
     S: Storage<T>,
 {
     fn mark_dirty(&self) {
-        self.write().or_poisoned().state = ReactiveNodeState::Dirty;
+        self.reactivity.write().or_poisoned().state = ReactiveNodeState::Dirty;
         self.mark_subscribers_check();
     }
 
     fn mark_check(&self) {
         {
-            let mut lock = self.write().or_poisoned();
+            let mut lock = self.reactivity.write().or_poisoned();
             if lock.state != ReactiveNodeState::Dirty {
                 lock.state = ReactiveNodeState::Check;
             }
         }
-        for sub in (&self.read().or_poisoned().subscribers).into_iter() {
+        for sub in
+            (&self.reactivity.read().or_poisoned().subscribers).into_iter()
+        {
             sub.mark_check();
         }
     }
 
     fn mark_subscribers_check(&self) {
-        let lock = self.read().or_poisoned();
+        let lock = self.reactivity.read().or_poisoned();
         for sub in (&lock.subscribers).into_iter() {
             sub.mark_check();
         }
@@ -85,7 +94,7 @@ where
 
     fn update_if_necessary(&self) -> bool {
         let (state, sources) = {
-            let inner = self.read().or_poisoned();
+            let inner = self.reactivity.read().or_poisoned();
             (inner.state, inner.sources.clone())
         };
 
@@ -94,32 +103,37 @@ where
             ReactiveNodeState::Dirty => true,
             ReactiveNodeState::Check => (&sources).into_iter().any(|source| {
                 source.update_if_necessary()
-                    || self.read().or_poisoned().state
+                    || self.reactivity.read().or_poisoned().state
                         == ReactiveNodeState::Dirty
             }),
         };
 
         if needs_update {
-            let (fun, value, owner) = {
-                let mut lock = self.write().or_poisoned();
-                (lock.fun.clone(), lock.value.take(), lock.owner.clone())
-            };
+            let fun = self.fun.clone();
+            let owner = self.owner.clone();
+            // No deadlock risk, because we only hold the value lock.
+            let value = self.value.write().or_poisoned().take();
 
             let any_subscriber =
-                { self.read().or_poisoned().any_subscriber.clone() };
+                { self.reactivity.read().or_poisoned().any_subscriber.clone() };
             any_subscriber.clear_sources(&any_subscriber);
             let (new_value, changed) = owner.with_cleanup(|| {
                 any_subscriber
                     .with_observer(|| fun(value.map(StorageAccess::into_taken)))
             });
 
-            let mut lock = self.write().or_poisoned();
-            lock.value = Some(S::wrap(new_value));
-            lock.state = ReactiveNodeState::Clean;
+            // Two locks are aquired, so order matters.
+            let mut reactivity_lock = self.reactivity.write().or_poisoned();
+            {
+                // Safety: Can block endlessly if the user is has a ReadGuard on the value
+                let mut value_lock = self.value.write().or_poisoned();
+                *value_lock = Some(S::wrap(new_value));
+            }
+            reactivity_lock.state = ReactiveNodeState::Clean;
 
             if changed {
-                let subs = lock.subscribers.clone();
-                drop(lock);
+                let subs = reactivity_lock.subscribers.clone();
+                drop(reactivity_lock);
                 for sub in subs {
                     // don't trigger reruns of effects/memos
                     // basically: if one of the observers has triggered this memo to
@@ -128,49 +142,54 @@ where
                         sub.mark_dirty();
                     }
                 }
+            } else {
+                drop(reactivity_lock);
             }
 
             changed
         } else {
-            if let Ok(mut lock) = self.try_write() {
-                lock.state = ReactiveNodeState::Clean;
-            }
+            let mut lock = self.reactivity.write().or_poisoned();
+            lock.state = ReactiveNodeState::Clean;
             false
         }
     }
 }
 
-impl<T: 'static, S> Source for RwLock<MemoInner<T, S>>
+impl<T: 'static, S> Source for MemoInner<T, S>
 where
     S: Storage<T>,
 {
     fn add_subscriber(&self, subscriber: AnySubscriber) {
-        if let Ok(mut lock) = self.try_write() {
-            lock.subscribers.subscribe(subscriber);
-        }
+        let mut lock = self.reactivity.write().or_poisoned();
+        lock.subscribers.subscribe(subscriber);
     }
 
     fn remove_subscriber(&self, subscriber: &AnySubscriber) {
-        self.write()
+        self.reactivity
+            .write()
             .or_poisoned()
             .subscribers
             .unsubscribe(subscriber);
     }
 
     fn clear_subscribers(&self) {
-        self.write().or_poisoned().subscribers.take();
+        self.reactivity.write().or_poisoned().subscribers.take();
     }
 }
 
-impl<T: 'static, S> Subscriber for RwLock<MemoInner<T, S>>
+impl<T: 'static, S> Subscriber for MemoInner<T, S>
 where
     S: Storage<T>,
 {
     fn add_source(&self, source: AnySource) {
-        self.write().or_poisoned().sources.insert(source);
+        self.reactivity.write().or_poisoned().sources.insert(source);
     }
 
     fn clear_sources(&self, subscriber: &AnySubscriber) {
-        self.write().or_poisoned().sources.clear_sources(subscriber);
+        self.reactivity
+            .write()
+            .or_poisoned()
+            .sources
+            .clear_sources(subscriber);
     }
 }
diff --git a/reactive_graph/src/computed/memo.rs b/reactive_graph/src/computed/memo.rs
index 83263b68..fab52f0c 100644
--- a/reactive_graph/src/computed/memo.rs
+++ b/reactive_graph/src/computed/memo.rs
@@ -1,4 +1,4 @@
-use super::{inner::MemoInner, ArcMemo};
+use super::ArcMemo;
 use crate::{
     owner::{ArenaItem, FromLocal, LocalStorage, Storage, SyncStorage},
     signal::{
@@ -306,7 +306,8 @@ where
     T: 'static,
     S: Storage<ArcMemo<T, S>> + Storage<T>,
 {
-    type Value = ReadGuard<T, Mapped<Plain<MemoInner<T, S>>, T>>;
+    type Value =
+        ReadGuard<T, Mapped<Plain<Option<<S as Storage<T>>::Wrapped>>, T>>;
 
     fn try_read_untracked(&self) -> Option<Self::Value> {
         self.inner
diff --git a/reactive_graph/src/wrappers.rs b/reactive_graph/src/wrappers.rs
index 771c79e8..2b9b8455 100644
--- a/reactive_graph/src/wrappers.rs
+++ b/reactive_graph/src/wrappers.rs
@@ -3,7 +3,7 @@
 /// Types that abstract over signals with values that can be read.
 pub mod read {
     use crate::{
-        computed::{ArcMemo, Memo, MemoInner},
+        computed::{ArcMemo, Memo},
         graph::untrack,
         owner::{
             ArcStoredValue, ArenaItem, FromLocal, LocalStorage, Storage,
@@ -1735,22 +1735,40 @@ pub mod read {
     }
 
     /// The content of a [`Signal`] wrapper read guard, variable depending on the signal type.
-    #[derive(Debug)]
     pub enum SignalReadGuard<T: 'static, S: Storage<T>> {
         /// A read signal guard.
         Read(ReadGuard<T, Plain<T>>),
+        #[allow(clippy::type_complexity)]
         /// A memo guard.
-        Memo(ReadGuard<T, Mapped<Plain<MemoInner<T, S>>, T>>),
+        Memo(
+            ReadGuard<T, Mapped<Plain<Option<<S as Storage<T>>::Wrapped>>, T>>,
+        ),
         /// A fake guard for derived signals, the content had to actually be cloned, so it's not a guard but we pretend it is.
         Owned(T),
     }
 
+    impl<T: 'static + std::fmt::Debug, S: Storage<T> + std::fmt::Debug>
+        std::fmt::Debug for SignalReadGuard<T, S>
+    where
+        <S as Storage<T>>::Wrapped: std::fmt::Debug,
+    {
+        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+            match self {
+                Self::Read(arg0) => f.debug_tuple("Read").field(arg0).finish(),
+                Self::Memo(arg0) => f.debug_tuple("Memo").field(arg0).finish(),
+                Self::Owned(arg0) => {
+                    f.debug_tuple("Owned").field(arg0).finish()
+                }
+            }
+        }
+    }
+
     impl<T, S> Clone for SignalReadGuard<T, S>
     where
         S: Storage<T>,
         T: Clone,
         Plain<T>: Clone,
-        Mapped<Plain<MemoInner<T, S>>, T>: Clone,
+        Mapped<Plain<Option<<S as Storage<T>>::Wrapped>>, T>: Clone,
     {
         fn clone(&self) -> Self {
             match self {


Test patch:
diff --git a/reactive_graph/tests/memo.rs b/reactive_graph/tests/memo.rs
index 8eef23dd..6d63471b 100644
--- a/reactive_graph/tests/memo.rs
+++ b/reactive_graph/tests/memo.rs
@@ -444,6 +444,69 @@ fn unsync_derived_signal_and_memo() {
     assert_eq!(f.get_untracked(), 6);
 }
 
+#[cfg(feature = "effects")]
+#[tokio::test]
+async fn test_memo_multiple_read_guards() {
+    // regression test for https://github.com/leptos-rs/leptos/issues/3158
+    let owner = Owner::new();
+    owner.set();
+    use imports::*;
+
+    _ = Executor::init_tokio();
+    let owner = Owner::new();
+    owner.set();
+    task::LocalSet::new()
+        .run_until(async {
+            let memo = Memo::<i32>::new_with_compare(|_| 42, |_, _| true);
+
+            Effect::new(move |_| {
+                let guard_a = memo.read();
+                let guard_b = memo.read();
+                assert_eq!(guard_a, 42);
+                assert_eq!(guard_b, 42);
+            });
+            Executor::tick().await;
+        })
+        .await
+}
+
+#[cfg(feature = "effects")]
+#[tokio::test]
+async fn test_memo_read_guard_held() {
+    // regression test for https://github.com/leptos-rs/leptos/issues/3252
+    let owner = Owner::new();
+    owner.set();
+    use imports::*;
+
+    _ = Executor::init_tokio();
+    let owner = Owner::new();
+    owner.set();
+    task::LocalSet::new()
+        .run_until(async {
+            let source = RwSignal::new(0);
+
+            let directly_derived =
+                Memo::new_with_compare(move |_| source.get(), |_, _| true);
+            let indirect = Memo::new_with_compare(
+                move |_| directly_derived.get(),
+                |_, _| true,
+            );
+
+            Effect::new(move |_| {
+                let direct_value = directly_derived.read();
+                let indirect_value = indirect.get();
+                assert_eq!(direct_value, indirect_value);
+            });
+
+            Executor::tick().await;
+            source.set(1);
+            Executor::tick().await;
+            source.set(2);
+            Executor::tick().await;
+        })
+        .await
+}
+
 #[test]
 fn memo_updates_even_if_not_read_until_later() {
     #![allow(clippy::bool_assert_comparison)]