diff options
author | Andreas Kling <awesomekling@gmail.com> | 2019-01-30 18:26:19 +0100 |
---|---|---|
committer | Andreas Kling <awesomekling@gmail.com> | 2019-01-30 18:47:18 +0100 |
commit | b4e478aa508407b999766efea882e4ec4f2f3b6d (patch) | |
tree | f25cc9faa4c7e57a0d46f10532358f176164707d /Kernel | |
parent | 027d26cd5d6dbfceef0c2ade42a51068dcdcf43f (diff) | |
download | serenity-b4e478aa508407b999766efea882e4ec4f2f3b6d.zip |
Deallocate PTY's when they close.
This required a fair bit of plumbing. The CharacterDevice::close() virtual
will now be closed by ~FileDescriptor(), allowing device implementations to
do custom cleanup at that point.
One big problem remains: if the master PTY is closed before the slave PTY,
we go into crashy land.
Diffstat (limited to 'Kernel')
-rw-r--r-- | Kernel/CharacterDevice.cpp | 4 | ||||
-rw-r--r-- | Kernel/CharacterDevice.h | 1 | ||||
-rw-r--r-- | Kernel/DevPtsFS.cpp | 3 | ||||
-rw-r--r-- | Kernel/DevPtsFS.h | 2 | ||||
-rw-r--r-- | Kernel/FileDescriptor.cpp | 9 | ||||
-rw-r--r-- | Kernel/FileDescriptor.h | 2 | ||||
-rw-r--r-- | Kernel/MasterPTY.cpp | 22 | ||||
-rw-r--r-- | Kernel/MasterPTY.h | 6 | ||||
-rw-r--r-- | Kernel/PTYMultiplexer.cpp | 20 | ||||
-rw-r--r-- | Kernel/PTYMultiplexer.h | 6 | ||||
-rw-r--r-- | Kernel/Process.cpp | 12 | ||||
-rw-r--r-- | Kernel/Process.h | 3 | ||||
-rw-r--r-- | Kernel/SlavePTY.cpp | 6 | ||||
-rw-r--r-- | Kernel/SlavePTY.h | 1 | ||||
-rw-r--r-- | Kernel/VirtualFileSystem.cpp | 5 | ||||
-rw-r--r-- | Kernel/VirtualFileSystem.h | 1 | ||||
-rw-r--r-- | Kernel/init.cpp | 1 |
17 files changed, 93 insertions, 11 deletions
diff --git a/Kernel/CharacterDevice.cpp b/Kernel/CharacterDevice.cpp index b76e6caba4..77ab998dd9 100644 --- a/Kernel/CharacterDevice.cpp +++ b/Kernel/CharacterDevice.cpp @@ -10,6 +10,10 @@ RetainPtr<FileDescriptor> CharacterDevice::open(int& error, int options) return VFS::the().open(*this, error, options); } +void CharacterDevice::close() +{ +} + int CharacterDevice::ioctl(Process&, unsigned, unsigned) { return -ENOTTY; diff --git a/Kernel/CharacterDevice.h b/Kernel/CharacterDevice.h index c37cbf0600..179a739e1d 100644 --- a/Kernel/CharacterDevice.h +++ b/Kernel/CharacterDevice.h @@ -14,6 +14,7 @@ public: InodeMetadata metadata() const { return { }; } virtual RetainPtr<FileDescriptor> open(int& error, int options); + virtual void close(); virtual bool can_read(Process&) const = 0; virtual bool can_write(Process&) const = 0; diff --git a/Kernel/DevPtsFS.cpp b/Kernel/DevPtsFS.cpp index a59e31b649..4deaac06e7 100644 --- a/Kernel/DevPtsFS.cpp +++ b/Kernel/DevPtsFS.cpp @@ -1,4 +1,5 @@ -#include "DevPtsFS.h" +#include <Kernel/DevPtsFS.h> +#include <Kernel/SlavePTY.h> #include <Kernel/VirtualFileSystem.h> #include <AK/StringBuilder.h> diff --git a/Kernel/DevPtsFS.h b/Kernel/DevPtsFS.h index 6c98e6e639..94587e0031 100644 --- a/Kernel/DevPtsFS.h +++ b/Kernel/DevPtsFS.h @@ -1,10 +1,10 @@ #pragma once -#include "SlavePTY.h" #include <AK/Types.h> #include <Kernel/SyntheticFileSystem.h> class Process; +class SlavePTY; class DevPtsFS final : public SynthFS { public: diff --git a/Kernel/FileDescriptor.cpp b/Kernel/FileDescriptor.cpp index 527eda23a7..d2b1fffcb4 100644 --- a/Kernel/FileDescriptor.cpp +++ b/Kernel/FileDescriptor.cpp @@ -40,8 +40,15 @@ FileDescriptor::FileDescriptor(RetainPtr<CharacterDevice>&& device) FileDescriptor::~FileDescriptor() { - if (m_fifo) + if (m_device) { + m_device->close(); + m_device = nullptr; + } + if (m_fifo) { m_fifo->close(fifo_direction()); + m_fifo = nullptr; + } + m_inode = nullptr; } RetainPtr<FileDescriptor> FileDescriptor::clone() diff --git a/Kernel/FileDescriptor.h b/Kernel/FileDescriptor.h index 77477d2423..38195d4c1e 100644 --- a/Kernel/FileDescriptor.h +++ b/Kernel/FileDescriptor.h @@ -86,5 +86,7 @@ private: RetainPtr<FIFO> m_fifo; FIFO::Direction m_fifo_direction { FIFO::Neither }; + + bool m_closed { false }; }; diff --git a/Kernel/MasterPTY.cpp b/Kernel/MasterPTY.cpp index 5a688072c4..2f6a90896a 100644 --- a/Kernel/MasterPTY.cpp +++ b/Kernel/MasterPTY.cpp @@ -1,15 +1,18 @@ #include "MasterPTY.h" #include "SlavePTY.h" +#include "PTYMultiplexer.h" +#include <LibC/errno_numbers.h> MasterPTY::MasterPTY(unsigned index) : CharacterDevice(10, index) - , m_slave(*new SlavePTY(*this, index)) + , m_slave(adopt(*new SlavePTY(*this, index))) , m_index(index) { } MasterPTY::~MasterPTY() { + PTYMultiplexer::the().notify_master_destroyed(Badge<MasterPTY>(), m_index); } String MasterPTY::pts_name() const @@ -19,17 +22,23 @@ String MasterPTY::pts_name() const ssize_t MasterPTY::read(Process&, byte* buffer, size_t size) { + if (!m_slave && m_buffer.is_empty()) + return 0; return m_buffer.read(buffer, size); } ssize_t MasterPTY::write(Process&, const byte* buffer, size_t size) { - m_slave.on_master_write(buffer, size); + if (!m_slave) + return -EIO; + m_slave->on_master_write(buffer, size); return size; } bool MasterPTY::can_read(Process&) const { + if (!m_slave) + return true; return !m_buffer.is_empty(); } @@ -38,6 +47,15 @@ bool MasterPTY::can_write(Process&) const return true; } +void MasterPTY::notify_slave_closed(Badge<SlavePTY>) +{ + dbgprintf("MasterPTY(%u): slave closed, my retains: %u, slave retains: %u\n", m_index, retain_count(), m_slave->retain_count()); + // +1 retain for my MasterPTY::m_slave + // +1 retain for FileDescriptor::m_device + if (m_slave->retain_count() == 2) + m_slave = nullptr; +} + void MasterPTY::on_slave_write(const byte* data, size_t size) { m_buffer.write(data, size); diff --git a/Kernel/MasterPTY.h b/Kernel/MasterPTY.h index cc375feab6..29d7106927 100644 --- a/Kernel/MasterPTY.h +++ b/Kernel/MasterPTY.h @@ -1,7 +1,8 @@ #pragma once +#include <AK/Badge.h> #include <Kernel/CharacterDevice.h> -#include "DoubleBuffer.h" +#include <Kernel/DoubleBuffer.h> class SlavePTY; @@ -21,12 +22,13 @@ public: String pts_name() const; void on_slave_write(const byte*, size_t); bool can_write_from_slave() const; + void notify_slave_closed(Badge<SlavePTY>); private: // ^CharacterDevice virtual const char* class_name() const override { return "MasterPTY"; } - SlavePTY& m_slave; + RetainPtr<SlavePTY> m_slave; unsigned m_index; DoubleBuffer m_buffer; }; diff --git a/Kernel/PTYMultiplexer.cpp b/Kernel/PTYMultiplexer.cpp index 95da1b96a7..9d2a13e45b 100644 --- a/Kernel/PTYMultiplexer.cpp +++ b/Kernel/PTYMultiplexer.cpp @@ -3,10 +3,23 @@ #include <LibC/errno_numbers.h> static const unsigned s_max_pty_pairs = 8; +static PTYMultiplexer* s_the; + +PTYMultiplexer& PTYMultiplexer::the() +{ + ASSERT(s_the); + return *s_the; +} + +void PTYMultiplexer::initialize_statics() +{ + s_the = nullptr; +} PTYMultiplexer::PTYMultiplexer() : CharacterDevice(5, 2) { + s_the = this; m_freelist.ensure_capacity(s_max_pty_pairs); for (int i = s_max_pty_pairs; i > 0; --i) m_freelist.unchecked_append(i - 1); @@ -28,3 +41,10 @@ RetainPtr<FileDescriptor> PTYMultiplexer::open(int& error, int options) dbgprintf("PTYMultiplexer::open: Vending master %u\n", master->index()); return VFS::the().open(move(master), error, options); } + +void PTYMultiplexer::notify_master_destroyed(Badge<MasterPTY>, unsigned index) +{ + LOCKER(m_lock); + m_freelist.append(index); + dbgprintf("PTYMultiplexer: %u added to freelist\n", index); +} diff --git a/Kernel/PTYMultiplexer.h b/Kernel/PTYMultiplexer.h index ee338fd135..c2062c2db7 100644 --- a/Kernel/PTYMultiplexer.h +++ b/Kernel/PTYMultiplexer.h @@ -1,6 +1,7 @@ #pragma once #include <Kernel/CharacterDevice.h> +#include <AK/Badge.h> #include <AK/Lock.h> class MasterPTY; @@ -11,6 +12,9 @@ public: PTYMultiplexer(); virtual ~PTYMultiplexer() override; + static PTYMultiplexer& the(); + static void initialize_statics(); + // ^CharacterDevice virtual RetainPtr<FileDescriptor> open(int& error, int options) override; virtual ssize_t read(Process&, byte*, size_t) override { return 0; } @@ -18,6 +22,8 @@ public: virtual bool can_read(Process&) const override { return true; } virtual bool can_write(Process&) const override { return true; } + void notify_master_destroyed(Badge<MasterPTY>, unsigned index); + private: // ^CharacterDevice virtual const char* class_name() const override { return "PTYMultiplexer"; } diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 61e388cc41..13f892acab 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -735,7 +735,7 @@ void Process::sys$exit(int status) kprintf("sys$exit: %s(%u) exit with status %d\n", name().characters(), pid(), status); #endif - set_state(Dead); + die(); m_termination_status = status; m_termination_signal = 0; @@ -750,7 +750,7 @@ void Process::terminate_due_to_signal(byte signal) dbgprintf("terminate_due_to_signal %s(%u) <- %u\n", name().characters(), pid(), signal); m_termination_status = 0; m_termination_signal = signal; - set_state(Dead); + die(); } void Process::send_signal(byte signal, Process* sender) @@ -935,8 +935,8 @@ void Process::crash() ASSERT_INTERRUPTS_DISABLED(); ASSERT(state() != Dead); m_termination_signal = SIGSEGV; - set_state(Dead); dumpRegions(); + die(); Scheduler::pick_next_and_switch_now(); ASSERT_NOT_REACHED(); } @@ -2130,3 +2130,9 @@ int Process::sys$chmod(const char* pathname, mode_t mode) return error; return 0; } + +void Process::die() +{ + set_state(Dead); + m_fds.clear(); +} diff --git a/Kernel/Process.h b/Kernel/Process.h index a92e4effec..66f6676ce4 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -128,6 +128,7 @@ public: void setSelector(word s) { m_farPtr.selector = s; } void set_state(State s) { m_state = s; } + void die(); pid_t sys$setsid(); pid_t sys$getsid(pid_t); @@ -315,7 +316,7 @@ private: struct FileDescriptorAndFlags { operator bool() const { return !!descriptor; } void clear() { descriptor = nullptr; flags = 0; } - void set(RetainPtr<FileDescriptor>&& d, dword f = 0) { descriptor = move(d), flags = f; } + void set(RetainPtr<FileDescriptor>&& d, dword f = 0) { descriptor = move(d); flags = f; } RetainPtr<FileDescriptor> descriptor; dword flags { 0 }; }; diff --git a/Kernel/SlavePTY.cpp b/Kernel/SlavePTY.cpp index 5e0c66541f..9944a33a3a 100644 --- a/Kernel/SlavePTY.cpp +++ b/Kernel/SlavePTY.cpp @@ -15,6 +15,7 @@ SlavePTY::SlavePTY(MasterPTY& master, unsigned index) SlavePTY::~SlavePTY() { DevPtsFS::the().unregister_slave_pty(*this); + VFS::the().unregister_character_device(*this); } String SlavePTY::tty_name() const @@ -37,3 +38,8 @@ bool SlavePTY::can_write(Process&) const { return m_master.can_write_from_slave(); } + +void SlavePTY::close() +{ + m_master.notify_slave_closed(Badge<SlavePTY>()); +} diff --git a/Kernel/SlavePTY.h b/Kernel/SlavePTY.h index 4d7fcaf147..a2a95cf051 100644 --- a/Kernel/SlavePTY.h +++ b/Kernel/SlavePTY.h @@ -22,6 +22,7 @@ private: // ^CharacterDevice virtual bool can_write(Process&) const override; virtual const char* class_name() const override { return "SlavePTY"; } + virtual void close() override; friend class MasterPTY; SlavePTY(MasterPTY&, unsigned index); diff --git a/Kernel/VirtualFileSystem.cpp b/Kernel/VirtualFileSystem.cpp index 5b187f9232..261fdc9493 100644 --- a/Kernel/VirtualFileSystem.cpp +++ b/Kernel/VirtualFileSystem.cpp @@ -510,6 +510,11 @@ void VFS::register_character_device(CharacterDevice& device) m_character_devices.set(encodedDevice(device.major(), device.minor()), &device); } +void VFS::unregister_character_device(CharacterDevice& device) +{ + m_character_devices.remove(encodedDevice(device.major(), device.minor())); +} + void VFS::for_each_mount(Function<void(const Mount&)> callback) const { for (auto& mount : m_mounts) { diff --git a/Kernel/VirtualFileSystem.h b/Kernel/VirtualFileSystem.h index 23d9c37a96..563a2e89e9 100644 --- a/Kernel/VirtualFileSystem.h +++ b/Kernel/VirtualFileSystem.h @@ -72,6 +72,7 @@ public: bool chmod(const String& path, mode_t, Inode& base, int& error); void register_character_device(CharacterDevice&); + void unregister_character_device(CharacterDevice&); size_t mount_count() const { return m_mounts.size(); } void for_each_mount(Function<void(const Mount&)>) const; diff --git a/Kernel/init.cpp b/Kernel/init.cpp index 37d9bdde11..0847c8c243 100644 --- a/Kernel/init.cpp +++ b/Kernel/init.cpp @@ -142,6 +142,7 @@ void init() gdt_init(); idt_init(); + PTYMultiplexer::initialize_statics(); VFS::initialize_globals(); vfs = new VFS; |