From 4bbbe72cbc6fbf09c7a14cbcc14d3d32df12db7b Mon Sep 17 00:00:00 2001 From: Ruihan Li Date: Fri, 26 Dec 2025 11:25:55 +0800 Subject: [PATCH] Fix `accepts_new_subscribers` race --- kernel/src/fs/notify/mod.rs | 27 +++++++++++++++------------ kernel/src/fs/path/dentry.rs | 6 ++---- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/kernel/src/fs/notify/mod.rs b/kernel/src/fs/notify/mod.rs index fad8b9052..036258ae6 100644 --- a/kernel/src/fs/notify/mod.rs +++ b/kernel/src/fs/notify/mod.rs @@ -35,13 +35,9 @@ pub struct FsEventPublisher { impl Debug for FsEventPublisher { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - let subscribers = self.subscribers.read(); - write!( - f, - "FsEventPublisher: num_subscribers: {}", - subscribers.len() - )?; - Ok(()) + f.debug_struct("FsEventPublisher") + .field("num_subscribers", &self.subscribers.read().len()) + .finish_non_exhaustive() } } @@ -62,10 +58,12 @@ impl FsEventPublisher { /// Adds a subscriber to this publisher. pub fn add_subscriber(&self, subscriber: Arc) -> bool { - if !self.accepts_new_subscribers.load(Ordering::Acquire) { + let mut subscribers = self.subscribers.write(); + // This check must be done after locking `self.subscribers.write()` to avoid race + // conditions. + if !self.accepts_new_subscribers.load(Ordering::Relaxed) { return false; } - let mut subscribers = self.subscribers.write(); let current = self.all_interesting_events.load(Ordering::Relaxed); self.all_interesting_events .store(current | subscriber.interesting_events(), Ordering::Relaxed); @@ -101,9 +99,14 @@ impl FsEventPublisher { num_subscribers } - /// Forbids new subscribers from attaching to this publisher. - pub fn disable_new_subscribers(&self) { - self.accepts_new_subscribers.store(false, Ordering::Release); + /// Forbids new subscribers from attaching to this publisher and removes all existing + /// subscribers. + pub fn disable_new_and_remove_subscribers(&self) -> usize { + // Do this before calling `remove_all_subscribers` so that the `self.subscribers.write()` + // lock will synchronize this variable. + self.accepts_new_subscribers.store(false, Ordering::Relaxed); + + self.remove_all_subscribers() } /// Broadcasts an event to all the subscribers of this publisher. diff --git a/kernel/src/fs/path/dentry.rs b/kernel/src/fs/path/dentry.rs index f7f4667ba..45fbb75c9 100644 --- a/kernel/src/fs/path/dentry.rs +++ b/kernel/src/fs/path/dentry.rs @@ -279,8 +279,7 @@ impl Dentry { // `FsEventPublisher` instance on a dying inode. However, it isn't possible because we // need to disable new subscribers. let publisher = child_inode.fs_event_publisher_or_init(); - publisher.disable_new_subscribers(); - let removed_nr_subscribers = publisher.remove_all_subscribers(); + let removed_nr_subscribers = publisher.disable_new_and_remove_subscribers(); child_inode .fs() .fs_event_subscriber_stats() @@ -332,8 +331,7 @@ impl Dentry { // `FsEventPublisher` on a dying inode. However, it isn't possible because we need to // disable new subscribers. let publisher = child_inode.fs_event_publisher_or_init(); - publisher.disable_new_subscribers(); - let removed_nr_subscribers = publisher.remove_all_subscribers(); + let removed_nr_subscribers = publisher.disable_new_and_remove_subscribers(); child_inode .fs() .fs_event_subscriber_stats()