Cleaning up a pile of bugs

sys/amd64/critical.c

@@ -41,6 +41,12 @@ Critical_Exit()
     }
 }
 
+uint32_t
+Critical_Level()
+{
+    return lockLevel[CPU()];
+}
+
 static void
 Debug_Critical(int argc, const char *argv[])
 {

sys/include/spinlock.h

@@ -30,6 +30,7 @@ typedef struct Spinlock
 void Critical_Init();
 void Critical_Enter();
 void Critical_Exit();
+uint32_t Critical_Level();
 
 void Spinlock_EarlyInit();
 void Spinlock_Init(Spinlock *lock, const char *name, uint64_t type);

sys/kern/mutex.c

@@ -47,6 +47,12 @@ Mutex_Destroy(Mutex *mtx)
 void
 Mutex_Lock(Mutex *mtx)
 {
+    /*
+     * You cannot hold a spinlock while trying to acquire a Mutex that may 
+     * sleep!
+     */
+    ASSERT(Critical_Level() == 0);
+
     Spinlock_Lock(&mtx->lock);
     while (mtx->status == MTX_STATUS_LOCKED) {
 	WaitChannel_Lock(&mtx->chan);

sys/kern/process.c

@@ -189,9 +189,18 @@ Process_Wait(Process *proc, uint64_t pid)
     status = (p->pid << 16) | p->exitCode;
 
     // Release threads
-    TAILQ_FOREACH_SAFE(thr, &p->zombieQueue, schedQueue, thr_temp) {
+    Spinlock_Lock(&proc->lock);
+    while (!TAILQ_EMPTY(&p->zombieQueue)) {
+	thr = TAILQ_FIRST(&p->zombieQueue);
+	TAILQ_REMOVE(&p->zombieQueue, thr, schedQueue);
+	Spinlock_Unlock(&proc->lock);
+
+	ASSERT(thr->proc->pid != 1);
 	Thread_Release(thr);
+
+	Spinlock_Lock(&proc->lock);
     }
+    Spinlock_Unlock(&proc->lock);
 
     // Release process
     Process_Release(p);

sys/kern/sched.c

@@ -65,6 +65,8 @@ Sched_SetWaiting(Thread *thr)
 {
     Spinlock_Lock(&schedLock);
 
+    ASSERT(thr->schedState == SCHED_STATE_RUNNING);
+
     thr->schedState = SCHED_STATE_WAITING;
     TAILQ_INSERT_TAIL(&waitQueue, thr, schedQueue);
     thr->waitStart = KTime_GetEpochNS();
@@ -77,13 +79,6 @@ Sched_SetZombie(Thread *thr)
 {
     Process *proc = thr->proc;
 
-    Spinlock_Lock(&schedLock);
-
-    thr->schedState = SCHED_STATE_ZOMBIE;
-    Spinlock_Lock(&proc->lock);
-    TAILQ_INSERT_TAIL(&proc->zombieQueue, thr, schedQueue);
-    Spinlock_Unlock(&proc->lock);
-
     if (proc->threads == 1) {
 	// All processes have parents except 'init' and 'kernel'
 	ASSERT(proc->parent != NULL);
@@ -95,10 +90,22 @@ Sched_SetZombie(Thread *thr)
 	Spinlock_Unlock(&proc->parent->lock);
 	CV_Signal(&proc->zombieProcPCV);
 	CV_Signal(&proc->parent->zombieProcCV);
-	Mutex_Unlock(&proc->parent->zombieProcLock);
     }
 
+    /*
+     * Set as zombie just before releasing the zombieProcLock in case we had to 
+     * sleep to acquire the zombieProcLock.
+     */
+    Spinlock_Lock(&schedLock);
+    thr->schedState = SCHED_STATE_ZOMBIE;
     Spinlock_Unlock(&schedLock);
+
+    Spinlock_Lock(&proc->lock);
+    TAILQ_INSERT_TAIL(&proc->zombieQueue, thr, schedQueue);
+    Spinlock_Unlock(&proc->lock);
+
+    if (proc->threads == 1)
+	Mutex_Unlock(&proc->parent->zombieProcLock);
 }
 
 static void
@@ -132,6 +139,7 @@ Sched_Scheduler()
 	Spinlock_Unlock(&schedLock);
 	return;
     }
+    ASSERT(next->schedState == SCHED_STATE_RUNNABLE);
     TAILQ_REMOVE(&runnableQueue, next, schedQueue);
 
     prev = curProc[CPU()];

sys/kern/thread.c

@@ -192,6 +192,9 @@ Thread_Destroy(Thread *thr)
 {
     Process *proc = thr->proc;
 
+    // Don't free kernel threads
+    ASSERT(proc->pid != 1);
+
     // Free userspace stack
 
     Spinlock_Lock(&proc->lock);
@@ -201,10 +204,11 @@ Thread_Destroy(Thread *thr)
 
     // Free AS
     PAlloc_Release((void *)thr->kstack);
-    Slab_Free(&threadSlab, thr);
 
     // Release process handle
     Process_Release(thr->proc);
+
+    Slab_Free(&threadSlab, thr);
 }
 
 Thread *
@@ -262,6 +266,7 @@ Thread_Wait(Thread *thr, uint64_t tid)
 	return SYSCALL_PACK(0, status);
     }
 
+    // XXXURGENT
     TAILQ_FOREACH(t, &thr->proc->zombieQueue, schedQueue) {
 	if (t->tid == tid) {
 	    TAILQ_REMOVE(&thr->proc->zombieQueue, t, schedQueue);
@@ -293,12 +298,20 @@ ThreadKThreadEntry(TrapFrame *tf) __NO_LOCK_ANALYSIS
 void
 Thread_Dump(Thread *thr)
 {
+    const char *states[] = {
+	"NULL",
+	"RUNNABLE",
+	"RUNNING",
+	"WAITING",
+	"ZOMBIE"
+    };
+
     // Thread_DumpArch(thr)
     kprintf("space      %016llx\n", thr->space);
     kprintf("kstack     %016llx\n", thr->kstack);
     kprintf("tid        %llu\n", thr->tid);
     kprintf("refCount   %d\n", thr->refCount);
-    kprintf("state      %d\n", thr->schedState);
+    kprintf("state      %s\n", states[thr->schedState]);
     kprintf("ctxswtch   %llu\n", thr->ctxSwitches);
     kprintf("utime      %llu\n", thr->userTime);
     kprintf("ktime      %llu\n", thr->kernTime);

sys/kern/waitchannel.c

@@ -118,7 +118,6 @@ WaitChannel_WakeAll(WaitChannel *wchan)
     Spinlock_Lock(&wchan->lock);
 
     TAILQ_FOREACH_SAFE(thr, &wchan->chanQueue, chanQueue, thrTemp) {
-	thr = TAILQ_FIRST(&wchan->chanQueue);
 	TAILQ_REMOVE(&wchan->chanQueue, thr, chanQueue);
 	Sched_SetRunnable(thr);
 	Thread_Release(thr);