From 34288a5e37613bf7cfde8ebe506ab67e2bcf2e93 Mon Sep 17 00:00:00 2001
From: "Tate, Hongliang Tian" <tate.thl@antfin.com>
Date: Wed, 18 Nov 2020 14:54:35 +0000
Subject: [PATCH] Use HostFd to manage the lifetime of host OS resources

---
 src/libos/src/fs/event_file.rs                |  7 ------
 src/libos/src/fs/host_fd.rs                   | 22 ++++++++++++++-----
 .../epoll/host_file_epoller.rs                |  8 -------
 src/libos/src/net/socket/host_socket/mod.rs   |  7 ------
 4 files changed, 16 insertions(+), 28 deletions(-)

diff --git a/src/libos/src/fs/event_file.rs b/src/libos/src/fs/event_file.rs
index 56bbea11..0d99b1d8 100644
--- a/src/libos/src/fs/event_file.rs
+++ b/src/libos/src/fs/event_file.rs
@@ -49,13 +49,6 @@ extern "C" {
     fn occlum_ocall_eventfd(ret: *mut i32, init_val: u32, flags: i32) -> sgx_status_t;
 }
 
-impl Drop for EventFile {
-    fn drop(&mut self) {
-        let ret = unsafe { libc::ocall::close(self.host_fd.to_raw() as i32) };
-        assert!(ret == 0);
-    }
-}
-
 impl File for EventFile {
     fn read(&self, buf: &mut [u8]) -> Result<usize> {
         let (buf_ptr, buf_len) = buf.as_mut().as_mut_ptr_and_len();
diff --git a/src/libos/src/fs/host_fd.rs b/src/libos/src/fs/host_fd.rs
index 78db73ea..3a795b3a 100644
--- a/src/libos/src/fs/host_fd.rs
+++ b/src/libos/src/fs/host_fd.rs
@@ -5,7 +5,16 @@ use super::*;
 
 /// A unique fd from the host OS.
 ///
-/// The uniqueness property is important both
+/// There are two benefits of using `HostFd` instead of `FileDesc`.
+///
+/// 1. Uniqueness. Each instance of `HostFd` is guaranteed to have a different
+/// value. The uniqueness property makes it possible to use `HostFd` as keys of
+/// a hash table.
+///
+/// 2. Resource Acquisition Is Initialization (RAII). The acquisition and release
+/// of the host resource represented by a host fd is bound to the lifetime
+/// of the corresponding instance of `HostFd`. This makes resource management
+/// simpler and more robust.
 #[derive(Debug, PartialEq, Eq, Hash)]
 pub struct HostFd(FileDesc);
 
@@ -22,11 +31,12 @@ impl HostFd {
 
 impl Drop for HostFd {
     fn drop(&mut self) {
-        HOST_FD_REGISTRY
-            .lock()
-            .unwrap()
-            .unregister(self.to_raw())
-            .unwrap();
+        let raw_fd = self.to_raw();
+        HOST_FD_REGISTRY.lock().unwrap().unregister(raw_fd).unwrap();
+        // Note that close MUST be done after unregistering
+        unsafe {
+            libc::ocall::close(raw_fd as i32);
+        }
     }
 }
 
diff --git a/src/libos/src/net/io_multiplexing/epoll/host_file_epoller.rs b/src/libos/src/net/io_multiplexing/epoll/host_file_epoller.rs
index 2d3286c7..19102a5d 100644
--- a/src/libos/src/net/io_multiplexing/epoll/host_file_epoller.rs
+++ b/src/libos/src/net/io_multiplexing/epoll/host_file_epoller.rs
@@ -180,11 +180,3 @@ impl HostFileEpoller {
         &self.host_epoll_fd
     }
 }
-
-impl Drop for HostFileEpoller {
-    fn drop(&mut self) {
-        unsafe {
-            libc::ocall::close(self.host_epoll_fd.to_raw() as i32);
-        }
-    }
-}
diff --git a/src/libos/src/net/socket/host_socket/mod.rs b/src/libos/src/net/socket/host_socket/mod.rs
index 972cf566..e2dd9e91 100644
--- a/src/libos/src/net/socket/host_socket/mod.rs
+++ b/src/libos/src/net/socket/host_socket/mod.rs
@@ -134,13 +134,6 @@ impl HostSocket {
     }
 }
 
-impl Drop for HostSocket {
-    fn drop(&mut self) {
-        let ret = unsafe { libc::ocall::close(self.host_fd.to_raw() as i32) };
-        assert!(ret == 0);
-    }
-}
-
 pub trait HostSocketType {
     fn as_host_socket(&self) -> Result<&HostSocket>;
 }