From b668173cbaced0afe91ca37c37f562305ca1e543 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Fri, 6 Jun 2025 11:09:50 +0300 Subject: [PATCH] Kernel: Fix pseudo terminal writability --- .../include/kernel/Terminal/PseudoTerminal.h | 8 +++++++- kernel/include/kernel/Terminal/Serial.h | 15 +++++++------- kernel/include/kernel/Terminal/TTY.h | 10 ++++++---- kernel/include/kernel/Terminal/VirtualTTY.h | 15 +++++++------- kernel/kernel/OpenFileDescriptorSet.cpp | 4 ++-- kernel/kernel/Terminal/PseudoTerminal.cpp | 20 ++++++++++++++++++- kernel/kernel/Terminal/TTY.cpp | 19 +++++++++++------- kernel/kernel/Terminal/VirtualTTY.cpp | 2 +- 8 files changed, 63 insertions(+), 30 deletions(-) diff --git a/kernel/include/kernel/Terminal/PseudoTerminal.h b/kernel/include/kernel/Terminal/PseudoTerminal.h index 36b3ef77..2a241cb2 100644 --- a/kernel/include/kernel/Terminal/PseudoTerminal.h +++ b/kernel/include/kernel/Terminal/PseudoTerminal.h @@ -27,10 +27,12 @@ namespace Kernel BAN::ErrorOr write_impl(off_t, BAN::ConstByteSpan) override; bool can_read_impl() const override { SpinLockGuard _(m_buffer_lock); return m_buffer_size > 0; } - bool can_write_impl() const override { SpinLockGuard _(m_buffer_lock); return m_buffer_size < m_buffer->size(); } + bool can_write_impl() const override { return true; } bool has_error_impl() const override { return false; } bool has_hungup_impl() const override { return !m_slave.valid(); } + void on_close(int) override; + BAN::ErrorOr ioctl_impl(int, void*) override; private: @@ -48,6 +50,7 @@ namespace Kernel const dev_t m_rdev; + friend class PseudoTerminalSlave; friend class BAN::RefPtr; }; @@ -62,8 +65,11 @@ namespace Kernel void clear() override; protected: + bool master_has_closed() const override { return !m_master.valid(); } + bool putchar_impl(uint8_t ch) override; + bool can_write_impl() const override; bool has_hungup_impl() const override { return !m_master.valid(); } BAN::ErrorOr ioctl_impl(int, void*) override; diff --git a/kernel/include/kernel/Terminal/Serial.h b/kernel/include/kernel/Terminal/Serial.h index ce1d6f8c..4a4b2f5c 100644 --- a/kernel/include/kernel/Terminal/Serial.h +++ b/kernel/include/kernel/Terminal/Serial.h @@ -42,18 +42,19 @@ namespace Kernel public: static BAN::ErrorOr> create(Serial); - virtual uint32_t width() const override; - virtual uint32_t height() const override; + uint32_t width() const override; + uint32_t height() const override; - virtual void clear() override { putchar_impl('\e'); putchar_impl('['); putchar_impl('2'); putchar_impl('J'); } + void clear() override { putchar_impl('\e'); putchar_impl('['); putchar_impl('2'); putchar_impl('J'); } - virtual void update() override; + void update() override; - virtual void handle_irq() override; + void handle_irq() override; protected: - virtual BAN::StringView name() const override { return m_name; } - virtual bool putchar_impl(uint8_t) override; + BAN::StringView name() const override { return m_name; } + bool putchar_impl(uint8_t) override; + bool can_write_impl() const override { return true; } private: SerialTTY(Serial); diff --git a/kernel/include/kernel/Terminal/TTY.h b/kernel/include/kernel/Terminal/TTY.h index 1244132e..3ff63695 100644 --- a/kernel/include/kernel/Terminal/TTY.h +++ b/kernel/include/kernel/Terminal/TTY.h @@ -66,18 +66,19 @@ namespace Kernel virtual BAN::ErrorOr ioctl_impl(int, void*) override; virtual bool can_read_impl() const override { return m_output.flush; } - virtual bool can_write_impl() const override { return true; } virtual bool has_error_impl() const override { return false; } virtual bool has_hungup_impl() const override { return false; } + virtual bool master_has_closed() const { return false; } + protected: TTY(termios termios, mode_t mode, uid_t uid, gid_t gid); virtual bool putchar_impl(uint8_t ch) = 0; - virtual void update_cursor() {} + virtual void after_write() {} - virtual BAN::ErrorOr read_impl(off_t, BAN::ByteSpan) override; - virtual BAN::ErrorOr write_impl(off_t, BAN::ConstByteSpan) override; + virtual BAN::ErrorOr read_impl(off_t, BAN::ByteSpan) final override; + virtual BAN::ErrorOr write_impl(off_t, BAN::ConstByteSpan) final override; private: bool putchar(uint8_t ch); @@ -110,6 +111,7 @@ namespace Kernel protected: RecursiveSpinLock m_write_lock; + ThreadBlocker m_write_blocker; }; } diff --git a/kernel/include/kernel/Terminal/VirtualTTY.h b/kernel/include/kernel/Terminal/VirtualTTY.h index 42bffa9d..9fbdf927 100644 --- a/kernel/include/kernel/Terminal/VirtualTTY.h +++ b/kernel/include/kernel/Terminal/VirtualTTY.h @@ -17,17 +17,18 @@ namespace Kernel public: static BAN::ErrorOr> create(BAN::RefPtr); - virtual BAN::ErrorOr set_font(LibFont::Font&&) override; + BAN::ErrorOr set_font(LibFont::Font&&) override; - virtual uint32_t height() const override { return m_height; } - virtual uint32_t width() const override { return m_width; } + uint32_t height() const override { return m_height; } + uint32_t width() const override { return m_width; } - virtual void clear() override; + void clear() override; protected: - virtual BAN::StringView name() const override { return m_name; } - virtual bool putchar_impl(uint8_t ch) override; - void update_cursor() override; + BAN::StringView name() const override { return m_name; } + bool putchar_impl(uint8_t ch) override; + bool can_write_impl() const override { return true; } + void after_write() override; private: VirtualTTY(BAN::RefPtr); diff --git a/kernel/kernel/OpenFileDescriptorSet.cpp b/kernel/kernel/OpenFileDescriptorSet.cpp index 309f69f7..3f88fb45 100644 --- a/kernel/kernel/OpenFileDescriptorSet.cpp +++ b/kernel/kernel/OpenFileDescriptorSet.cpp @@ -360,7 +360,7 @@ namespace Kernel { LockGuard _(inode->m_mutex); if (is_nonblock && !inode->can_read()) - return 0; + return BAN::Error::from_errno(EAGAIN); nread = TRY(inode->read(offset, buffer)); } @@ -395,7 +395,7 @@ namespace Kernel { LockGuard _(inode->m_mutex); if (is_nonblock && !inode->can_write()) - return BAN::Error::from_errno(EWOULDBLOCK); + return BAN::Error::from_errno(EAGAIN); nwrite = TRY(inode->write(offset, buffer)); } diff --git a/kernel/kernel/Terminal/PseudoTerminal.cpp b/kernel/kernel/Terminal/PseudoTerminal.cpp index e6b62036..0dd737d1 100644 --- a/kernel/kernel/Terminal/PseudoTerminal.cpp +++ b/kernel/kernel/Terminal/PseudoTerminal.cpp @@ -130,6 +130,9 @@ namespace Kernel m_buffer_size -= to_copy; m_buffer_tail = (m_buffer_tail + to_copy) % m_buffer->size(); + if (auto slave = m_slave.lock()) + slave->m_write_blocker.unblock(); + epoll_notify(EPOLLOUT); return to_copy; @@ -139,12 +142,18 @@ namespace Kernel { auto slave = m_slave.lock(); if (!slave) - return BAN::Error::from_errno(ENODEV); + return BAN::Error::from_errno(EIO); for (size_t i = 0; i < buffer.size(); i++) slave->handle_input_byte(buffer[i]); return buffer.size(); } + void PseudoTerminalMaster::on_close(int) + { + if (auto slave = m_slave.lock()) + slave->m_write_blocker.unblock(); + } + BAN::ErrorOr PseudoTerminalMaster::ioctl_impl(int request, void* argument) { auto slave = m_slave.lock(); @@ -186,6 +195,15 @@ namespace Kernel return false; } + bool PseudoTerminalSlave::can_write_impl() const + { + auto master = m_master.lock(); + if (!master) + return false; + SpinLockGuard _(master->m_buffer_lock); + return master->m_buffer_size < master->m_buffer->size(); + } + BAN::ErrorOr PseudoTerminalSlave::ioctl_impl(int request, void* argument) { switch (request) diff --git a/kernel/kernel/Terminal/TTY.cpp b/kernel/kernel/Terminal/TTY.cpp index c1c9290b..ea088089 100644 --- a/kernel/kernel/Terminal/TTY.cpp +++ b/kernel/kernel/Terminal/TTY.cpp @@ -219,7 +219,7 @@ namespace Kernel auto* ptr = reinterpret_cast(ansi_c_str); while (*ptr) handle_input_byte(*ptr++); - update_cursor(); + after_write(); } } @@ -297,12 +297,8 @@ namespace Kernel if (should_append) { - // FIXME: don't ignore these bytes if (m_output.bytes >= m_output.buffer.size()) - { - dwarnln("TTY input full"); return; - } m_output.buffer[m_output.bytes++] = ch; } @@ -405,6 +401,8 @@ namespace Kernel if (m_output.bytes == 0) { + if (master_has_closed()) + return 0; m_output.flush = false; return 0; } @@ -431,6 +429,13 @@ namespace Kernel BAN::ErrorOr TTY::write_impl(off_t, BAN::ConstByteSpan buffer) { + while (!can_write_impl()) + { + if (master_has_closed()) + return BAN::Error::from_errno(EIO); + TRY(Thread::current().block_or_eintr_indefinite(m_write_blocker, &m_mutex)); + } + size_t written = 0; { @@ -438,7 +443,7 @@ namespace Kernel for (; written < buffer.size(); written++) if (!putchar(buffer[written])) break; - update_cursor(); + after_write(); } if (can_write_impl()) @@ -452,7 +457,7 @@ namespace Kernel ASSERT(s_tty); SpinLockGuard _(s_tty->m_write_lock); s_tty->putchar(ch); - s_tty->update_cursor(); + s_tty->after_write(); } bool TTY::is_initialized() diff --git a/kernel/kernel/Terminal/VirtualTTY.cpp b/kernel/kernel/Terminal/VirtualTTY.cpp index 647a67cb..d51f0a38 100644 --- a/kernel/kernel/Terminal/VirtualTTY.cpp +++ b/kernel/kernel/Terminal/VirtualTTY.cpp @@ -577,7 +577,7 @@ namespace Kernel return true; } - void VirtualTTY::update_cursor() + void VirtualTTY::after_write() { if (m_cursor_shown != m_last_cursor_shown) m_terminal_driver->set_cursor_shown(m_cursor_shown);