kernel: Separate FileDescriptors and OpenFileDescription

Also, add a did_close() callback for OpenFileDescriptions losing all their references.

kernel/src/fs/VFS.h

@@ -92,6 +92,10 @@ namespace VFS
             return err(ENOTTY);
         }
 
+        virtual void did_close()
+        {
+        }
+
         // Directory-specific methods
         virtual Result<SharedPtr<Inode>> find(const char* name) const = 0;
 

kernel/src/sys/exec.cpp

@@ -92,11 +92,7 @@ Result<u64> sys_execve(Registers* regs, SyscallArgs args)
     {
         auto& descriptor = current->fd_table[i];
         if (!descriptor.has_value()) continue;
-        if (descriptor->flags & O_CLOEXEC)
+        if (descriptor->description->flags & O_CLOEXEC) { descriptor = {}; }
-        {
-            descriptor->inode->remove_handle();
-            descriptor = {};
-        }
     }
 
     if (VFS::is_setuid(inode)) current->auth.euid = current->auth.suid = inode->uid();
@@ -148,11 +144,7 @@ Result<u64> sys_fork(Registers* regs, SyscallArgs)
     thread->umask = current->umask;
     thread->parent = current;
 
-    for (int i = 0; i < FD_MAX; i++)
+    for (int i = 0; i < FD_MAX; i++) { thread->fd_table[i] = current->fd_table[i]; }
-    {
-        thread->fd_table[i] = current->fd_table[i];
-        if (current->fd_table[i].has_value()) current->fd_table[i]->inode->add_handle();
-    }
 
     image->apply(thread);
 

kernel/src/sys/file.cpp

@@ -26,7 +26,7 @@ Result<u64> sys_read(Registers* regs, SyscallArgs args)
 
     if (!descriptor.is_readable()) return err(EBADF);
 
-    while (descriptor.inode->blocking())
+    while (descriptor.inode()->blocking())
     {
         if (descriptor.should_block()) kernel_sleep(10);
         else
@@ -40,9 +40,9 @@ Result<u64> sys_read(Registers* regs, SyscallArgs args)
         }
     }
 
-    usize nread = TRY(descriptor.inode->read(buf, descriptor.offset, size));
+    usize nread = TRY(descriptor.inode()->read(buf, descriptor.offset, size));
 
-    if (VFS::is_seekable(descriptor.inode)) descriptor.offset += nread;
+    if (VFS::is_seekable(descriptor.inode())) descriptor.offset += nread;
 
     return nread;
 }
@@ -63,11 +63,12 @@ Result<u64> sys_write(Registers*, SyscallArgs args)
 
     if (!descriptor.is_writable()) return err(EBADF);
 
-    if (descriptor.should_append() && VFS::is_seekable(descriptor.inode)) descriptor.offset = descriptor.inode->size();
+    if (descriptor.should_append() && VFS::is_seekable(descriptor.inode()))
+        descriptor.offset = descriptor.inode()->size();
 
-    usize nwritten = TRY(descriptor.inode->write(buf, descriptor.offset, size));
+    usize nwritten = TRY(descriptor.inode()->write(buf, descriptor.offset, size));
 
-    if (VFS::is_seekable(descriptor.inode)) descriptor.offset += nwritten;
+    if (VFS::is_seekable(descriptor.inode())) descriptor.offset += nwritten;
 
     return nwritten;
 }
@@ -82,9 +83,9 @@ Result<u64> sys_lseek(Registers*, SyscallArgs args)
 
     auto& descriptor = *TRY(current->resolve_fd(fd));
 
-    if (descriptor.inode->type() == VFS::InodeType::FIFO) return err(ESPIPE);
+    if (descriptor.inode()->type() == VFS::InodeType::FIFO) return err(ESPIPE);
 
-    if (!VFS::is_seekable(descriptor.inode)) return descriptor.offset;
+    if (!VFS::is_seekable(descriptor.inode())) return descriptor.offset;
 
     off_t new_offset;
 
@@ -92,7 +93,7 @@ Result<u64> sys_lseek(Registers*, SyscallArgs args)
     {
     case SEEK_SET: new_offset = offset; break;
     case SEEK_CUR: new_offset = TRY(safe_add((long)descriptor.offset, offset)); break;
-    case SEEK_END: new_offset = TRY(safe_add((long)descriptor.inode->size(), offset)); break;
+    case SEEK_END: new_offset = TRY(safe_add((long)descriptor.inode()->size(), offset)); break;
     default: return err(EINVAL);
     }
 
@@ -123,28 +124,28 @@ Result<u64> sys_fcntl(Registers*, SyscallArgs args)
 
         current->fd_table[new_fd] = descriptor;
 
