Fix a kernel deadlock in empty().

For more information, see b3b5d1b864

src/context/memory.rs

@@ -35,6 +35,17 @@ pub fn page_flags(flags: MapFlags) -> PageFlags<RmmA> {
         //TODO: PROT_READ
 }
 
+pub struct UnmapResult {
+    pub file_desc: Option<FileDescriptor>,
+}
+impl Drop for UnmapResult {
+    fn drop(&mut self) {
+        if let Some(fd) = self.file_desc.take() {
+            let _ = fd.close();
+        }
+    }
+}
+
 #[derive(Debug, Default)]
 pub struct UserGrants {
     pub inner: BTreeSet<Grant>,
@@ -486,7 +497,7 @@ impl Grant {
         self.flags
     }
 
-    pub fn unmap(mut self) {
+    pub fn unmap(mut self) -> UnmapResult {
         assert!(self.mapped);
 
         let mut active_table = unsafe { ActivePageTable::new(self.start_address().kind()) };
@@ -507,16 +518,13 @@ impl Grant {
 
         flush_all.flush();
 
-        if let Some(desc) = self.desc_opt.take() {
-            println!("Grant::unmap: close desc {:?}", desc);
-            //TODO: This imposes a large cost on unmapping, but that cost cannot be avoided without modifying fmap and funmap
-            let _ = desc.close();
-        }
-
         self.mapped = false;
+
+        // TODO: This imposes a large cost on unmapping, but that cost cannot be avoided without modifying fmap and funmap
+        UnmapResult { file_desc: self.desc_opt.take() }
     }
 
-    pub fn unmap_inactive(mut self, new_table: &mut InactivePageTable) {
+    pub fn unmap_inactive(mut self, new_table: &mut InactivePageTable) -> UnmapResult {
         assert!(self.mapped);
 
         let start_page = Page::containing_address(self.start_address());
@@ -533,13 +541,10 @@ impl Grant {
 
         ipi(IpiKind::Tlb, IpiTarget::Other);
 
-        if let Some(desc) = self.desc_opt.take() {
-            println!("Grant::unmap_inactive: close desc {:?}", desc);
-            //TODO: This imposes a large cost on unmapping, but that cost cannot be avoided without modifying fmap and funmap
-            let _ = desc.close();
-        }
-
         self.mapped = false;
+
+        // TODO: This imposes a large cost on unmapping, but that cost cannot be avoided without modifying fmap and funmap
+        UnmapResult { file_desc: self.desc_opt.take() }
     }
 
     /// Extract out a region into a separate grant. The return value is as

src/syscall/process.rs

@@ -8,11 +8,11 @@ use alloc::{
 use core::alloc::{GlobalAlloc, Layout};
 use core::ops::DerefMut;
 use core::{intrinsics, mem, str};
-use spin::RwLock;
+use spin::{RwLock, RwLockWriteGuard};
 
 use crate::context::file::FileDescriptor;
-use crate::context::{ContextId, WaitpidKey};
 use crate::context::memory::{UserGrants, Region};
+use crate::context::{Context, ContextId, WaitpidKey};
 use crate::context;
 #[cfg(not(feature="doc"))]
 use crate::elf::{self, program_header};
@@ -525,7 +525,7 @@ pub fn clone(flags: CloneFlags, stack_base: usize) -> Result<ContextId> {
     Ok(pid)
 }
 
-fn empty(context: &mut context::Context, reaping: bool) {
+fn empty<'lock>(context_lock: &'lock RwLock<Context>, mut context: RwLockWriteGuard<'lock, Context>, reaping: bool) -> RwLockWriteGuard<'lock, Context> {
     if reaping {
         // Memory should already be unmapped
         assert!(context.image.is_empty());
@@ -559,17 +559,26 @@ fn empty(context: &mut context::Context, reaping: bool) {
 
         let grants = mem::replace(&mut *grants_guard, UserGrants::default());
         for grant in grants.inner.into_iter() {
-            if reaping {
+            let unmap_result = if reaping {
                 log::error!("{}: {}: Grant should not exist: {:?}", context.id.into(), *context.name.read(), grant);
 
                 let mut new_table = unsafe { InactivePageTable::from_address(context.arch.get_page_utable()) };
 
-                grant.unmap_inactive(&mut new_table);
+                grant.unmap_inactive(&mut new_table)
             } else {
-                grant.unmap();
+                grant.unmap()
+            };
+
+            if unmap_result.file_desc.is_some() {
+                drop(context);
+
+                drop(unmap_result);
+
+                context = context_lock.write();
             }
         }
     }
+    context
 }
 
 struct ExecFile(FileHandle);
@@ -607,7 +616,7 @@ fn fexec_noreturn(
 
             context.name = Arc::new(RwLock::new(name));
 
-            empty(&mut context, false);
+            context = empty(&context_lock, context, false);
 
             context.grants.write().insert(phdr_grant);
 
@@ -1091,7 +1100,7 @@ pub fn exit(status: usize) -> ! {
         let (vfork, children) = {
             let mut context = context_lock.write();
 
-            empty(&mut context, false);
+            context = empty(&context_lock, context, false);
 
             let vfork = context.vfork;
             context.vfork = false;
@@ -1439,7 +1448,7 @@ fn reap(pid: ContextId) -> Result<ContextId> {
     let context_lock = contexts.remove(pid).ok_or(Error::new(ESRCH))?;
     {
         let mut context = context_lock.write();
-        empty(&mut context, true);
+        context = empty(&context_lock, context, true);
     }
     drop(context_lock);