vfs: FsNode::release() implementation and weak ref to VFS node in file descriptors

ableos/src/filesystem/ext2.rs

@@ -4,12 +4,13 @@
  * SPDX-License-Identifier: MPL-2.0
  */
 
+use alloc::sync::Arc;
 use ext2::fs::{sync::Synced, Ext2};
 use ext2::sector::SectorSize;
 use ext2::volume::Volume;
 
 use super::errors::FsError;
-use super::{FsResult as Result, StorageDevice, DirectoryEntry, FsNode, FileDescriptor};
+use super::{DirectoryEntry, FileDescriptor, FsNode, FsResult as Result, StorageDevice};
 
 pub struct Ext2StorageDevice<S, V>
 where
@@ -36,32 +37,33 @@ where
     S: SectorSize + Send,
     V: Volume<u8, S> + Send,
 {
-    fn open(&self, node: &super::FsNode /* TODO: flags */) -> Result<FileDescriptor> {
+    fn open(&self, node: Arc<FsNode> /* TODO: flags */) -> Result<FileDescriptor> {
         let inode = self
             .fs
             .inode_nth(node.inode as usize)
             .ok_or_else(|| FsError::InodeNotFound)?;
 
-        Ok(FileDescriptor::new(node.flags, inode.size(), node.inode))
+        Ok(FileDescriptor::new(Arc::downgrade(&node), node.flags, inode.size(), node.inode))
     }
 
-    fn release(&self, node: &super::FsNode) -> Result<()> {
+    fn release(&self, _node: FsNode) -> Result<()> {
+        // TODO: flush to disk and whatnot
+        Ok(())
+    }
+
+    fn read(&self, _node: &FsNode, _offset: usize, _size: usize) -> Result<Box<[u8]>> {
         todo!()
     }
 
-    fn read(&self, node: &super::FsNode, offset: usize, size: usize) -> Result<Box<[u8]>> {
+    fn write(&self, _node: &FsNode, _offset: usize, _buffer: Box<[u8]>) -> Result<()> {
         todo!()
     }
 
-    fn write(&self, node: &super::FsNode, offset: usize, buffer: Box<[u8]>) -> Result<()> {
+    fn read_dir(&self, _node: &FsNode, _index: usize) -> Result<DirectoryEntry> {
         todo!()
     }
 
-    fn read_dir(&self, node: &super::FsNode, index: usize) -> Result<DirectoryEntry> {
-        todo!()
-    }
-
-    fn find_dir(&self, node: &super::FsNode, name: &str) -> Result<FsNode> {
+    fn find_dir(&self, _node: &FsNode, _name: &str) -> Result<FsNode> {
         todo!()
     }
 }

ableos/src/filesystem/mod.rs

@@ -7,7 +7,7 @@
 pub mod errors;
 pub mod ext2;
 
-use alloc::rc::Weak;
+use alloc::sync::{Weak, Arc};
 use bitflags::bitflags;
 
 use crate::{handle::Handle, KERNEL_STATE};
@@ -21,8 +21,8 @@ pub trait StorageDevice
 where
     Self: Send,
 {
-    fn open(&self, node: &FsNode /* TODO: flags */) -> Result<FileDescriptor>;
-    fn release(&self, node: &FsNode) -> Result<()>;
+    fn open(&self, node: Arc<FsNode> /* TODO: flags */) -> Result<FileDescriptor>;
+    fn release(&self, node: FsNode) -> Result<()>;
     fn read(&self, node: &FsNode, offset: usize, size: usize) -> Result<Box<[u8]>>;
     fn write(&self, node: &FsNode, offset: usize, buffer: Box<[u8]>) -> Result<()>;
     fn read_dir(&self, node: &FsNode, index: usize) -> Result<DirectoryEntry>;
@@ -34,10 +34,10 @@ where
 /// VFS nodes are created whenever a file that doesn't have an open VFS node is
 /// opened. When there are no open file descriptors to a file, the associated
 /// VFS node is dropped.
+#[derive(Debug)]
 pub struct FsNode {
     flags: FsFlags,
     length: usize,         // in bytes
-    fd_count: usize,       // count of open file descriptors
     inode: usize,          // implementation specific identifier for the node
     device_handle: Handle, // uniquely assigned device handle
     ptr: Weak<FsNode>,     // used by mountpoints and symlinks
@@ -56,7 +56,6 @@ impl FsNode {
         Self {
             flags,
             length,
-            fd_count: 0,
             inode,
             device_handle,
             ptr,
@@ -65,27 +64,31 @@ impl FsNode {
 
     /// This method opens a new file descriptor for this VFS node.
     // TODO: make this take flags
-    pub fn open(&mut self) -> Result<Handle> {
-        let state = KERNEL_STATE.lock();
+    pub fn open(self: Arc<Self>) -> Result<Handle> {
+        let mut state = KERNEL_STATE.lock();
         let device = state
             .get_storage_device(self.device_handle)
             .ok_or_else(|| FsError::InvalidDevice)?;
         let fd = device.open(self)?;
-        let mut kernel_state = KERNEL_STATE.lock();
-        let handle = kernel_state.add_file_descriptor(fd);
+        let handle = state.open_file_descriptor(fd);
 
         Ok(handle)
     }
 
-    /// This method is for closing the VFS node, which is done when no open file
-    /// descriptors for this file are left.
-    pub fn release(&self) -> Result<()> {
+    /// This method is for closing the VFS node, which is usually done when no
+    /// open file descriptors for this file are left. File descriptors have
+    /// weak references to the VFS node, meaning it is okay to close the VFS
+    /// node before all file descriptors are closed.
+    pub fn release(self) -> Result<()> {
         let state = KERNEL_STATE.lock();
         let device = state
             .get_storage_device(self.device_handle)
             .ok_or_else(|| FsError::InvalidDevice)?;
 
-        device.release(self)
+        device.release(self)?;
+        // self is moved into device.release, and at the end of it, self should
+        // be dropped.
+        Ok(())
     }
 
     /// This method reads from the VFS node without opening a new file
@@ -131,6 +134,12 @@ impl FsNode {
     }
 }
 
+impl Drop for FsNode {
+    fn drop(&mut self) {
+        trace!("dropping: {self:#?}")
+    }
+}
+
 bitflags! {
     /// Flags associated with VFS nodes and file descriptors
     ///
@@ -147,25 +156,38 @@ bitflags! {
 }
 
 /// A file descriptor.
+#[derive(Debug)]
 pub struct FileDescriptor {
+    vfs_node: Weak<FsNode>, // ptr to the associated VFS node
     flags: FsFlags,
     length: usize, // in bytes
-    inode: usize, // implementation specific identifier for the node
-    // TODO: permissions mask
-    // TODO: owning user/group
-    // TODO: read file position?
-    // FIXME: I'm not sure if this needs to have a ptr to the VFS node,
-    //        figure that out.
+    inode: usize,  // implementation specific identifier for the node
+                   // TODO: permissions mask
+                   // TODO: owning user/group
+                   // TODO: read file position?
 }
 
 impl FileDescriptor {
-    fn new(flags: FsFlags, length: usize, inode: usize) -> Self {
+    fn new(vfs_node: Weak<FsNode>, flags: FsFlags, length: usize, inode: usize) -> Self {
         Self {
+            vfs_node,
             flags,
             length,
             inode,
         }
     }
+
+    /// This method is for closing the file descriptor.
+    pub fn close(fd_handle: Handle) {
+        let mut state = KERNEL_STATE.lock();
+        state.close_file_descriptor(fd_handle);
+    }
+}
+
+impl Drop for FileDescriptor {
+    fn drop(&mut self) {
+        trace!("dropping: {self:#?}")
+    }
 }
 
 pub struct DirectoryEntry {

ableos/src/kernel_state.rs

@@ -2,7 +2,7 @@ use hashbrown::HashMap;
 use spin::Lazy;
 
 use crate::{
-    filesystem::{StorageDevice, FileDescriptor},
+    filesystem::{FileDescriptor, StorageDevice},
     handle::{Handle, HandleResource},
 };
 
@@ -40,12 +40,16 @@ impl KernelInternalState {
         self.storage_devices.get(&handle).map(|d| &**d)
     }
 
-    pub fn add_file_descriptor(&mut self, fd: FileDescriptor) -> Handle {
+    pub fn open_file_descriptor(&mut self, fd: FileDescriptor) -> Handle {
         let handle = Handle::new(HandleResource::FileDescriptor);
         self.fd_table.insert(handle, fd);
         handle
     }
 
+    pub fn close_file_descriptor(&mut self, fd_handle: Handle) {
+        self.fd_table.remove(&fd_handle);
+    }
+
     pub fn shutdown(&mut self) {
         self.should_shutdown = true;
     }