Jleong/change exit behavior

src/lib/kernel/list.c

@@ -251,7 +251,19 @@ list_remove (struct list_elem *elem)
   ASSERT (is_interior (elem));
   elem->prev->next = elem->next;
   elem->next->prev = elem->prev;
-  return elem->next;
+  struct list_elem *ret = elem->next;
+  elem->prev = NULL;
+  elem->next = NULL;
+  return ret;
+}
+
+struct list_elem *
+try_remove (struct list_elem *elem)
+{
+  if (is_interior(elem)) {
+      return list_remove(elem);
+  }
+  return NULL;
 }
 
 /* Removes the front element from LIST and returns it.

src/lib/kernel/list.h

@@ -145,6 +145,7 @@ void list_push_back (struct list *, struct list_elem *);
 
 /* List removal. */
 struct list_elem *list_remove (struct list_elem *);
+struct list_elem *try_remove (struct list_elem *);
 struct list_elem *list_pop_front (struct list *);
 struct list_elem *list_pop_back (struct list *);
 

src/threads/thread.c

@@ -112,6 +112,20 @@ struct thread *get_initial_thread(void) {
     return initial_thread;
 }
 
+/*! Returns the child thread with this tid. */
+struct thread *get_child_thread(tid_t child_tid) {
+    struct thread *cur = thread_current();
+    struct list_elem *e;
+    for (e = list_begin(&cur->kids); e != list_end(&cur->kids);
+         e = list_next(e)) {
+        struct thread *kid = list_entry(e, struct thread, kid_elem);
+        if (kid->tid == child_tid) {
+            return kid;
+        }
+    }
+    return NULL;
+}
+
 /*! Initializes the threading system by transforming the code
     that's currently running into a thread.  This can't work in
     general and it is possible in this case only because loader.S
@@ -269,6 +283,8 @@ tid_t thread_create(const char *name, int priority, thread_func *function,
     struct thread *parent = thread_current();
     list_push_back(&parent->kids, &t->kid_elem);
     t->parent = parent;
+    ASSERT(t->done_sema.value == 1);
+    sema_down(&t->done_sema);
 
     /* Add to run queue. */
     int prev_highest_priority = get_highest_priority();
