Unregister a file from epoll files when the file is closed

Usually, files are unregistered from an epoll file via the EPOLL_CTL_DEL command
explicitly. But for the sake of users' convenience, Linux supports
unregistering a file automatically from the epoll files that monitor the file
when the file is closed. This commit adds this capability.
This commit is contained in:
Tate, Hongliang Tian 2020-11-18 10:48:31 +00:00 committed by Zongmin.Gu
parent 787df74be0
commit 83ce318f6c
4 changed files with 91 additions and 7 deletions

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

@ -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<Option<FileTableEntry>>,
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<FileTableEvent>;
#[derive(Debug, Clone)]
pub struct FileTableEntry {
file: FileRef,

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

@ -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<Self> {
@ -95,6 +105,20 @@ impl EpollFile {
strong_self
}
fn register_to_file_table(&self) {
let weak_observer = self.weak_self.clone() as Weak<dyn Observer<_>>;
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<dyn Observer<_>>;
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<IoEvents> for EpollFile {
}
}
impl Observer<FileTableEvent> for EpollFile {
fn on_event(&self, event: &FileTableEvent, _metadata: &Option<Weak<dyn Any + Send + Sync>>) {
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")