From 9c90e73fda0d5df2e1f11b32d459d3bb07a63192 Mon Sep 17 00:00:00 2001
From: sotech117 <michael_foiani@brown.edu>
Date: Tue, 20 Feb 2024 19:49:59 +0000
Subject: small conceptual issue with retvals when threads are cancelled. will
 ask

---
 kernel/proc/kthread.c  |  4 +---
 kernel/proc/proc.c     |  6 +++++-
 kernel/proc/sched.c    |  3 +--
 kernel/test/proctest.c | 10 +++++++++-
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/kernel/proc/kthread.c b/kernel/proc/kthread.c
index fa447ae..d066dac 100644
--- a/kernel/proc/kthread.c
+++ b/kernel/proc/kthread.c
@@ -162,14 +162,12 @@ void kthread_cancel(kthread_t *thr, void *retval)
     // NOT_YET_IMPLEMENTED("PROCS: kthread_cancel");
     KASSERT(thr != curthr);
 
-    // TODO: ask about the use of check_curthr_cancelled() in syscall_handler()
+    // FIXME: ask about the use of check_curthr_cancelled() in syscall_handler()
     int status = (int) retval;
     dbg(DBG_THR, "Cancelling thread with proc name=%s, id=%d, status=%d\n", 
         thr->kt_proc->p_name, thr->kt_proc->p_pid, status);
     thr->kt_retval = retval;
     sched_cancel(thr);
-
-    //check_curthr_cancelled();
 }
 
 /*
diff --git a/kernel/proc/proc.c b/kernel/proc/proc.c
index 7350acb..a1f68f8 100644
--- a/kernel/proc/proc.c
+++ b/kernel/proc/proc.c
@@ -268,7 +268,11 @@ void proc_cleanup(long status)
 
     // update state and status
     curproc->p_state = PROC_DEAD;
-    curproc->p_status = status;
+    if (curthr->kt_cancelled) {
+        curproc->p_status = curthr->kt_retval;
+    } else {
+        curproc->p_status = status;
+    }
 }
 
 /*
diff --git a/kernel/proc/sched.c b/kernel/proc/sched.c
index cd8e438..31976bb 100644
--- a/kernel/proc/sched.c
+++ b/kernel/proc/sched.c
@@ -74,7 +74,6 @@ void sched_queue_init(ktqueue_t *queue)
  */
 static void ktqueue_enqueue(ktqueue_t *queue, kthread_t *thr)
 {
-    // TODO: ask in mentor meeting about what causes this assertion error
     KASSERT(!thr->kt_wchan);
 
     list_assert_sanity(&queue->tq_list);
@@ -238,7 +237,7 @@ void sched_switch(ktqueue_t *queue)
 
     curcore.kc_queue = queue;
     last_thread_context = &curthr->kt_ctx;
-
+    
     context_switch(&curthr->kt_ctx, &curcore.kc_ctx);
 
     intr_enable();
diff --git a/kernel/test/proctest.c b/kernel/test/proctest.c
index f49da72..ab69512 100644
--- a/kernel/test/proctest.c
+++ b/kernel/test/proctest.c
@@ -277,10 +277,17 @@ void test_out_of_order_termination()
 /*
     Test threads' cancellation fields.
 */
+void *did_run_func(long arg1, void *arg2)
+{
+    // arg2 is a boolean flag that indicated this ran
+    *(int *)arg2 = 1;
+    return NULL;
+}
 void test_cancellation() 
 {
     proc_t *new_proc1 = proc_create("proc test 1");
-    kthread_t *new_kthread1 = kthread_create(new_proc1, test_func, new_proc1->p_pid, new_proc1);
+    int did_run = 0;
+    kthread_t *new_kthread1 = kthread_create(new_proc1, did_run_func, new_proc1->p_pid, (void *)&did_run);
     test_assert(new_kthread1->kt_cancelled == 0, "Thread should not be cancelled");
     sched_make_runnable(new_kthread1);
 
@@ -293,6 +300,7 @@ void test_cancellation()
     int ret = do_waitpid(new_proc1->p_pid, &status, 0);
     test_assert(ret != -ECHILD, "Should have found the process");
     test_assert(status == 1, "Returned status not set correctly");
+    test_assert(did_run == 0, "Thread should not have run if it was cancelled");
 }
 
 
-- 
cgit v1.2.3-70-g09d2