From dff44997e41420a1f4249ad3763f6b50224312bd Mon Sep 17 00:00:00 2001 From: Nagata Parama Aptana <61454123+lifers@users.noreply.github.com> Date: Fri, 16 Aug 2024 11:40:54 -0400 Subject: [PATCH] Prefer more explicit drop order --- crates/libs/core/src/event.rs | 71 ++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/crates/libs/core/src/event.rs b/crates/libs/core/src/event.rs index 534fcf0e71..50eca22d1f 100644 --- a/crates/libs/core/src/event.rs +++ b/crates/libs/core/src/event.rs @@ -29,51 +29,54 @@ impl Event { /// Registers a delegate with the event object. pub fn add(&self, delegate: &T) -> Result { - 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.