RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 18:27:35 +00:00
Code Issues Activity Actions

Kernel: Use non-locking {Nonnull,}RefPtr for OpenFileDescription

Browse source 
This patch switches away from {Nonnull,}LockRefPtr to the non-locking
smart pointers throughout the kernel.

I've looked at the handful of places where these were being persisted
and I don't see any race situations.

Note that the process file descriptor table (Process::m_fds) was already
guarded via MutexProtected.
This commit is contained in:
Andreas Kling 2023-03-06 19:29:25 +01:00
parent 36b0ecfe9e
commit d1371d66f7
34 changed files with 82 additions and 80 deletions
Download patch file Download diff file Expand all files Collapse all files

4
Kernel/FileSystem/FIFO.cpp
View file

@ -22,7 +22,7 @@ ErrorOr<NonnullLockRefPtr<FIFO>> FIFO::try_create(UserID uid)
return adopt_nonnull_lock_ref_or_enomem(new (nothrow) FIFO(uid, move(buffer)));
}
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> FIFO::open_direction(FIFO::Direction direction)
ErrorOr<NonnullRefPtr<OpenFileDescription>> FIFO::open_direction(FIFO::Direction direction)
{
auto description = TRY(OpenFileDescription::try_create(*this));
attach(direction);
@ -30,7 +30,7 @@ ErrorOr<NonnullLockRefPtr<OpenFileDescription>> FIFO::open_direction(FIFO::Direc
return description;
}
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> FIFO::open_direction_blocking(FIFO::Direction direction)
ErrorOr<NonnullRefPtr<OpenFileDescription>> FIFO::open_direction_blocking(FIFO::Direction direction)
{
MutexLocker locker(m_open_lock);

4
Kernel/FileSystem/FIFO.h
View file

@ -29,8 +29,8 @@ public:
UserID uid() const { return m_uid; }
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> open_direction(Direction);
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> open_direction_blocking(Direction);
ErrorOr<NonnullRefPtr<OpenFileDescription>> open_direction(Direction);
ErrorOr<NonnullRefPtr<OpenFileDescription>> open_direction_blocking(Direction);
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Woverloaded-virtual"

2
Kernel/FileSystem/File.cpp
View file

@ -15,7 +15,7 @@ namespace Kernel {
File::File() = default;
File::~File() = default;
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> File::open(int options)
ErrorOr<NonnullRefPtr<OpenFileDescription>> File::open(int options)
{
auto description = OpenFileDescription::try_create(*this);
if (!description.is_error()) {

2
Kernel/FileSystem/File.h
View file

@ -78,7 +78,7 @@ public:
virtual void will_be_destroyed() { }
virtual ~File();
virtual ErrorOr<NonnullLockRefPtr<OpenFileDescription>> open(int options);
virtual ErrorOr<NonnullRefPtr<OpenFileDescription>> open(int options);
virtual ErrorOr<void> close();
virtual bool can_read(OpenFileDescription const&, u64) const = 0;

2
Kernel/FileSystem/FileBackedFileSystem.h
View file

@ -38,6 +38,6 @@ private:
virtual bool is_file_backed() const override { return true; }
IntrusiveListNode<FileBackedFileSystem> m_file_backed_file_system_node;
mutable NonnullLockRefPtr<OpenFileDescription> m_file_description;
NonnullRefPtr<OpenFileDescription> m_file_description;
};
}

8
Kernel/FileSystem/OpenFileDescription.cpp
View file

@ -23,19 +23,19 @@
namespace Kernel {
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> OpenFileDescription::try_create(Custody& custody)
ErrorOr<NonnullRefPtr<OpenFileDescription>> OpenFileDescription::try_create(Custody& custody)
{
auto inode_file = TRY(InodeFile::create(custody.inode()));
auto description = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) OpenFileDescription(move(inode_file))));
auto description = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) OpenFileDescription(move(inode_file))));
description->m_state.with([&](auto& state) { state.custody = custody; });
TRY(description->attach());
return description;
}
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> OpenFileDescription::try_create(File& file)
ErrorOr<NonnullRefPtr<OpenFileDescription>> OpenFileDescription::try_create(File& file)
{
auto description = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) OpenFileDescription(file)));
auto description = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) OpenFileDescription(file)));
TRY(description->attach());
return description;
}

4
Kernel/FileSystem/OpenFileDescription.h
View file

