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.
This commit is contained in:
Andreas Kling 2019-01-30 18:26:19 +01:00
parent 027d26cd5d
commit b4e478aa50
19 changed files with 104 additions and 12 deletions

7
AK/Badge.h Normal file
View file

@ -0,0 +1,7 @@
#pragma once
template<typename T>
class Badge {
friend T;
Badge() { }
};

View file

@ -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;

View file

@ -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;

View file

@ -1,4 +1,5 @@
#include "DevPtsFS.h"
#include <Kernel/DevPtsFS.h>
#include <Kernel/SlavePTY.h>
#include <Kernel/VirtualFileSystem.h>
#include <AK/StringBuilder.h>

View file

@ -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:

View file

@ -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()

View file

@ -86,5 +86,7 @@ private:
RetainPtr<FIFO> m_fifo;
FIFO::Direction m_fifo_direction { FIFO::Neither };
bool m_closed { false };
};

View file

@ -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);

View file

@ -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;
};

View file

@ -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);
}

View file

@ -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"; }

View file

@ -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();
}

View file

@ -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 };
};

View file

@ -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>());
}

View file

@ -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);

View file

@ -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) {

View file

@ -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;

View file

@ -142,6 +142,7 @@ void init()
gdt_init();
idt_init();
PTYMultiplexer::initialize_statics();
VFS::initialize_globals();
vfs = new VFS;

View file

@ -97,7 +97,10 @@ int main(int, char**)
perror("read(ptm)");
continue;
}
assert(nread != 0);
if (nread == 0) {
dbgprintf("Terminal: EOF on master pty, closing.\n");
break;
}
for (ssize_t i = 0; i < nread; ++i)
terminal.on_char(buffer[i]);
terminal.update();