From 4d4fb3b6ec11a3e461c02de17ef161c51a1ac46f Mon Sep 17 00:00:00 2001 From: Bananymous Date: Thu, 29 May 2025 01:02:22 +0300 Subject: [PATCH] Kernel: Cleanup and fix pipe pipe now sends SIGPIPE and returns EPIPE when writing and no readers are open --- kernel/include/kernel/FS/Inode.h | 3 ++ kernel/include/kernel/FS/Pipe.h | 8 +++-- kernel/kernel/FS/Pipe.cpp | 46 ++++++++++++++++++++----- kernel/kernel/OpenFileDescriptorSet.cpp | 42 +++++++--------------- 4 files changed, 58 insertions(+), 41 deletions(-) diff --git a/kernel/include/kernel/FS/Inode.h b/kernel/include/kernel/FS/Inode.h index 9941873abe..7d58c9ef42 100644 --- a/kernel/include/kernel/FS/Inode.h +++ b/kernel/include/kernel/FS/Inode.h @@ -133,6 +133,9 @@ namespace Kernel void del_epoll(class Epoll*); void epoll_notify(uint32_t event); + virtual void on_close(int status_flags) { (void)status_flags; } + virtual void on_clone(int status_flags) { (void)status_flags; } + protected: // Directory API virtual BAN::ErrorOr> find_inode_impl(BAN::StringView) { return BAN::Error::from_errno(ENOTSUP); } diff --git a/kernel/include/kernel/FS/Pipe.h b/kernel/include/kernel/FS/Pipe.h index a4ac876bfc..82f497dd9c 100644 --- a/kernel/include/kernel/FS/Pipe.h +++ b/kernel/include/kernel/FS/Pipe.h @@ -13,8 +13,9 @@ namespace Kernel static BAN::ErrorOr> create(const Credentials&); virtual bool is_pipe() const override { return true; } - void clone_writing(); - void close_writing(); + + void on_close(int status_flags) override; + void on_clone(int status_flags) override; virtual ino_t ino() const override { return 0; } // FIXME virtual Mode mode() const override { return { Mode::IFIFO | Mode::IRUSR | Mode::IWUSR }; } @@ -39,7 +40,7 @@ namespace Kernel virtual bool can_read_impl() const override { return m_buffer_size > 0; } virtual bool can_write_impl() const override { return true; } - virtual bool has_error_impl() const override { return false; } + virtual bool has_error_impl() const override { return m_reading_count == 0; } virtual bool has_hungup_impl() const override { return m_writing_count == 0; } private: @@ -58,6 +59,7 @@ namespace Kernel size_t m_buffer_tail { 0 }; BAN::Atomic m_writing_count { 1 }; + BAN::Atomic m_reading_count { 1 }; }; } diff --git a/kernel/kernel/FS/Pipe.cpp b/kernel/kernel/FS/Pipe.cpp index 30199fbc4d..703e4f4aa4 100644 --- a/kernel/kernel/FS/Pipe.cpp +++ b/kernel/kernel/FS/Pipe.cpp @@ -3,6 +3,7 @@ #include #include +#include #include namespace Kernel @@ -26,19 +27,41 @@ namespace Kernel m_ctime = current_time; } - void Pipe::clone_writing() + void Pipe::on_clone(int status_flags) { - [[maybe_unused]] auto old_writing_count = m_writing_count.fetch_add(1); - ASSERT(old_writing_count > 0); + if (status_flags & O_WRONLY) + { + [[maybe_unused]] auto old_writing_count = m_writing_count.fetch_add(1); + ASSERT(old_writing_count > 0); + } + + if (status_flags & O_RDONLY) + { + [[maybe_unused]] auto old_reading_count = m_reading_count.fetch_add(1); + ASSERT(old_reading_count > 0); + } } - void Pipe::close_writing() + void Pipe::on_close(int status_flags) { - auto old_writing_count = m_writing_count.fetch_sub(1); - ASSERT(old_writing_count > 0); - if (old_writing_count != 1) - return; - epoll_notify(EPOLLHUP); + if (status_flags & O_WRONLY) + { + auto old_writing_count = m_writing_count.fetch_sub(1); + ASSERT(old_writing_count > 0); + if (old_writing_count != 1) + return; + epoll_notify(EPOLLHUP); + } + + if (status_flags & O_RDONLY) + { + auto old_reading_count = m_reading_count.fetch_sub(1); + ASSERT(old_reading_count > 0); + if (old_reading_count != 1) + return; + epoll_notify(EPOLLERR); + } + m_thread_blocker.unblock(); } @@ -84,6 +107,11 @@ namespace Kernel while (m_buffer_size >= m_buffer.size()) { + if (m_reading_count == 0) + { + Thread::current().add_signal(SIGPIPE); + return BAN::Error::from_errno(EPIPE); + } LockFreeGuard lock_free(m_mutex); TRY(Thread::current().block_or_eintr_or_timeout_ms(m_thread_blocker, 100, false)); } diff --git a/kernel/kernel/OpenFileDescriptorSet.cpp b/kernel/kernel/OpenFileDescriptorSet.cpp index 1e60b66fe9..7cf764c398 100644 --- a/kernel/kernel/OpenFileDescriptorSet.cpp +++ b/kernel/kernel/OpenFileDescriptorSet.cpp @@ -43,12 +43,7 @@ namespace Kernel auto& open_file = m_open_files[fd]; open_file.description = other.m_open_files[fd].description; open_file.descriptor_flags = other.m_open_files[fd].descriptor_flags; - - if (open_file.path() == ""_sv) - { - ASSERT(open_file.inode()->is_pipe()); - static_cast(open_file.inode().ptr())->clone_writing(); - } + open_file.inode()->on_clone(open_file.status_flags()); } return {}; @@ -204,14 +199,10 @@ namespace Kernel (void)close(fildes2); - m_open_files[fildes2].description = m_open_files[fildes].description; - m_open_files[fildes2].descriptor_flags = 0; - - if (m_open_files[fildes2].path() == ""_sv) - { - ASSERT(m_open_files[fildes2].inode()->is_pipe()); - static_cast(m_open_files[fildes2].inode().ptr())->clone_writing(); - } + auto& open_file = m_open_files[fildes2]; + open_file.description = m_open_files[fildes].description; + open_file.descriptor_flags = 0; + open_file.inode()->on_clone(open_file.status_flags()); return fildes; } @@ -229,13 +220,10 @@ namespace Kernel { const int new_fd = TRY(get_free_fd()); - m_open_files[new_fd].description = m_open_files[fd].description; - m_open_files[new_fd].descriptor_flags = (cmd == F_DUPFD_CLOEXEC) ? O_CLOEXEC : 0; - if (m_open_files[new_fd].path() == ""_sv) - { - ASSERT(m_open_files[new_fd].inode()->is_pipe()); - static_cast(m_open_files[new_fd].inode().ptr())->clone_writing(); - } + auto& open_file = m_open_files[new_fd]; + open_file.description = m_open_files[fd].description; + open_file.descriptor_flags = (cmd == F_DUPFD_CLOEXEC) ? O_CLOEXEC : 0; + open_file.inode()->on_clone(open_file.status_flags()); return new_fd; } @@ -313,14 +301,10 @@ namespace Kernel TRY(validate_fd(fd)); - if (m_open_files[fd].path() == ""_sv) - { - ASSERT(m_open_files[fd].inode()->is_pipe()); - static_cast(m_open_files[fd].inode().ptr())->close_writing(); - } - - m_open_files[fd].description.clear(); - m_open_files[fd].descriptor_flags = 0; + auto& open_file = m_open_files[fd]; + open_file.inode()->on_close(open_file.status_flags()); + open_file.description.clear(); + open_file.descriptor_flags = 0; return {}; }