Add test for notifying and dropping subscriptions in an update cycle

Max Brunsfeld created 3 years ago

Change summary

crates/gpui/src/app.rs                     | 25 ++++++++++++++++++++++++
crates/gpui/src/app/callback_collection.rs | 12 ++--------
2 files changed, 28 insertions(+), 9 deletions(-)

Detailed changes

crates/gpui/src/app.rs

@@ -6097,6 +6097,31 @@ mod tests {
         assert_eq!(view.read(cx).events, ["before notify"]);
     }
 
+    #[crate::test(self)]
+    fn test_notify_and_drop_observe_subscription_in_same_update_cycle(cx: &mut MutableAppContext) {
+        struct Model;
+        impl Entity for Model {
+            type Event = ();
+        }
+
+        let model = cx.add_model(|_| Model);
+        let (_, view) = cx.add_window(Default::default(), |_| TestView::default());
+
+        view.update(cx, |_, cx| {
+            model.update(cx, |_, cx| cx.notify());
+            drop(cx.observe(&model, move |this, _, _| {
+                this.events.push("model notified".into());
+            }));
+            model.update(cx, |_, cx| cx.notify());
+        });
+
+        for _ in 0..3 {
+            model.update(cx, |_, cx| cx.notify());
+        }
+
+        assert_eq!(view.read(cx).events, Vec::<String>::new());
+    }
+
     #[crate::test(self)]
     fn test_dropping_observers(cx: &mut MutableAppContext) {
         struct Model;

crates/gpui/src/app/callback_collection.rs

@@ -58,14 +58,6 @@ impl<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
         }
     }
 
-    pub fn count(&mut self, key: K) -> usize {
-        self.internal
-            .lock()
-            .callbacks
-            .get(&key)
-            .map_or(0, |callbacks| callbacks.len())
-    }
-
     pub fn add_callback(&mut self, key: K, subscription_id: usize, callback: F) {
         let mut this = self.internal.lock();
         if !this.dropped_subscriptions.contains(&(key, subscription_id)) {
@@ -77,7 +69,9 @@ impl<K: Clone + Hash + Eq + Copy, F> CallbackCollection<K, F> {
     }
 
     pub fn remove(&mut self, key: K) {
-        self.internal.lock().callbacks.remove(&key);
+        let callbacks = self.internal.lock().callbacks.remove(&key);
+        // drop these after releasing the lock
+        drop(callbacks);
     }
 
     pub fn emit<C: FnMut(&mut F, &mut MutableAppContext) -> bool>(