@@ -348,44 +364,41 @@ void thread_exit(void) {
        and schedule another process.  That process will destroy us
        when it calls thread_schedule_tail(). */
     struct thread *cur = thread_current();
+    struct list_elem *e;
 
     /* Tell blocking lock we are no longer waiting for it. */
     if (cur->blocking_lock != NULL) {
         list_remove(&cur->lock_elem);
     }
 
     /* Free all locks. */
-    struct list_elem *e;
-    for (e = list_begin(&cur->locks_acquired);
-         e != list_end(&cur->locks_acquired);
-         /* increment in loop */) {
+    while (!list_empty(&cur->locks_acquired)) {
+        e = list_begin(&cur->locks_acquired);
         struct lock *lock = list_entry(e, struct lock, elem);
-        e = list_next(e);
         lock_release(lock);
     }
 
     /* All file buffers should be freed in sys_exit. */
     ASSERT (list_empty(&cur->open_files));
 
     /* Let kids know that parent is dead so that their page is freed without
-       waiting for the parent to free them. Will be freed in
-       thread_schedule_tail() instead of process_wait().*/
-    for (e = list_begin(&cur->kids); e != list_end(&cur->kids);
-         /* increment in loop */) {
+       waiting for the parent. Will be freed in thread_schedule_tail().*/
+
+    while (!list_empty(&cur->kids)) {
+        e = list_pop_front(&cur->kids);
         struct thread *kid = list_entry(e, struct thread, kid_elem);
-        e = list_next(e);
         kid->parent = NULL;
-
-        if (kid != NULL && kid->status == THREAD_DYING
-            && kid != initial_thread) {
-            palloc_free_page(kid);
-        }
+        sema_up(&kid->done_sema);
     }
 
 #ifdef USERPROG
     process_exit();
 #endif
 
+    if (cur->parent != NULL) {
+        list_remove(&cur->kid_elem);
+    }
+
     intr_disable();
     list_remove(&cur->allelem);
     cur->status = THREAD_DYING;
@@ -593,17 +606,22 @@ int next_fd(struct thread *cur) {
 int add_open_file(struct thread *cur, struct file *file, int fd) {
     /* Initialize file */
     struct sys_file *new_file = palloc_get_page(PAL_ZERO);
+    /* Not enough memory. */
+    if (new_file == NULL) {
+        file_close(file);
+        return ERR;
+    }
     memset(new_file, 0, sizeof *new_file);
     new_file->file = file;
     new_file->fd = fd;
 
-    if (is_valid_fd(fd)) {
+    if (is_valid_fd(fd) && !is_existing_fd(cur, fd)) {
         list_push_back(&cur->open_files, &new_file->file_elem);
         return fd;
     }
 
     /* Couldn't find file. */
-    return -1;
+    return ERR;
 }
 
 /*! Get file with file descriptor *fd*. */
@@ -634,6 +652,7 @@ void close_fd(struct thread *cur, int fd) {
         struct sys_file *cur_file = list_entry(e, struct sys_file, file_elem);
         if (cur_file->fd == fd) {
             list_remove(&cur_file->file_elem);
+            file_close(cur_file->file);
             palloc_free_page(cur_file);
             return;
         }
@@ -719,11 +738,12 @@ static void init_thread(struct thread *t, const char *name, int priority) {
     list_init(&t->open_files);
     /* Block process_wait of parent until this process is ready to die. */
     sema_init(&t->wait_sema, 0);
-    sema_init(&t->exec_load, 0);
-    lock_init(&t->filesys_lock);
+    sema_init(&t->done_sema, 1);
+    lock_init(&t->wait_lock);
     t->loaded = false;
     t->waited_on = false;
     t->num_files = 0;
+    t->parent = NULL;
 
     if (list_empty(&all_list)) {
         t->niceness = 0;  /* Set niceness to 0 on initial thread */
@@ -827,12 +847,15 @@ void thread_schedule_tail(struct thread *prev) {
     if (prev != NULL && prev->status == THREAD_DYING &&
         prev != initial_thread) {
         ASSERT(prev != cur);
-        /* Let parent know it is done. */
-        sema_up(&prev->wait_sema);
-        if (prev->parent == NULL) {
-            /* Don't need to wait for parent to kill kid */
-            palloc_free_page(prev);
-        }
+        ASSERT(try_remove(&prev->allelem) == NULL);
+        ASSERT(try_remove(&prev->sleep_elem) == NULL);
+        ASSERT(try_remove(&prev->elem) == NULL);
+        ASSERT(try_remove(&prev->lock_elem) == NULL);
+        ASSERT(try_remove(&prev->kid_elem) == NULL);
+        ASSERT(list_empty(&prev->locks_acquired));
+        ASSERT(list_empty(&prev->open_files));
+        ASSERT(list_empty(&prev->kids));
+        palloc_free_page(prev);
     }
 }
 

src/threads/thread.h

@@ -139,7 +139,9 @@ struct thread {
     struct list kids;                   /*!< List of children processes. */
     struct list_elem kid_elem;          /*!< List element for parent's kids list. */
     struct semaphore wait_sema;         /*!< Sempahore for process_wait. */
+    struct semaphore done_sema;         /*!< Sempahore for process_exit. */
     bool waited_on;                     /*!< True if process_wait has been called. */
+    struct lock wait_lock;              /*!< Lock for checking waited_on. */
     /**@}*/
 
     /*! Shared between by userprog/process.c and userprog.syscall.c and
@@ -149,7 +151,6 @@ struct thread {
     struct semaphore exec_load;         /*!< Semaphore for checking when executable has loaded. */
     bool loaded;                        /*!< Check if exec loaded successfully. */
     struct file *executable;            /*!< Executable to keep open until done. */
-    struct lock filesys_lock;           /*!< Lock for filesys calls. */
     /**@}*/
 
 #ifdef USERPROG
@@ -220,6 +221,7 @@ void add_sleep_thread(struct thread *);
 void sleep_threads(void);
 
 struct thread *get_initial_thread(void);
+struct thread *get_child_thread(tid_t child_tid);
 
 #endif /* threads/thread.h */
 

src/userprog/DESIGNDOC

@@ -12,20 +12,23 @@ Justin Leong <jleong@caltech.edu>
 John Li <jzli@caltech.edu>
 Christina Lin <cylin@caltech.edu>
 
->> Specify how many late tokens you are using on this assignment: 2
+>> Specify how many late tokens you are using on this assignment: 4
 
 >> What is the Git repository and commit hash for your submission?
    (You only need to include the commit-hash in the file you submit
    on Moodle.)
 
    Repository URL: https://github.com/omegaphoenix/cheetOS
-   commit: aa0dfcc3c89b57c3faf9934ee469aa02a013e5ff
+   commit: 
+   tag: project4-3
 
 ---- PRELIMINARIES ----
 
 >> If you have any preliminary comments on your submission, notes for the
 >> TAs, or extra credit, please give them here.
 
+All tests pass on qemu.
+
 >> Please cite any offline or online sources you consulted while
 >> preparing your submission, other than the Pintos documentation, course
 >> text, lecture notes, and course instructors.
@@ -57,8 +60,7 @@ which parts.
 >> L1: How many hours did each team member spend on this assignment?
    Make sure that each member's total time is listed.
 
-  Justin Leong: Approximately 31:22:50 (hh/mm/ss) (45 hours total including
-  time after deadline)
+  Justin Leong: At least 51:22:55 (hh/mm/ss)
   John Li: Approximately 8 hours. Was busy planning and executing several events this week.
   Christina Lin: Approximately 26-27 hours.
 
@@ -133,15 +135,16 @@ can have two levels of validation.
 We added this variable to process.c to synchronize filesystem calls.
     struct lock filesys_lock;          /*! Lock that belongs to process. */
 
-In our thread struct, we included these attributes and their following descriptions:
-
+We added this struct to thread.h for the list of open files.
     /* Open file. This is for a linked list of open files in each thread. */
     struct sys_file {
         struct file *file;
         int fd;
         struct list_elem file_elem;
     };
 
+In our thread struct, we included these attributes and their following descriptions:
+
     /*! Shared between thread.c and userprog/syscall.c. */
     /**@{*/
     int num_files;                      /*!< Number of open files (counting closed ones). */
@@ -154,7 +157,9 @@ In our thread struct, we included these attributes and their following descripti
     struct list kids;                   /*!< List of children processes. */
     struct list_elem kid_elem;          /*!< List element for parent's kids list. */
     struct semaphore wait_sema;         /*!< Sempahore for process_wait. */
+    struct semaphore done_sema;         /*!< Sempahore for process_exit. */
     bool waited_on;                     /*!< True if process_wait has been called. */
+    struct lock wait_lock;              /*!< Lock for checking waited_on. */
     /**@}*/
 
     /*! Shared between by userprog/process.c and userprog.syscall.c and
@@ -164,26 +169,25 @@ In our thread struct, we included these attributes and their following descripti
     struct semaphore exec_load;         /*!< Semaphore for checking when executable has loaded. */
     bool loaded;                        /*!< Check if exec loaded successfully. */
     struct file *executable;            /*!< Executable to keep open until done. */
-    struct lock filesys_lock;           /*!< Lock for filesys calls. */
     /**@}*/
 
 
 >> B2: Describe how file descriptors are associated with open files.
 >> Are file descriptors unique within the entire OS or just within a
 >> single process?
 
-File descriptors are only unique within a single process. The thread contains
-an array of pointers to open files and finds an empty (null) index to place
-an open file in. When an open file is closed, then it's spot in the array
-is set to NULL.
+File descriptors are only unique within a single thread. The thread contains
+a list of open files using sys_file. Every time a new file is opened num_files
+is incremented and then the new file gets a fd of num_files + 2 before it is
+incremented since 0 and 1 are reserved for stdin and stdout file descriptors.
 
 ---- ALGORITHMS ----
 
 >> B3: Describe your code for reading and writing user data from the
 >> kernel.
 
 We use valid_read_addr to check the pointer to the buffer and the buffer. If
-we are reading/writing from/to a file, we get the file form the thread and
+we are reading/writing from/to a file, we get the file from the thread and
 make the filesys call. If we are reading/writing from/to the terminal, we
 read one byte at a time and we output 300 bytes at a time.
 
@@ -230,7 +234,9 @@ functions.
 
 When an error is detected, we free allocated resources by calling sys_exit()
 which calls thread_exit() which iterates through all of its open files and
-locks and frees them. The executable is also freed here.
+locks and frees them. thread_exit() then calls process_exit() and the
+executable is freed here. If we know no files have been allocated we can call
+thread_exit() directly.
 
 Example: If an argument is not a valid read address, we call sys_exit(-1)
 which sets the exit status to -1 and then calls thread_exit(). thread_exit()
@@ -244,7 +250,7 @@ that it is waiting on that it is no longer waiting.
 >> loading.  How does your code ensure this?  How is the load
 >> success/failure status passed back to the thread that calls "exec"?
 
-We add a semaphore which is blocked initially and so sys_exec() waits
+We add a wait_sema semaphore which is blocked initially and so sys_exec() waits
 on this sempahore before returning. The semaphore is up'ed when the
 executable has completed loading and the thread has a boolean "loaded"
 which is true if successful. Once sys_exec() resumes, it can check
@@ -259,20 +265,27 @@ possibly free the thread.
 >> terminates without waiting, before C exits?  After C exits?  Are
 >> there any special cases?
 
-When P calls wait(C) before C exits, the kid's status is not set to
-THREAD_DYING yet and so P waits on a semaphore that is initialized to 0
-and which is incremented when the kid is ready to die. Then it gets the
-exit status and frees the kid.
+When P calls wait(C) before C exits, C is not finished yet so it has not
+up'ed C->wait_sema. So P is blocked on C->wait_sema. When C calls
+process_exit() when it is done, it up's C->wait_sema and tries to down
+C->done_sema which is set to 0 when C is created. Then P retrieves
+C->exit_status and up's C->done_sema which tells C that it can now free
+it's memory.
 
-After C exits, if it hasn't been freed by a previous wait call,
-P just gets the exit status and frees the kid. If it has been freed, P
-returns -1;
+After C exits, if it hasn't been freed by a previous wait call, it waits
+on C->done_sema which is 0 and thus blocking. When P makes the wait call,
+P just gets the exit status and frees the kid by up'ing C->done_sema.
+If wait has already been called on C, P returns -1;
 
 When P terminates without waiting, it tells each one of it's kids that they
-don't have a parent anymore by setting their struct thread *parent to NULL.
-If this is before C exits, then when C exits, thread_schedule_tail() will
-free the kid. If this is after C exits, then the kid is freed right away
-before the parent dies.
+don't have a parent anymore by setting their struct thread *parent to NULL
+and up'ing their C->done_sema. Then when C calls process_exit(), it will down
+the C->done_sema semaphore and since it was 1, it will be able to continue
+and free it's page.
+
+The special case is the initial thread where we need to initialize the
+done_sema to 1 so that when it exits, it is not waiting on a parent to
+up it's done_sema.
 
 ---- RATIONALE ----
 
@@ -289,6 +302,7 @@ distinguishes between read and write for clarity.
 >> B10: What advantages or disadvantages can you see to your design
 >> for file descriptors?
 
+Disadvantages of current design:
 The first design we used is to use an array of pointers to files. The advantage
 of this approach is that it is quick and easy to access open_files from the array
 since fd maps directly to an index (by subtracting 2 for stdin and stout).
@@ -298,6 +312,7 @@ processes. Using a linked list would reduce the space requirement on the
 thread struct but would increase the access time and complexity of getting the
 file associated with a file descriptor.
 
+Advantages of current design:
 We reimplemented to use a linked list instead which solved some of our memory
 management issues and allowed us to handle as many files as necessary. We
 created a struct to make this implementation easier which kept track of the