From 9b19a84ed640cb15e6ca87ddc025c90d78a5fbdc Mon Sep 17 00:00:00 2001
From: manuel <manuel@mausz.at>
Date: Fri, 11 May 2012 20:48:58 +0200
Subject: * fix possible race in thread_set_priority * fill in same parts of
 proj1.txt * use thread_get_priority() whenever possible

---
 threads/synch.c  | 16 ++++++++--------
 threads/thread.c | 21 ++++++++++++++++++++-
 2 files changed, 28 insertions(+), 9 deletions(-)

(limited to 'threads')

diff --git a/threads/synch.c b/threads/synch.c
index 98905b1..a1b7549 100644
--- a/threads/synch.c
+++ b/threads/synch.c
@@ -128,7 +128,7 @@ sema_up (struct semaphore *sema)
       list_sort (&sema->waiters, &thread_priority_cmp_greater, NULL);
       struct thread *t = list_entry (list_pop_front (&sema->waiters), struct thread, elem);
       /* thread_unblock() doesn't yield so check if there's a need */
-      if (t->priority > thread_current ()->priority)
+      if (t->priority > thread_get_priority ())
         yield = true;
       thread_unblock (t);
     }
@@ -240,10 +240,10 @@ lock_acquire (struct lock *lock)
   while (blocked_lock != NULL && blocked_lock->holder != NULL)
     {
       /* check for a possible priority donation */
-      if (cur->priority > blocked_lock->priority)
+      if (thread_get_priority () > blocked_lock->priority)
         {
           /* the new priority of this lock is our own priority */
-          blocked_lock->priority = cur->priority;
+          blocked_lock->priority = thread_get_priority ();
 
           /* the lock priority changed so we need to sort the lock list of the lock holder */
           list_remove (&blocked_lock->elem);
@@ -264,7 +264,7 @@ lock_acquire (struct lock *lock)
   cur->blocked_lock = NULL;
 
   /* the priority of this lock is our own priority */
-  lock->priority = cur->priority;
+  lock->priority = thread_get_priority ();
 
   /* save the locks we hold in descending order of their priority */
   list_insert_ordered (&lock->holder->locks, &lock->elem,
@@ -305,7 +305,7 @@ lock_release (struct lock *lock)
   ASSERT (lock_held_by_current_thread (lock));
 
   lock->holder = NULL;
-  //lock->priority = we don't care as the next lock_acquire()-call does
+  //lock->priority = we don't care since the next lock_acquire()-call does
   sema_up (&lock->semaphore);
 
   /* we don't hold the lock any longer so remove it from the lock list */
@@ -424,9 +424,9 @@ cond_signal (struct condition *cond, struct lock *lock UNUSED)
      this is the same as with sema_down()/sema_up() */
   if (!list_empty (&cond->waiters))
   {
-      list_sort (&cond->waiters, &semaphore_priority_cmp_greater, NULL);
-      sema_up (&list_entry (list_pop_front (&cond->waiters),
-            struct semaphore_elem, elem)->semaphore);
+    list_sort (&cond->waiters, &semaphore_priority_cmp_greater, NULL);
+    sema_up (&list_entry (list_pop_front (&cond->waiters),
+          struct semaphore_elem, elem)->semaphore);
   }
 }
 
diff --git a/threads/thread.c b/threads/thread.c
index 61ab5d9..cf404b6 100644
--- a/threads/thread.c
+++ b/threads/thread.c
@@ -379,12 +379,26 @@ thread_set_priority (int new_priority)
 void
 thread_other_set_priority (struct thread *t, int new_priority)
 {
+  enum intr_level old_level;
+  bool yield = false;
+
   ASSERT (is_thread (t));
   ASSERT (new_priority >= PRI_MIN && new_priority <= PRI_MAX);
 
   if (t->priority == new_priority)
     return;
 
+  /* we need to disable interrupts here because after changing the thread's
+     priority and a possible timer interrupt which triggers thread_yield() we
+     could run into a couple of races. e.g. no proper sorted ready list,
+     corrupt ready list (wrong internal pointers) or the timer interrupt fires
+     after calling list_empty() but before list_front() while having only two
+     threads (one active, the other one ready). the scheduler then schedules the
+     other thread which runs and terminates, thus removing itself from all lists.
+     after that our thread gets scheduled again and reads from an empty list,
+     although it wasn't empty before. */
+  old_level = intr_disable ();
+
   t->priority = new_priority;
 
   if (t->status == THREAD_READY)
@@ -398,8 +412,13 @@ thread_other_set_priority (struct thread *t, int new_priority)
       /* compare priority with the highest priority in the list */
       struct thread *t2 = list_entry (list_front (&ready_list), struct thread, elem);
       if (t2->priority > t->priority)
-        thread_yield ();
+        yield = true;
     }
+
+  intr_set_level (old_level);
+
+  if (yield)
+    thread_yield ();
 }
 
 void
-- 
cgit v1.2.3