Prefer more explicit drop order

microsoft · Aug 16, 2024 · dff4499 · dff4499
1 parent 520b595
commit dff4499
Showing 1 changed file with 37 additions and 34 deletions.
diff --git a/crates/libs/core/src/event.rs b/crates/libs/core/src/event.rs
@@ -29,51 +29,54 @@ impl<T: Interface> Event<T> {
 
     /// Registers a delegate with the event object.
     pub fn add(&self, delegate: &T) -> Result<i64> {
-        let mut _lock_free_drop = None;
-        Ok({
-            let mut guard = self.delegates.write().unwrap();
-            let new_delegate = Delegate::new(delegate)?;
-            let token = new_delegate.to_token();
-            let new_iter = once(new_delegate);
-
-            let new_list = if let Some(old_delegates) = guard.as_ref() {
-                Arc::from_iter(old_delegates.iter().cloned().chain(new_iter))
-            } else {
-                Arc::from_iter(new_iter)
-            };
+        let mut guard = self.delegates.write().unwrap();
+        let new_delegate = Delegate::new(delegate)?;
+        let token = new_delegate.to_token();
+        let new_iter = once(new_delegate);
+
+        let new_list = if let Some(old_delegates) = guard.as_ref() {
+            Arc::from_iter(old_delegates.iter().cloned().chain(new_iter))
+        } else {
+            Arc::from_iter(new_iter)
+        };
+
+        let old_list = guard.replace(new_list);
+        drop(guard);
+        drop(old_list); // drop the old delegates _after_ releasing lock
 
-            _lock_free_drop = guard.replace(new_list);
-            token
-        })
+        Ok(token)
     }
 
     /// Revokes a delegate's registration from the event object.
     pub fn remove(&self, token: i64) {
-        let mut _lock_free_drop = None;
-        {
-            let mut guard = self.delegates.write().unwrap();
-            if let Some(old_delegates) = guard.as_ref() {
-                // `self.delegates` is only modified if the token is found.
-                if let Some(i) = old_delegates
-                    .iter()
-                    .position(|old_delegate| old_delegate.to_token() == token)
-                {
-                    let new_list = Arc::from_iter(
-                        old_delegates[..i]
-                            .iter()
-                            .chain(old_delegates[i + 1..].iter())
-                            .cloned(),
-                    );
-
-                    _lock_free_drop = guard.replace(new_list);
-                }
+        let mut guard = self.delegates.write().unwrap();
+        let mut old_list = None;
+        if let Some(old_delegates) = guard.as_ref() {
+            // `self.delegates` is only modified if the token is found.
+            if let Some(i) = old_delegates
+                .iter()
+                .position(|old_delegate| old_delegate.to_token() == token)
+            {
+                let new_list = Arc::from_iter(
+                    old_delegates[..i]
+                        .iter()
+                        .chain(old_delegates[i + 1..].iter())
+                        .cloned(),
+                );
+
+                old_list = guard.replace(new_list);
             }
         }
+        drop(guard);
+        drop(old_list); // drop the old delegates _after_ releasing lock
     }
 
     /// Clears the event, removing all delegates.
     pub fn clear(&self) {
-        let _lock_free_drop = self.delegates.write().unwrap().take();
+        let mut guard = self.delegates.write().unwrap();
+        let old_list = guard.take();
+        drop(guard);
+        drop(old_list); // drop the old delegates _after_ releasing lock
     }
 
     /// Invokes all of the event object's registered delegates with the provided callback.