From b43289d544116515e4afcccd37e3941b7ec1d483 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Thu, 26 Feb 2026 16:11:28 -0500 Subject: [PATCH] ramfs: Rework NodeMTimeUpdater into NodeStatChangeNotifier. This now does what notify_if_stat_changed used to, fixing a race: if some thread was modifying a file, and some other thread (or the same thread, in some other mtime-updating operation) updated that file and this called MarkUnmodified() before the first thread close()d it, then when the first thread called close(), no node monitor notification would've been sent, as fModified would've already been unset. So, now every time we are to mark a file "unmodified", we also send the node monitor notification at the same time. Fixes the remainder of #19910. --- .../kernel/file_systems/ramfs/Node.cpp | 30 ++++++++ src/add-ons/kernel/file_systems/ramfs/Node.h | 25 +----- .../file_systems/ramfs/kernel_interface.cpp | 77 +++++++------------ 3 files changed, 60 insertions(+), 72 deletions(-) diff --git a/src/add-ons/kernel/file_systems/ramfs/Node.cpp b/src/add-ons/kernel/file_systems/ramfs/Node.cpp index 5331df76c8..64fd8a601f 100644 --- a/src/add-ons/kernel/file_systems/ramfs/Node.cpp +++ b/src/add-ons/kernel/file_systems/ramfs/Node.cpp @@ -135,6 +135,19 @@ Node::SetMTime(time_t mTime) } +uint32 +Node::MarkUnmodified() +{ + uint32 modified = fModified; + if (modified) { + fCTime = time(NULL); + SetMTime(fCTime); + fModified = 0; + } + return modified; +} + + status_t Node::CheckPermissions(int mode) const { @@ -317,3 +330,20 @@ Node::GetAllocationInfo(AllocationInfo &info) while (GetNextAttribute(&attribute) == B_OK) attribute->GetAllocationInfo(info); } + + +NodeStatChangeNotifier::~NodeStatChangeNotifier() +{ + if (fNode == NULL || !fNode->IsModified()) + return; + + uint32 statFields = fNode->MarkUnmodified(); + for (Entry* entry = fNode->GetFirstReferrer(); entry != NULL; + entry = fNode->GetNextReferrer(entry)) { + ino_t parentID = -1; + if (entry->GetParent() != NULL) + parentID = ((Node*)entry->GetParent())->GetID(); + notify_stat_changed(fNode->GetVolume()->GetID(), parentID, + fNode->GetID(), statFields); + } +} diff --git a/src/add-ons/kernel/file_systems/ramfs/Node.h b/src/add-ons/kernel/file_systems/ramfs/Node.h index 9a7a8b0561..0857d7640d 100644 --- a/src/add-ons/kernel/file_systems/ramfs/Node.h +++ b/src/add-ons/kernel/file_systems/ramfs/Node.h @@ -129,32 +129,15 @@ protected: DoublyLinkedList fReferrers; }; -// MarkUnmodified -inline -uint32 -Node::MarkUnmodified() -{ - uint32 modified = fModified; - if (modified) { - fCTime = time(NULL); - SetMTime(fCTime); - fModified = 0; - } - return modified; -} -// NodeMTimeUpdater -class NodeMTimeUpdater { +class NodeStatChangeNotifier { public: - NodeMTimeUpdater(Node *node) : fNode(node) {} - ~NodeMTimeUpdater() - { - if (fNode && fNode->IsModified()) - fNode->MarkUnmodified(); - } + NodeStatChangeNotifier(Node *node) : fNode(node) {} + ~NodeStatChangeNotifier(); private: Node *fNode; }; + #endif // NODE_H diff --git a/src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp b/src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp index 48c626b8b2..23a3dcc925 100644 --- a/src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/ramfs/kernel_interface.cpp @@ -68,23 +68,6 @@ static const size_t kOptimalIOSize = 65536; static const bigtime_t kNotificationInterval = 1000000LL; -static void -notify_if_stat_changed(Volume *volume, Node *node) -{ - if (volume == NULL || node == NULL || !node->IsModified()) - return; - - uint32 statFields = node->MarkUnmodified(); - for (Entry* entry = node->GetFirstReferrer(); entry != NULL; - entry = node->GetNextReferrer(entry)) { - ino_t parentID = -1; - if (entry->GetParent() != NULL) - parentID = entry->GetParent()->GetID(); - notify_stat_changed(volume->GetID(), parentID, node->GetID(), statFields); - } -} - - // #pragma mark - FS @@ -418,7 +401,7 @@ ramfs_create_symlink(fs_volume* _volume, fs_vnode* _dir, const char *name, RETURN_ERROR(B_ERROR); status_t error = B_OK; - NodeMTimeUpdater mTimeUpdater(dir); + NodeStatChangeNotifier statNotifier(dir); // directory deleted? bool removed; if (get_vnode_removed(volume->FSVolume(), dir->GetID(), &removed) @@ -448,7 +431,7 @@ ramfs_create_symlink(fs_volume* _volume, fs_vnode* _dir, const char *name, } } } - NodeMTimeUpdater mTimeUpdater2(node); + NodeStatChangeNotifier statNotifier2(node); // notify listeners if (error == B_OK) { notify_entry_created(volume->GetID(), dir->GetID(), name, @@ -476,7 +459,7 @@ ramfs_link(fs_volume* _volume, fs_vnode* _dir, const char *name, RETURN_ERROR(B_ERROR); status_t error = B_OK; - NodeMTimeUpdater mTimeUpdater(dir); + NodeStatChangeNotifier statNotifier(dir); // directory deleted? bool removed; if (get_vnode_removed(volume->FSVolume(), dir->GetID(), &removed) @@ -522,7 +505,7 @@ ramfs_unlink(fs_volume* _volume, fs_vnode* _dir, const char *name) if (!locker.IsLocked()) RETURN_ERROR(B_ERROR); - NodeMTimeUpdater mTimeUpdater(dir); + NodeStatChangeNotifier statNotifier(dir); // check directory write permissions error = dir->CheckPermissions(W_OK); ino_t nodeID = -1; @@ -568,8 +551,8 @@ ramfs_rename(fs_volume* _volume, fs_vnode* _oldDir, const char *oldName, status_t error = B_OK; - NodeMTimeUpdater mTimeUpdater1(oldDir); - NodeMTimeUpdater mTimeUpdater2(newDir); + NodeStatChangeNotifier statNotifier1(oldDir); + NodeStatChangeNotifier statNotifier2(newDir); // target directory deleted? bool removed; @@ -733,7 +716,7 @@ ramfs_write_stat(fs_volume* _volume, fs_vnode* _node, const struct stat *st, mask, st) != B_OK) RETURN_ERROR(B_NOT_ALLOWED); - NodeMTimeUpdater mTimeUpdater(node); + NodeStatChangeNotifier statNotifier(node); status_t error = B_OK; if ((mask & B_STAT_SIZE) != 0) @@ -759,10 +742,6 @@ ramfs_write_stat(fs_volume* _volume, fs_vnode* _node, const struct stat *st, node->SetCrTime(st->st_crtime); } - // notify listeners - if (error == B_OK) - notify_if_stat_changed(volume, node); - RETURN_ERROR(error); } @@ -814,7 +793,7 @@ ramfs_create(fs_volume* _volume, fs_vnode* _dir, const char *name, int openMode, if (!locker.IsLocked()) RETURN_ERROR(B_ERROR); - NodeMTimeUpdater mTimeUpdater(dir); + NodeStatChangeNotifier statNotifier(dir); status_t error = B_OK; // directory deleted? @@ -879,7 +858,7 @@ ramfs_create(fs_volume* _volume, fs_vnode* _dir, const char *name, int openMode, else if (cookie) delete cookie; } - NodeMTimeUpdater mTimeUpdater2(node); + NodeStatChangeNotifier statNotifier2(node); // notify listeners if (error == B_OK) notify_entry_created(volume->GetID(), dir->GetID(), name, *vnid); @@ -906,7 +885,7 @@ ramfs_create_special_node(fs_volume *_volume, fs_vnode *_dir, const char *name, if (!locker.IsLocked()) RETURN_ERROR(B_ERROR); - NodeMTimeUpdater mTimeUpdater(dir); + NodeStatChangeNotifier statNotifier(dir); status_t error = B_OK; // directory deleted? @@ -944,7 +923,7 @@ ramfs_create_special_node(fs_volume *_volume, fs_vnode *_dir, const char *name, RETURN_ERROR(error); } - NodeMTimeUpdater mTimeUpdater2(node); + NodeStatChangeNotifier statNotifier2(node); // notify listeners notify_entry_created(volume->GetID(), dir->GetID(), name, *vnid); @@ -989,7 +968,7 @@ ramfs_open(fs_volume* _volume, fs_vnode* _node, int openMode, void** _cookie) VolumeWriteLocker writeLocker(volume); error = node->SetSize(0); - NodeMTimeUpdater mTimeUpdater(node); + NodeStatChangeNotifier statNotifier(node); } // set result / cleanup on failure @@ -1011,15 +990,12 @@ ramfs_close(fs_volume* _volume, fs_vnode* _node, void* /*cookie*/) FUNCTION(("node: %lld\n", node->GetID())); - VolumeReadLocker locker(volume); + VolumeWriteLocker locker(volume); if (!locker.IsLocked()) RETURN_ERROR(B_ERROR); - status_t error = B_OK; - // notify listeners - notify_if_stat_changed(volume, node); - - return error; + NodeStatChangeNotifier _(node); + return B_OK; } @@ -1103,9 +1079,9 @@ ramfs_write(fs_volume* _volume, fs_vnode* _node, void* _cookie, off_t pos, } } // notify listeners - if (error == B_OK && cookie->NotificationIntervalElapsed(true)) - notify_if_stat_changed(volume, node); - NodeMTimeUpdater mTimeUpdater(node); + if (error == B_OK && cookie->NotificationIntervalElapsed(true)) { + NodeStatChangeNotifier _(node); + } RETURN_ERROR(error); } @@ -1210,7 +1186,7 @@ ramfs_create_dir(fs_volume* _volume, fs_vnode* _dir, const char *name, int mode) if (!locker.IsLocked()) RETURN_ERROR(B_ERROR); - NodeMTimeUpdater mTimeUpdater(dir); + NodeStatChangeNotifier statNotifier(dir); // directory deleted? bool removed; status_t error = B_OK; @@ -1241,7 +1217,7 @@ ramfs_create_dir(fs_volume* _volume, fs_vnode* _dir, const char *name, int mode) } } } - NodeMTimeUpdater mTimeUpdater2(node); + NodeStatChangeNotifier statNotifier2(node); // notify listeners if (error == B_OK) { notify_entry_created(volume->GetID(), dir->GetID(), name, @@ -1268,7 +1244,7 @@ ramfs_remove_dir(fs_volume* _volume, fs_vnode* _dir, const char *name) if (!locker.IsLocked()) RETURN_ERROR(B_ERROR); - NodeMTimeUpdater mTimeUpdater(dir); + NodeStatChangeNotifier statNotifier(dir); // check directory write permissions status_t error = dir->CheckPermissions(W_OK); ino_t nodeID = -1; @@ -1638,7 +1614,7 @@ ramfs_create_attr(fs_volume* _volume, fs_vnode* _node, const char *name, if (error != B_OK) return error; } - NodeMTimeUpdater mTimeUpdater(node); + NodeStatChangeNotifier statNotifier(node); // success cookieDeleter.Detach(); @@ -1689,7 +1665,7 @@ ramfs_open_attr(fs_volume* _volume, fs_vnode* _node, const char *name, // truncate if requested if (error == B_OK && (openMode & O_TRUNC)) error = attribute->SetSize(0); - NodeMTimeUpdater mTimeUpdater(node); + NodeStatChangeNotifier statNotifier(node); // set result / cleanup on failure if (error == B_OK) @@ -1713,8 +1689,7 @@ ramfs_close_attr(fs_volume* _volume, fs_vnode* _node, void* _cookie) if (!locker.IsLocked()) RETURN_ERROR(B_ERROR); - // notify listeners - notify_if_stat_changed(volume, node); + NodeStatChangeNotifier _(node); return B_OK; } @@ -1788,7 +1763,7 @@ ramfs_write_attr(fs_volume* _volume, fs_vnode* _node, void* _cookie, if (!locker.IsLocked()) RETURN_ERROR(B_ERROR); - NodeMTimeUpdater mTimeUpdater(node); + NodeStatChangeNotifier statNotifier(node); // find the attribute Attribute *attribute = NULL; @@ -1864,7 +1839,7 @@ ramfs_remove_attr(fs_volume* _volume, fs_vnode* _node, const char *name) if (!locker.IsLocked()) RETURN_ERROR(B_ERROR); - NodeMTimeUpdater mTimeUpdater(node); + NodeStatChangeNotifier statNotifier(node); // check permissions error = node->CheckPermissions(W_OK);