-        if (is_cloexec) current->fd_table[new_fd]->flags |= O_CLOEXEC;
+        if (is_cloexec) current->fd_table[new_fd]->flags() |= O_CLOEXEC;
         else
-            current->fd_table[new_fd]->flags &= ~O_CLOEXEC;
+            current->fd_table[new_fd]->flags() &= ~O_CLOEXEC;
 
         return (u64)new_fd;
     }
-    case F_GETFD: return (u64) !!(descriptor.flags & O_CLOEXEC);
+    case F_GETFD: return (u64) !!(descriptor.flags() & O_CLOEXEC);
     case F_SETFD: {
         int arg = (int)args[2];
-        if (arg == FD_CLOEXEC) descriptor.flags |= O_CLOEXEC;
+        if (arg == FD_CLOEXEC) descriptor.flags() |= O_CLOEXEC;
         else
-            descriptor.flags &= ~O_CLOEXEC;
+            descriptor.flags() &= ~O_CLOEXEC;
         return 0;
     }
-    case F_GETFL: return (u64)(descriptor.flags & ~O_CLOEXEC);
+    case F_GETFL: return (u64)(descriptor.flags() & ~O_CLOEXEC);
     case F_SETFL: {
         int arg = (int)args[2];
 
-        descriptor.flags &= ~(O_APPEND | O_NONBLOCK);
+        descriptor.flags() &= ~(O_APPEND | O_NONBLOCK);
         arg &= (O_APPEND | O_NONBLOCK);
 
-        descriptor.flags |= arg;
+        descriptor.flags() |= arg;
 
         return 0;
     }
@@ -161,7 +162,7 @@ Result<u64> sys_ioctl(Registers*, SyscallArgs args)
     Thread* current = Scheduler::current();
     auto& descriptor = *TRY(current->resolve_fd(fd));
 
-    return descriptor.inode->ioctl(request, arg);
+    return descriptor.inode()->ioctl(request, arg);
 }
 
 Result<u64> sys_isatty(Registers*, SyscallArgs args)
@@ -171,7 +172,7 @@ Result<u64> sys_isatty(Registers*, SyscallArgs args)
     Thread* current = Scheduler::current();
     auto& descriptor = *TRY(current->resolve_fd(fd));
 
-    return descriptor.inode->isatty();
+    return descriptor.inode()->isatty();
 }
 
 Result<u64> sys_dup2(Registers*, SyscallArgs args)
@@ -188,7 +189,7 @@ Result<u64> sys_dup2(Registers*, SyscallArgs args)
     if (newfd == oldfd) return (u64)newfd;
 
     current->fd_table[newfd] = descriptor;
-    current->fd_table[newfd]->flags &= ~O_CLOEXEC;
+    current->fd_table[newfd]->flags() &= ~O_CLOEXEC;
 
     return (u64)newfd;
 }
@@ -210,8 +211,8 @@ Result<u64> sys_pipe(Registers*, SyscallArgs args)
 
     TRY(Pipe::create(rpipe, wpipe));
 
-    current->fd_table[rfd] = FileDescriptor { rpipe, 0, O_RDONLY };
+    current->fd_table[rfd] = FileDescriptor { TRY(make_shared<OpenFileDescription>(rpipe, O_RDONLY)), 0 };
-    current->fd_table[wfd] = FileDescriptor { wpipe, 0, O_WRONLY };
+    current->fd_table[wfd] = FileDescriptor { TRY(make_shared<OpenFileDescription>(wpipe, O_WRONLY)), 0 };
 
     return 0;
 }

kernel/src/sys/getdents.cpp

@@ -14,13 +14,13 @@ Result<u64> sys_getdents(Registers*, SyscallArgs args)
 
     auto& descriptor = *TRY(current->resolve_fd(fd));
 
-    if (descriptor.inode->type() != VFS::InodeType::Directory) return err(ENOTDIR);
+    if (descriptor.inode()->type() != VFS::InodeType::Directory) return err(ENOTDIR);
 
     usize nwrite = 0;
     while (nwrite < count)
     {
         VFS::DirectoryEntry entry;
-        bool ok = descriptor.inode->get(descriptor.offset).try_set_value(entry);
+        bool ok = descriptor.inode()->get(descriptor.offset).try_set_value(entry);
         if (!ok) break;
 
         descriptor.offset++;

kernel/src/sys/open.cpp

@@ -82,9 +82,7 @@ Result<u64> sys_openat(Registers*, SyscallArgs args)
     kdbgln("openat: opening file %s from dirfd %d, flags %d, mode %#o = fd %d", path.chars(), dirfd, flags, mode, fd);
 #endif
 
-    inode->add_handle();
+    current->fd_table[fd] = FileDescriptor { TRY(make_shared<OpenFileDescription>(inode, flags & FLAGS_TO_KEEP)), 0 };
-
-    current->fd_table[fd] = FileDescriptor { inode, 0, flags & FLAGS_TO_KEEP };
 
     return (u64)fd;
 }
