From 031496ff0d6da03f2cdb0fe3b473b765e2402b29 Mon Sep 17 00:00:00 2001
From: 4lDO2 <4lDO2@protonmail.com>
Date: Sat, 27 Feb 2021 15:28:12 +0100
Subject: [PATCH] Fix "Grant should not exist" errors.

This is done by making sure that when empty() is called on a context,
the grants Arc will be replaced with a new unused Arc, hence
decrementing the refcount. Previously this was only done when the
context was actually reaped, but since there is no guarantee as far as I
am aware about when this must happen, the grants could be completely
leaked, leading to the error.
---
 src/syscall/process.rs | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/src/syscall/process.rs b/src/syscall/process.rs
index 08d0407..0adbabd 100644
--- a/src/syscall/process.rs
+++ b/src/syscall/process.rs
@@ -558,12 +558,29 @@ fn empty(context: &mut context::Context, reaping: bool) {
         drop(context.tls.take());
     }
 
-    let mut grants = context.grants.write();
-    if Arc::strong_count(&context.grants) == 1 {
-        let grants = mem::replace(&mut *grants, UserGrants::default());
+    // NOTE: If we do not replace the grants `Arc`, then a strange situation can appear where the
+    // main thread and another thread exit simultaneously before either one is reaped. If that
+    // happens, then the last context that runs exit will think that there is still are still
+    // remaining references to the grants, where there are in fact none. However, if either one is
+    // reaped before, then that reference will disappear, and no leak will occur.
+    //
+    // By removing the reference to the grants when the context will no longer be used, this
+    // problem will never occur.
+
+    // FIXME, UNOPTIMIZED: Right now, this will allocate memory in order to store the new empty
+    // grants, which may not even be used (only in fexec I think). We should turn grants into an
+    // `Option`, and only reinitialize it there.
+    let mut grants_arc = mem::take(&mut context.grants);
+
+    if let Some(grants_lock_mut) = Arc::get_mut(&mut grants_arc) {
+        // TODO: Use get_mut to bypass the need to acquire a lock when there we already have an
+        // exclusive reference from `Arc::get_mut`. This will require updating `spin`.
+        let mut grants_guard = grants_lock_mut.write();
+
+        let grants = mem::replace(&mut *grants_guard, UserGrants::default());
         for grant in grants.inner.into_iter() {
             if reaping {
-                println!("{}: {}: Grant should not exist: {:?}", context.id.into(), *context.name.read(), grant);
+                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_table()) };
                 let mut temporary_page = TemporaryPage::new(Page::containing_address(VirtualAddress::new(crate::USER_TMP_GRANT_OFFSET)));