kern/thread: really fix termination

Change the semantics of thread_join so that it not only waits for thread_exit
but also for the last reference to be dropped.

3 files changed, 23 insertions, 21 deletions

diff --git a/kern/thread.c b/kern/thread.c
index 78c75bad..17586eaf 100644
--- a/kern/thread.c
+++ b/kern/thread.c
@@ -1826,7 +1826,7 @@ thread_init(struct thread *thread, void *stack,
     memset(thread->tsd, 0, sizeof(thread->tsd));
     thread->join_waiter = NULL;
     spinlock_init(&thread->join_lock);
-    thread->exiting = false;
+    thread->terminating = false;
     thread->task = task;
     thread->stack = stack;
     strlcpy(thread->name, attr->name, sizeof(thread->name));
@@ -1953,7 +1953,7 @@ thread_free_stack(void *stack)
 
 #endif /* X15_THREAD_STACK_GUARD */
 
-void
+static void
 thread_destroy(struct thread *thread)
 {
     assert(thread != thread_self());
@@ -1984,7 +1984,7 @@ thread_join_common(struct thread *thread)
     assert(!thread->join_waiter);
     thread->join_waiter = self;
 
-    while (!thread->exiting) {
+    while (!thread->terminating) {
         thread_sleep(&thread->join_lock, thread, "exit");
     }
 
@@ -1996,7 +1996,15 @@ thread_join_common(struct thread *thread)
         thread_unlock_runq(runq, flags);
     } while (state != THREAD_DEAD);
 
-    thread_unref(thread);
+    thread_destroy(thread);
+}
+
+void thread_terminate(struct thread *thread)
+{
+    spinlock_lock(&thread->join_lock);
+    thread->terminating = true;
+    thread_wakeup(thread->join_waiter);
+    spinlock_unlock(&thread->join_lock);
 }
 
 static void
@@ -2391,16 +2399,9 @@ thread_exit(void)
         work_schedule(&zombie.work, 0);
     }
 
-    /*
-     * Disable preemption before waking up since step 2 of the termination
-     * protocol involves actively polling the thread state.
-     */
     thread_preempt_disable();
 
-    spinlock_lock(&thread->join_lock);
-    thread->exiting = true;
-    thread_wakeup(thread->join_waiter);
-    spinlock_unlock(&thread->join_lock);
+    thread_unref(thread);
 
     runq = thread_runq_local();
     spinlock_lock_intr_save(&runq->lock, &flags);
diff --git a/kern/thread.h b/kern/thread.h
index 43a98e87..f565c155 100644
--- a/kern/thread.h
+++ b/kern/thread.h
@@ -291,7 +291,7 @@ thread_unref(struct thread *thread)
     assert(nr_refs != 0);
 
     if (nr_refs == 1) {
-        thread_destroy(thread);
+        thread_terminate(thread);
     }
 }
 
diff --git a/kern/thread_i.h b/kern/thread_i.h
index 7cbd3c72..cd349771 100644
--- a/kern/thread_i.h
+++ b/kern/thread_i.h
@@ -173,16 +173,17 @@ struct thread {
      * Members related to termination.
      *
      * The termination protocol is made of two steps :
-     *  1/ The thread exits, thereby reporting that it is exiting while
-     *     holding the join lock. This includes waking up any thread
-     *     currently joining.
-     *  2/ The thread sets its state to dead and calls the scheduler.
-     *     The join operation polls the state, and releases a reference
-     *     when it sees the dead state.
+     *  1/ The thread exits, thereby releasing its self reference, and
+     *     sets its state to dead before calling the scheduler.
+     *  2/ Another thread must either already be joining, or join later.
+     *     When the thread reference counter drops to zero, the terminating
+     *     flag is set, and the joining thread is awaken, if any. After that,
+     *     the join operation polls the state until it sees the target thread
+     *     as dead, and then releases its resources.
      */
     struct thread *join_waiter;     /* (j) */
     struct spinlock join_lock;
-    bool exiting;                   /* (j) */
+    bool terminating;               /* (j) */
 
     struct task *task;              /* (T) */
     struct list task_node;          /* (T) */
@@ -192,7 +193,7 @@ struct thread {
 
 #define THREAD_ATTR_DETACHED 0x1
 
-void thread_destroy(struct thread *thread);
+void thread_terminate(struct thread *thread);
 
 /*
  * Flag access functions.