@@ -100,8 +98,6 @@ Result<u64> sys_close(Registers*, SyscallArgs args)
 
     if (!descriptor.has_value()) return err(EBADF);
 
-    descriptor->inode->remove_handle();
-
     descriptor = {};
 
     return 0;

kernel/src/sys/socket.cpp

@@ -20,7 +20,7 @@ Result<u64> sys_socket(Registers*, SyscallArgs args)
 
     int fd = TRY(current->allocate_fd(0));
 
-    current->fd_table[fd] = FileDescriptor { socket, 0, O_RDWR };
+    current->fd_table[fd] = FileDescriptor { TRY(make_shared<OpenFileDescription>(socket, O_RDWR)), 0 };
 
     return fd;
 }
@@ -37,7 +37,7 @@ Result<u64> sys_bind(Registers*, SyscallArgs args)
 
     auto* current = Scheduler::current();
 
-    auto inode = TRY(current->resolve_fd(sockfd))->inode;
+    auto inode = TRY(current->resolve_fd(sockfd))->inode();
 
     if (inode->type() != VFS::InodeType::Socket) return err(ENOTSOCK);
 

kernel/src/thread/Scheduler.cpp

@@ -192,11 +192,6 @@ namespace Scheduler
         {
             auto stack = thread->kernel_stack;
             MemoryManager::unmap_owned_and_free_vm(stack.bottom(), stack.bytes() / ARCH_PAGE_SIZE).release_value();
-
-            for (int i = 0; i < FD_MAX; i++)
-            {
-                if (thread->fd_table[i].has_value()) thread->fd_table[i]->inode->remove_handle();
-            }
         }
 
         delete thread;

kernel/src/thread/Thread.cpp

@@ -71,11 +71,11 @@ Result<SharedPtr<VFS::Inode>> Thread::resolve_atfile(int dirfd, const String& pa
 
     auto descriptor = TRY(resolve_fd(dirfd));
 
-    if (parent_inode) *parent_inode = descriptor->inode;
+    if (parent_inode) *parent_inode = descriptor->inode();
 
-    if (path.is_empty() && allow_empty_path) return descriptor->inode;
+    if (path.is_empty() && allow_empty_path) return descriptor->inode();
 
-    return VFS::resolve_path(path.chars(), this->auth, descriptor->inode, follow_last_symlink);
+    return VFS::resolve_path(path.chars(), this->auth, descriptor->inode(), follow_last_symlink);
 }
 
 [[noreturn]] void Thread::exit_and_signal_parent(int _status)
@@ -215,22 +215,33 @@ void Thread::send_signal(int signo)
     }
 }
 
+OpenFileDescription::OpenFileDescription(SharedPtr<VFS::Inode> ino, int fl) : inode(ino), flags(fl)
+{
+    inode->add_handle();
+}
+
+OpenFileDescription::~OpenFileDescription()
+{
+    inode->remove_handle();
+    inode->did_close();
+}
+
 bool FileDescriptor::should_append()
 {
-    return flags & O_APPEND;
+    return description->flags & O_APPEND;
 }
 
 bool FileDescriptor::should_block()
 {
-    return !(flags & O_NONBLOCK);
+    return !(description->flags & O_NONBLOCK);
 }
 
 bool FileDescriptor::is_readable()
 {
-    return flags & O_RDONLY;
+    return description->flags & O_RDONLY;
 }
 
 bool FileDescriptor::is_writable()
 {
-    return flags & O_WRONLY;
+    return description->flags & O_WRONLY;
 }

kernel/src/thread/Thread.h

@@ -28,16 +28,34 @@ enum class ThreadState
     Dying
 };
 
-struct FileDescriptor
+struct OpenFileDescription
 {
     SharedPtr<VFS::Inode> inode;
-    usize offset { 0 };
     int flags { 0 };
 
+    OpenFileDescription(SharedPtr<VFS::Inode>, int);
+    ~OpenFileDescription();
+};
+
+struct FileDescriptor
+{
+    SharedPtr<OpenFileDescription> description;
+    usize offset { 0 };
+
     bool should_append();
     bool should_block();
     bool is_writable();
     bool is_readable();
+
+    SharedPtr<VFS::Inode> inode()
+    {
+        return description->inode;
+    }
+
+    int& flags()
+    {
+        return description->flags;
+    }
 };
 
 static constexpr int FD_MAX = 64;