feat: proper cleanup for shared pages

src/userprog/process.c

@@ -172,8 +172,9 @@ start_process (void *proc_start_data)
  to store the command that executed the process. */
   if (data->success)
     {
-      data->success =
+      data->success = use_shared_file (exec_file)
-        process_init_stack (data->cmd_saveptr, &if_.esp, data->file_name);
+                      && process_init_stack (data->cmd_saveptr, &if_.esp,
+                                             data->file_name);
     }
 
   /* Signal that the process has finished attempting to load. */
@@ -366,6 +367,7 @@ process_exit (void)
   /* Clean up all open files */
   hash_destroy (&cur->open_files, fd_cleanup);
   hash_destroy (&cur->pages, page_cleanup);
+  unuse_shared_file (cur->exec_file);
 
   /* Close the executable file, implicitly allowing it to be written to. */
   if (cur->exec_file != NULL)

src/vm/page.c

@@ -12,14 +12,19 @@
 static unsigned page_hash (const struct hash_elem *e, void *aux UNUSED);
 static bool page_less (const struct hash_elem *a_, const struct hash_elem *b_,
                        void *aux UNUSED);
+static struct shared_file_entry *shared_file_insert (struct file *file);
 static struct shared_page_entry *shared_page_insert (struct file *file,
                                                      void *upage, void *frame);
 static struct shared_file_entry *shared_file_get (struct file *file);
 static struct shared_page_entry *shared_page_get (struct file *file,
                                                   void *upage);
+static unsigned shared_file_hash (const struct hash_elem *e, void *aux UNUSED);
+static bool shared_file_less (const struct hash_elem *a_,
+                              const struct hash_elem *b_, void *aux UNUSED);
 static unsigned shared_page_hash (const struct hash_elem *e, void *aux UNUSED);
 static bool shared_page_less (const struct hash_elem *a_,
                               const struct hash_elem *b_, void *aux UNUSED);
+static void shared_page_cleanup (struct hash_elem *e, void *aux UNUSED);
 
 /* Initialise a thread's supplemental pages table. */
 bool
@@ -180,7 +185,7 @@ page_cleanup (struct hash_elem *e, void *aux UNUSED)
 }
 
 /* Initialise the shared files and table and lock. */
-bool
+void
 shared_files_init ()
 {
   lock_init (&shared_files_lock);
@@ -188,16 +193,64 @@ shared_files_init ()
     PANIC ("Failed to initialise shared_files table.");
 }
 
+bool
+use_shared_file (struct file *file)
+{
+  lock_acquire (&shared_files_lock);
+  struct shared_file_entry *shared_file = shared_file_get (file);
+  if (shared_file == NULL)
+    {
+      shared_file = shared_file_insert (file);
+      if (shared_file == NULL)
+        {
+          lock_release (&shared_files_lock);
+          return false;
+        }
+    }
+  shared_file->ref_count++;
+  lock_release (&shared_files_lock);
+  return true;
+}
+
+bool
+unuse_shared_file (struct file *file)
+{
+  lock_acquire (&shared_files_lock);
+  struct shared_file_entry *shared_file = shared_file_get (file);
+  if (shared_file == NULL)
+    {
+      lock_release (&shared_files_lock);
+      return false;
+    }
+  shared_file->ref_count--;
+  if (shared_file->ref_count <= 0)
+    {
+      hash_destroy (&shared_file->pages, shared_page_cleanup);
+      hash_delete (&shared_files, &shared_file->elem);
+      free (shared_file);
+    }
+  lock_release (&shared_files_lock);
+}
+
+static void
+shared_page_cleanup (struct hash_elem *e, void *aux UNUSED)
+{
+  struct shared_page_entry *shared_page
+      = hash_entry (e, struct shared_page_entry, elem);
+  // frame_free (shared_page->frame);
+  free (shared_page);
+}
+
 /* Hashing function needed for the shared_file table. Returns a hash for an
    entry based on its file pointer. */
-unsigned
+static unsigned
 shared_file_hash (const struct hash_elem *e, void *aux UNUSED)
 {
   return file_hash (hash_entry (e, struct shared_file_entry, elem)->file);
 }
 
 /* Less function needed for the shared_file table. */
-bool
+static bool
 shared_file_less (const struct hash_elem *a_, const struct hash_elem *b_,
                   void *aux UNUSED)
 {
@@ -246,7 +299,7 @@ shared_file_get (struct file *file)
 
 /* Gets a shared_page_entry from the shared_pages table using the file and upage
    of the page. Returns NULL if no such page_entry exists in the hash map.*/
-struct shared_page_entry *
+static struct shared_page_entry *
 shared_page_get (struct file *file, void *upage)
 {
   /* Search first for the file within the shared_pages structure */
@@ -266,6 +319,27 @@ shared_page_get (struct file *file, void *upage)
   return hash_entry (e, struct shared_page_entry, elem);
 }
 
+static struct shared_file_entry *
+shared_file_insert (struct file *file)
+{
+  struct shared_file_entry *shared_file
+      = malloc (sizeof (struct shared_file_entry));
+  if (shared_file == NULL)
+    return NULL;
+
+  shared_file->file = file;
+  shared_file->ref_count = 0;
+  if (!hash_init (&shared_file->pages, shared_page_hash, shared_page_less,
+                  NULL))
+    {
+      free (shared_file);
+      return NULL;
+    }
+
+  hash_insert (&shared_files, &shared_file->elem);
+  return shared_file;
+}
+
 static struct shared_page_entry *
 shared_page_insert (struct file *file, void *upage, void *frame)
 {
@@ -280,23 +354,12 @@ shared_page_insert (struct file *file, void *upage, void *frame)
   /* If shared file doesn't exist in table, also create it. */
   if (shared_file == NULL)
     {
-      shared_file = malloc (sizeof (struct shared_file_entry));
+      shared_file = shared_file_insert (file);
       if (shared_file == NULL)
         {
           free (shared_page);
           return NULL;
         }
-
-      shared_file->file = file;
-      shared_file->ref_count = 0;
-      if (!hash_init (&shared_file->pages, shared_page_hash, shared_page_less, NULL))
-        {
-          free (shared_page);
-          free (shared_file);
-          return NULL;
-        }
-
-      hash_insert (&shared_files, &shared_file->elem);
     }
 
   shared_page->upage = upage;

src/vm/page.h

@@ -52,6 +52,8 @@ bool page_load (struct page_entry *page);
 void page_cleanup (struct hash_elem *e, void *aux UNUSED);
 
 void shared_files_init ();
+bool use_shared_file (struct file *file);
+bool unuse_shared_file (struct file *file);
 
 void page_set_swap (struct thread *, void *, size_t);
 size_t page_get_swap (struct thread *, void *);