diff --git a/src/libos/src/fs/file_ops/close.rs b/src/libos/src/fs/file_ops/close.rs index f12a6bbd..2dcfe5b5 100644 --- a/src/libos/src/fs/file_ops/close.rs +++ b/src/libos/src/fs/file_ops/close.rs @@ -4,6 +4,12 @@ pub fn do_close(fd: FileDesc) -> Result<()> { debug!("close: fd: {}", fd); let current = current!(); let mut files = current.files().lock().unwrap(); - files.del(fd)?; + let file = files.del(fd)?; + // Deadlock note: EpollFile's drop method needs to access file table. So + // if the drop method is invoked inside the del method, then there will be + // a deadlock. + // TODO: make FileTable a struct of internal mutability to avoid deadlock. + drop(files); + drop(file); Ok(()) } diff --git a/src/libos/src/fs/file_table.rs b/src/libos/src/fs/file_table.rs index 5ce2766c..df9cae80 100644 --- a/src/libos/src/fs/file_table.rs +++ b/src/libos/src/fs/file_table.rs @@ -1,12 +1,15 @@ use super::*; +use crate::events::{Event, Notifier}; + pub type FileDesc = u32; -#[derive(Debug, Default, Clone)] +#[derive(Debug)] #[repr(C)] pub struct FileTable { table: Vec>, num_fds: usize, + notifier: FileTableNotifier, } impl FileTable { @@ -14,6 +17,7 @@ impl FileTable { FileTable { table: Vec::with_capacity(4), num_fds: 0, + notifier: FileTableNotifier::new(), } } @@ -130,6 +134,7 @@ impl FileTable { match del_table_entry { Some(del_table_entry) => { self.num_fds -= 1; + self.broadcast_del(fd); Ok(del_table_entry.file) } None => return_errno!(EBADF, "Invalid file descriptor"), @@ -138,7 +143,8 @@ impl FileTable { /// Remove file descriptors that are close-on-spawn pub fn close_on_spawn(&mut self) { - for entry in self.table.iter_mut() { + let mut deleted_fds = Vec::new(); + for (fd, entry) in self.table.iter_mut().enumerate() { let need_close = if let Some(entry) = entry { entry.close_on_spawn } else { @@ -146,12 +152,51 @@ impl FileTable { }; if need_close { *entry = None; + deleted_fds.push(fd as FileDesc); self.num_fds -= 1; } } + + for fd in deleted_fds { + self.broadcast_del(fd); + } + } + + pub fn notifier(&self) -> &FileTableNotifier { + &self.notifier + } + + fn broadcast_del(&self, fd: FileDesc) { + let del_event = FileTableEvent::Del(fd); + self.notifier.broadcast(&del_event); } } +impl Clone for FileTable { + fn clone(&self) -> Self { + FileTable { + table: self.table.clone(), + num_fds: self.num_fds, + notifier: FileTableNotifier::new(), + } + } +} + +impl Default for FileTable { + fn default() -> Self { + FileTable::new() + } +} + +#[derive(Debug, Clone, Copy)] +pub enum FileTableEvent { + Del(FileDesc), +} + +impl Event for FileTableEvent {} + +pub type FileTableNotifier = Notifier; + #[derive(Debug, Clone)] pub struct FileTableEntry { file: FileRef, diff --git a/src/libos/src/fs/mod.rs b/src/libos/src/fs/mod.rs index a37eddf9..871e2b9a 100644 --- a/src/libos/src/fs/mod.rs +++ b/src/libos/src/fs/mod.rs @@ -18,7 +18,7 @@ pub use self::file_ops::{ occlum_ocall_ioctl, AccessMode, BuiltinIoctlNum, CreationFlags, FileMode, Flock, FlockType, IfConf, IoctlCmd, Stat, StatusFlags, StructuredIoctlArgType, StructuredIoctlNum, }; -pub use self::file_table::{FileDesc, FileTable}; +pub use self::file_table::{FileDesc, FileTable, FileTableEvent, FileTableNotifier}; pub use self::fs_view::FsView; pub use self::host_fd::HostFd; pub use self::inode_file::{AsINodeFile, INodeExt, INodeFile}; diff --git a/src/libos/src/net/io_multiplexing/epoll/epoll_file.rs b/src/libos/src/net/io_multiplexing/epoll/epoll_file.rs index 11b0c17b..5b159337 100644 --- a/src/libos/src/net/io_multiplexing/epoll/epoll_file.rs +++ b/src/libos/src/net/io_multiplexing/epoll/epoll_file.rs @@ -12,11 +12,18 @@ use super::epoll_waiter::EpollWaiter; use super::host_file_epoller::HostFileEpoller; use super::{EpollCtl, EpollEvent, EpollFlags}; use crate::events::{Observer, Waiter, WaiterQueue}; -use crate::fs::{AtomicIoEvents, File, HostFd, IoEvents, IoNotifier}; +use crate::fs::{ + AtomicIoEvents, File, FileTableEvent, FileTableNotifier, HostFd, IoEvents, IoNotifier, +}; use crate::prelude::*; // TODO: Prevent two epoll files from monitoring each other, which may cause // deadlock in the current implementation. +// TODO: Fix unreliable EpollFiles after process spawning. EpollFile is connected +// with the current process's file table by regitering itself as an observer +// to the file table. But if an EpollFile is cloned or inherited by a child +// process, then this EpollFile still has connection with the parent process's +// file table, which is problematic. /// A file that provides epoll API. /// @@ -70,7 +77,7 @@ impl EpollFile { let weak_self = Default::default(); let host_events = Atomic::new(IoEvents::empty()); - Self { + let arc_self = Self { interest, ready, waiters, @@ -79,7 +86,10 @@ impl EpollFile { weak_self, host_events, } - .wrap_self() + .wrap_self(); + + arc_self.register_to_file_table(); + arc_self } fn wrap_self(self) -> Arc { @@ -95,6 +105,20 @@ impl EpollFile { strong_self } + fn register_to_file_table(&self) { + let weak_observer = self.weak_self.clone() as Weak>; + let thread = current!(); + let file_table = thread.files().lock().unwrap(); + file_table.notifier().register(weak_observer, None, None); + } + + fn unregister_from_file_table(&self) { + let weak_observer = self.weak_self.clone() as Weak>; + let thread = current!(); + let file_table = thread.files().lock().unwrap(); + file_table.notifier().unregister(&weak_observer); + } + pub fn control(&self, cmd: &EpollCtl) -> Result<()> { debug!("epoll control: cmd = {:?}", cmd); @@ -432,6 +456,8 @@ impl Drop for EpollFile { notifier.unregister(&self_observer); } }); + + self.unregister_from_file_table(); } } @@ -450,6 +476,13 @@ impl Observer for EpollFile { } } +impl Observer for EpollFile { + fn on_event(&self, event: &FileTableEvent, _metadata: &Option>) { + let FileTableEvent::Del(fd) = event; + let _ = self.del_interest(*fd); + } +} + impl fmt::Debug for EpollFile { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("EpollFile")