@ -26,8 +26,8 @@ public:
class OpenFileDescription final : public AtomicRefCounted<OpenFileDescription> {
public:
static ErrorOr<NonnullLockRefPtr<OpenFileDescription>> try_create(Custody&);
static ErrorOr<NonnullLockRefPtr<OpenFileDescription>> try_create(File&);
static ErrorOr<NonnullRefPtr<OpenFileDescription>> try_create(Custody&);
static ErrorOr<NonnullRefPtr<OpenFileDescription>> try_create(File&);
~OpenFileDescription();
Thread::FileBlocker::BlockFlags should_unblock(Thread::FileBlocker::BlockFlags) const;

4
Kernel/FileSystem/ProcFS/ProcessExposed.cpp
View file

@ -262,7 +262,7 @@ ErrorOr<void> Process::procfs_get_fds_stats(KBufferBuilder& builder) const
return {};
}
bool cloexec = file_description_metadata.flags() & FD_CLOEXEC;
LockRefPtr<OpenFileDescription> description = file_description_metadata.description();
auto const* description = file_description_metadata.description();
auto description_object = TRY(array.add_object());
TRY(description_object.add("fd"sv, count));
// TODO: Better OOM handling.
@ -275,7 +275,7 @@ ErrorOr<void> Process::procfs_get_fds_stats(KBufferBuilder& builder) const
TRY(description_object.add("blocking"sv, description->is_blocking()));
TRY(description_object.add("can_read"sv, description->can_read()));
TRY(description_object.add("can_write"sv, description->can_write()));
Inode* inode = description->inode();
Inode const* inode = description->inode();
if (inode != nullptr) {
auto inode_object = TRY(description_object.add_object("inode"sv));
TRY(inode_object.add("fsid"sv, inode->fsid().value()));

8
Kernel/FileSystem/VirtualFileSystem.cpp
View file

@ -356,12 +356,12 @@ ErrorOr<NonnullLockRefPtr<FileBackedFileSystem>> VirtualFileSystem::find_already
}));
}
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> VirtualFileSystem::open(Credentials const& credentials, StringView path, int options, mode_t mode, Custody& base, Optional<UidAndGid> owner)
ErrorOr<NonnullRefPtr<OpenFileDescription>> VirtualFileSystem::open(Credentials const& credentials, StringView path, int options, mode_t mode, Custody& base, Optional<UidAndGid> owner)
{
return open(Process::current(), credentials, path, options, mode, base, owner);
}
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> VirtualFileSystem::open(Process const& process, Credentials const& credentials, StringView path, int options, mode_t mode, Custody& base, Optional<UidAndGid> owner)
ErrorOr<NonnullRefPtr<OpenFileDescription>> VirtualFileSystem::open(Process const& process, Credentials const& credentials, StringView path, int options, mode_t mode, Custody& base, Optional<UidAndGid> owner)
{
if ((options & O_CREAT) && (options & O_DIRECTORY))
return EINVAL;
@ -476,12 +476,12 @@ ErrorOr<void> VirtualFileSystem::mknod(Credentials const& credentials, StringVie
return {};
}
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> VirtualFileSystem::create(Credentials const& credentials, StringView path, int options, mode_t mode, Custody& parent_custody, Optional<UidAndGid> owner)
ErrorOr<NonnullRefPtr<OpenFileDescription>> VirtualFileSystem::create(Credentials const& credentials, StringView path, int options, mode_t mode, Custody& parent_custody, Optional<UidAndGid> owner)
{
return create(Process::current(), credentials, path, options, mode, parent_custody, owner);
}
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> VirtualFileSystem::create(Process const& process, Credentials const& credentials, StringView path, int options, mode_t mode, Custody& parent_custody, Optional<UidAndGid> owner)
ErrorOr<NonnullRefPtr<OpenFileDescription>> VirtualFileSystem::create(Process const& process, Credentials const& credentials, StringView path, int options, mode_t mode, Custody& parent_custody, Optional<UidAndGid> owner)
{
auto basename = KLexicalPath::basename(path);
auto parent_path = TRY(parent_custody.try_serialize_absolute_path());

8
Kernel/FileSystem/VirtualFileSystem.h
View file

@ -59,10 +59,10 @@ public:
ErrorOr<void> remount(Custody& mount_point, int new_flags);
ErrorOr<void> unmount(Custody& mount_point);
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> open(Credentials const&, StringView path, int options, mode_t mode, Custody& base, Optional<UidAndGid> = {});
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> open(Process const&, Credentials const&, StringView path, int options, mode_t mode, Custody& base, Optional<UidAndGid> = {});
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> create(Credentials const&, StringView path, int options, mode_t mode, Custody& parent_custody, Optional<UidAndGid> = {});
ErrorOr<NonnullLockRefPtr<OpenFileDescription>> create(Process const&, Credentials const&, StringView path, int options, mode_t mode, Custody& parent_custody, Optional<UidAndGid> = {});
ErrorOr<NonnullRefPtr<OpenFileDescription>> open(Credentials const&, StringView path, int options, mode_t mode, Custody& base, Optional<UidAndGid> = {});
ErrorOr<NonnullRefPtr<OpenFileDescription>> open(Process const&, Credentials const&, StringView path, int options, mode_t mode, Custody& base, Optional<UidAndGid> = {});
ErrorOr<NonnullRefPtr<OpenFileDescription>> create(Credentials const&, StringView path, int options, mode_t mode, Custody& parent_custody, Optional<UidAndGid> = {});
ErrorOr<NonnullRefPtr<OpenFileDescription>> create(Process const&, Credentials const&, StringView path, int options, mode_t mode, Custody& parent_custody, Optional<UidAndGid> = {});
ErrorOr<void> mkdir(Credentials const&, StringView path, mode_t mode, Custody& base);
ErrorOr<void> link(Credentials const&, StringView old_path, StringView new_path, Custody& base);
ErrorOr<void> unlink(Credentials const&, StringView path, Custody& base);