Notify `DELETE_SELF`/`IGNORED` correctly

This commit is contained in:
Ruihan Li 2026-01-05 09:20:51 +08:00
parent e6da21b559
commit 6255837b7f
5 changed files with 110 additions and 96 deletions

View File

@ -169,19 +169,9 @@ impl InotifyFile {
let subscriber = inotify_subscriber.clone() as Arc<dyn FsEventSubscriber>;
let inode = path.inode();
if !inode
inode
.fs_event_publisher_or_init()
.add_subscriber(subscriber)
{
// FIXME: This can be triggered by `unlink()` race conditions or
// `inotify_add_watch("/proc/self/fd/[fd]")`, where `[fd]` is a file descriptor
// pointing to a deleted file. Although Linux allows this, we don't support it, so an
// error is returned here.
return_errno_with_message!(
Errno::ENOENT,
"adding an inotify watch to a deleted inode is not supported yet"
);
}
.add_subscriber(subscriber);
let wd = inotify_subscriber.wd();
let entry = SubscriberEntry {

View File

@ -3,7 +3,7 @@
use alloc::{sync::Arc, vec::Vec};
use core::{
any::Any,
sync::atomic::{AtomicBool, AtomicU32, Ordering},
sync::atomic::{AtomicU32, Ordering},
};
use atomic_integer_wrapper::define_atomic_version_of_integer_like_type;
@ -29,8 +29,8 @@ pub struct FsEventPublisher {
subscribers: RwLock<Vec<Arc<dyn FsEventSubscriber>>>,
/// All interesting FS event types (aggregated from all subscribers).
all_interesting_events: AtomicFsEvents,
/// Whether this publisher still accepts new subscribers.
accepts_new_subscribers: AtomicBool,
/// FS events to be published when dropped.
events_on_drop: AtomicFsEvents,
/// A weak reference to the file system.
fs: Weak<dyn FileSystem>,
}
@ -43,26 +43,40 @@ impl Debug for FsEventPublisher {
}
}
impl Drop for FsEventPublisher {
fn drop(&mut self) {
let subscribers = self.subscribers.get_mut();
let num_subscribers = subscribers.len();
let events_on_drop = self.events_on_drop.load(Ordering::Relaxed);
for subscriber in subscribers.drain(..) {
if !events_on_drop.is_empty() {
subscriber.deliver_event(events_on_drop, None);
}
subscriber.deliver_event(FsEvents::IN_IGNORED, None);
}
if let Some(fs) = self.fs.upgrade() {
fs.fs_event_subscriber_stats()
.remove_subscribers(num_subscribers);
}
}
}
impl FsEventPublisher {
pub fn new(fs: Weak<dyn FileSystem>) -> Self {
Self {
subscribers: RwLock::new(Vec::new()),
all_interesting_events: AtomicFsEvents::new(FsEvents::empty()),
accepts_new_subscribers: AtomicBool::new(true),
events_on_drop: AtomicFsEvents::new(FsEvents::empty()),
fs,
}
}
/// Adds a subscriber to this publisher.
pub fn add_subscriber(&self, subscriber: Arc<dyn FsEventSubscriber>) -> bool {
pub fn add_subscriber(&self, subscriber: Arc<dyn FsEventSubscriber>) {
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 current = self.all_interesting_events.load(Ordering::Relaxed);
self.all_interesting_events
.store(current | subscriber.interesting_events(), Ordering::Relaxed);
@ -72,8 +86,6 @@ impl FsEventPublisher {
if let Some(fs) = self.fs.upgrade() {
fs.fs_event_subscriber_stats().add_subscriber();
}
true
}
/// Removes a subscriber from this publisher.
@ -95,38 +107,6 @@ impl FsEventPublisher {
removed
}
/// Removes all subscribers from this publisher.
pub fn remove_all_subscribers(&self) -> usize {
let mut subscribers = self.subscribers.write();
for subscriber in subscribers.iter() {
subscriber.deliver_event(FsEvents::IN_IGNORED, None);
}
let num_subscribers = subscribers.len();
subscribers.clear();
self.all_interesting_events
.store(FsEvents::empty(), Ordering::Relaxed);
if let Some(fs) = self.fs.upgrade() {
fs.fs_event_subscriber_stats()
.remove_subscribers(num_subscribers);
}
num_subscribers
}
/// 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.
pub fn publish_event(&self, events: FsEvents, name: Option<String>) {
let interesting = self.all_interesting_events.load(Ordering::Relaxed);
@ -179,6 +159,17 @@ impl FsEventPublisher {
.store(new_events, Ordering::Relaxed);
}
/// Sets the events to be published when the publisher is dropped.
///
/// [`FsEvents::IN_IGNORED`] always follows the specified FS events. If the
/// specified events are empty, only [`FsEvents::IN_IGNORED`] will be published
/// when the publisher is dropped.
///
/// Note that this will override any previous settings.
pub fn set_events_on_drop(&self, events: FsEvents) {
self.events_on_drop.store(events, Ordering::Relaxed);
}
/// Finds a subscriber and applies an action if found.
///
/// The matcher should return `Some(T)` if the subscriber matches and processing
@ -344,7 +335,9 @@ pub fn on_inode_removed(inode: &Arc<dyn Inode>) {
if !inode.fs().fs_event_subscriber_stats().has_any_subscribers() {
return;
}
notify_inode(inode, FsEvents::DELETE_SELF);
if let Some(publisher) = inode.fs_event_publisher() {
publisher.set_events_on_drop(FsEvents::DELETE_SELF);
}
}
/// Notifies that a file was linked to a directory.

View File

@ -9,7 +9,7 @@ use super::{is_dot, is_dot_or_dotdot, is_dotdot};
use crate::{
fs::{
self,
utils::{Inode, InodeExt, InodeMode, InodeType, MknodType},
utils::{Inode, InodeMode, InodeType, MknodType},
},
prelude::*,
};
@ -327,21 +327,9 @@ impl Dentry {
let nlinks = child_inode.metadata().nlinks;
fs::notify::on_link_count(&child_inode);
if nlinks == 0 {
// FIXME: `DELETE_SELF` should be generated after closing the last FD.
fs::notify::on_inode_removed(&child_inode);
}
fs::notify::on_delete(self.inode(), &child_inode, || name.to_string());
if nlinks == 0 {
// Ideally, we would use `fs_event_publisher()` here to avoid creating a
// `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();
let removed_nr_subscribers = publisher.disable_new_and_remove_subscribers();
child_inode
.fs()
.fs_event_subscriber_stats()
.remove_subscribers(removed_nr_subscribers);
}
Ok(())
}
@ -380,21 +368,9 @@ impl Dentry {
let nlinks = child_inode.metadata().nlinks;
if nlinks == 0 {
// FIXME: `DELETE_SELF` should be generated after closing the last FD.
fs::notify::on_inode_removed(&child_inode);
}
fs::notify::on_delete(self.inode(), &child_inode, || name.to_string());
if nlinks == 0 {
// Ideally, we would use `fs_event_publisher()` here to avoid creating a
// `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();
let removed_nr_subscribers = publisher.disable_new_and_remove_subscribers();
child_inode
.fs()
.fs_event_subscriber_stats()
.remove_subscribers(removed_nr_subscribers);
}
Ok(())
}

View File

@ -16,20 +16,79 @@ FN_TEST(unlink_add)
fd = TEST_RES(open(TEST_FILE, O_CREAT | O_WRONLY, 0644), _ret == 4);
TEST_SUCC(unlink(TEST_FILE));
// FIXME: Asterinas currently does not support adding inotify watches
// to deleted inodes.
#ifdef __asterinas__
TEST_ERRNO(inotify_add_watch(inotify_fd, "/proc/self/fd/4",
IN_DELETE_SELF),
ENOENT);
(void)wd;
#else
wd = TEST_SUCC(inotify_add_watch(inotify_fd, "/proc/self/fd/4",
IN_DELETE_SELF));
TEST_SUCC(inotify_rm_watch(inotify_fd, wd));
#endif
TEST_SUCC(close(fd));
TEST_SUCC(close(inotify_fd));
}
END_TEST()
FN_TEST(unlink_closed)
{
int inotify_fd, fd, wd;
struct inotify_event ev;
inotify_fd = TEST_SUCC(inotify_init1(O_NONBLOCK));
fd = TEST_SUCC(open(TEST_FILE, O_CREAT | O_WRONLY, 0644));
wd = TEST_SUCC(
inotify_add_watch(inotify_fd, TEST_FILE, IN_DELETE_SELF));
TEST_SUCC(close(fd));
TEST_SUCC(unlink(TEST_FILE));
// Both `IN_DELETE_SELF` and `IN_IGNORED` are generated.
TEST_RES(read(inotify_fd, &ev, sizeof(ev)),
_ret == sizeof(ev) && ev.mask == IN_DELETE_SELF);
TEST_RES(read(inotify_fd, &ev, sizeof(ev)),
_ret == sizeof(ev) && ev.mask == IN_IGNORED);
TEST_ERRNO(read(inotify_fd, &ev, sizeof(ev)), EAGAIN);
// The watch does not exist anymore.
TEST_ERRNO(inotify_rm_watch(inotify_fd, wd), EINVAL);
TEST_SUCC(close(inotify_fd));
}
END_TEST()
FN_TEST(unlink_remove)
{
int inotify_fd1, inotify_fd2, fd, wd1, wd2;
struct inotify_event ev;
inotify_fd1 = TEST_SUCC(inotify_init1(O_NONBLOCK));
inotify_fd2 = TEST_SUCC(inotify_init1(O_NONBLOCK));
fd = TEST_SUCC(open(TEST_FILE, O_CREAT | O_WRONLY, 0644));
wd1 = TEST_SUCC(
inotify_add_watch(inotify_fd1, TEST_FILE, IN_DELETE_SELF));
wd2 = TEST_SUCC(
inotify_add_watch(inotify_fd2, TEST_FILE, IN_DELETE_SELF));
TEST_SUCC(unlink(TEST_FILE));
TEST_ERRNO(read(inotify_fd1, &ev, sizeof(ev)), EAGAIN);
TEST_ERRNO(read(inotify_fd2, &ev, sizeof(ev)), EAGAIN);
// Removing the watch after unlinking is fine.
TEST_SUCC(inotify_rm_watch(inotify_fd2, wd2));
TEST_RES(read(inotify_fd2, &ev, sizeof(ev)),
_ret == sizeof(ev) && ev.mask == IN_IGNORED);
TEST_ERRNO(read(inotify_fd2, &ev, sizeof(ev)), EAGAIN);
// `IN_DELETE_SELF` and `IN_IGNORED` will be generated once we close `fd`.
TEST_SUCC(close(fd));
TEST_RES(read(inotify_fd1, &ev, sizeof(ev)),
_ret == sizeof(ev) && ev.mask == IN_DELETE_SELF);
TEST_RES(read(inotify_fd1, &ev, sizeof(ev)),
_ret == sizeof(ev) && ev.mask == IN_IGNORED);
TEST_ERRNO(read(inotify_fd1, &ev, sizeof(ev)), EAGAIN);
// The watch does not exist anymore after we close `fd`.
TEST_ERRNO(inotify_rm_watch(inotify_fd1, wd1), EINVAL);
TEST_SUCC(close(inotify_fd1));
TEST_SUCC(close(inotify_fd2));
}
END_TEST()

View File

@ -15,10 +15,6 @@ Inotify.ExcludeUnlinkInodeEvents_NoRandomSave
Inotify.ExcludeUnlinkMultipleChildren
Inotify.ExcludeUnlinkMultipleChildren_NoRandomSave
# TODO: Generate `DELETE_SELF`/`IGNORED` after closing the last FD.
Inotify.IncludeUnlinkedFile
Inotify.IncludeUnlinkedFile_NoRandomSave
# TODO: Report `MOVE_*` events at the `rename()` system call.
Inotify.MoveGeneratesEvents
Inotify.MoveWatchedTargetGeneratesEvents