From 85e34749e3ee4b17524fe97263154f441d6515bf Mon Sep 17 00:00:00 2001 From: John Nunley Date: Mon, 27 May 2024 15:18:47 -0700 Subject: [PATCH] bugfix: Disable early-out notify optimization The notify() function contains two optimizations: - If the `inner` field is not yet initialized (i.e. no listeners have been created yet), it returns `0` from the function early. - If the atomic `notified` field indicates that the current notification would do nothing, it returns `0` early as well. `loom` testing has exposed race conditions in these optimizations that can cause notifications to be missed, leading to deadlocks. This commit removes these optimizations in the hope of preventing these deadlocks. Ideally we can restore them in the future. Closes #138 cc #130 and #133 Signed-off-by: John Nunley --- src/lib.rs | 19 +++---------------- src/no_std.rs | 5 ----- src/std.rs | 5 ----- 3 files changed, 3 insertions(+), 26 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8ab3594..11b8404 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -111,7 +111,7 @@ use sync::Arc; #[cfg(not(loom))] use sync::WithMut; -use notify::{Internal, NotificationPrivate}; +use notify::NotificationPrivate; pub use notify::{IntoNotification, Notification}; /// Inner state of [`Event`]. @@ -437,21 +437,8 @@ impl Event { // Make sure the notification comes after whatever triggered it. notify.fence(notify::Internal::new()); - if let Some(inner) = self.try_inner() { - let limit = if notify.is_additional(Internal::new()) { - usize::MAX - } else { - notify.count(Internal::new()) - }; - - // Notify if there is at least one unnotified listener and the number of notified - // listeners is less than `limit`. - if inner.needs_notification(limit) { - return inner.notify(notify); - } - } - - 0 + let inner = unsafe { &*self.inner() }; + inner.notify(notify) } /// Return a reference to the inner state if it has been initialized. diff --git a/src/no_std.rs b/src/no_std.rs index 527fa9c..a6a2f25 100644 --- a/src/no_std.rs +++ b/src/no_std.rs @@ -45,11 +45,6 @@ impl crate::Inner { drop(self.try_lock()); } - pub(crate) fn needs_notification(&self, _limit: usize) -> bool { - // TODO: Figure out a stable way to do this optimization. - true - } - /// Add a new listener to the list. /// /// Does nothing if the list is already registered. diff --git a/src/std.rs b/src/std.rs index 0d39833..c509ebc 100644 --- a/src/std.rs +++ b/src/std.rs @@ -64,11 +64,6 @@ impl crate::Inner { } } - /// Whether or not this number of listeners would lead to a notification. - pub(crate) fn needs_notification(&self, limit: usize) -> bool { - self.notified.load(Ordering::Acquire) < limit - } - /// Add a new listener to the list. pub(crate) fn insert(&self, mut listener: Pin<&mut Option>>) { let mut inner = self.lock();