From: John Hodge Date: Tue, 17 Feb 2015 03:32:03 +0000 (+0800) Subject: Kernel/threads - Debug cleanups and (hopefully) race avoidance X-Git-Url: https://git.ucc.asn.au/?a=commitdiff_plain;h=bdab8e5cebaf249d291d19523d0358f8c1c98008;p=tpg%2Facess2.git Kernel/threads - Debug cleanups and (hopefully) race avoidance --- diff --git a/KernelLand/Kernel/include/threads_int.h b/KernelLand/Kernel/include/threads_int.h index 403a2eff..1c9a08d7 100644 --- a/KernelLand/Kernel/include/threads_int.h +++ b/KernelLand/Kernel/include/threads_int.h @@ -143,6 +143,7 @@ static const char * const casTHREAD_STAT[] = { "THREAD_STAT_DEAD", "THREAD_STAT_BURIED" }; +static const unsigned int ciTHREAD_STAT_COUNT = sizeof(casTHREAD_STAT)/sizeof(casTHREAD_STAT[0]); // === GLOBALS === extern BOOL gaThreads_NoTaskSwitch[MAX_CPUS]; diff --git a/KernelLand/Kernel/threads.c b/KernelLand/Kernel/threads.c index 469c5ec1..9aefeabf 100644 --- a/KernelLand/Kernel/threads.c +++ b/KernelLand/Kernel/threads.c @@ -493,7 +493,13 @@ tTID Threads_WaitTID(int TID, int *Status) ret = dead_thread->TID; // - Mark as dead (as opposed to undead) - ASSERTC(dead_thread->Status, ==, THREAD_STAT_ZOMBIE); + if( dead_thread->Status != THREAD_STAT_ZOMBIE ) { + Log_Error("Thread", "Thread %p(%i %s) is not ZOMBIE, instead %s", + dead_thread, dead_thread->TID, dead_thread->ThreadName, + (dead_thread->Status < ciTHREAD_STAT_COUNT ? casTHREAD_STAT[dead_thread->Status] : "?") + ); + ASSERTC(dead_thread->Status, ==, THREAD_STAT_ZOMBIE); + } dead_thread->Status = THREAD_STAT_DEAD; // - Set return status if(Status) @@ -503,7 +509,7 @@ tTID Threads_WaitTID(int TID, int *Status) } else { - Log_Error("Threads", "TODO: Threads_WaitTID(TID=-1) - Any Child"); + Log_Error("Threads", "TODO: Threads_WaitTID(TID=-1) - Woken with no child"); } return ret; } @@ -627,17 +633,13 @@ void Threads_Exit(int TID, int Status) */ void Threads_Kill(tThread *Thread, int Status) { - tMsg *msg; int isCurThread = Thread == Proc_GetCurThread(); // TODO: Disown all children? #if 1 { - tThread *child; // TODO: I should keep a .Children list - for(child = gAllThreads; - child; - child = child->GlobalNext) + for(tThread* child = gAllThreads; child; child = child->GlobalNext) { if(child->Parent == Thread) child->Parent = &gThreadZero; @@ -653,7 +655,7 @@ void Threads_Kill(tThread *Thread, int Status) // Clear Message Queue while( Thread->Messages ) { - msg = Thread->Messages->Next; + tMsg *msg = Thread->Messages->Next; free( Thread->Messages ); Thread->Messages = msg; } @@ -701,8 +703,8 @@ void Threads_Kill(tThread *Thread, int Status) if( !Threads_int_DelFromQueue( &gSleepingThreads, Thread ) ) { Log_Warning("Threads", - "Threads_Kill - Thread %p(%i,%s) marked as sleeping, but not on list", - Thread, Thread->TID, Thread->ThreadName + "Threads_Kill - Thread "PRIthread_fmt" marked as sleeping, but not on list", + PRIthread_args(Thread) ); } break; @@ -724,8 +726,6 @@ void Threads_Kill(tThread *Thread, int Status) // Save exit status Thread->RetStatus = Status; - SHORTREL( &Thread->IsLocked ); - Thread->Status = THREAD_STAT_ZOMBIE; SHORTREL( &glThreadListLock ); // TODO: It's possible that we could be timer-preempted here, should disable that... somehow @@ -735,7 +735,10 @@ void Threads_Kill(tThread *Thread, int Status) Threads_PostEvent( Thread->Parent, THREAD_EVENT_DEADCHILD ); // Process cleanup happens on reaping - Log("Thread %i went *hurk* (%i)", Thread->TID, Status); + Log("Thread "PRIthread_fmt" went *hurk* (%i) (isCurThread=%B)", PRIthread_args(Thread), Status, isCurThread); + //Log("Thread status = %i %s", Thread->Status, casTHREAD_STAT[Thread->Status]); + + SHORTREL( &Thread->IsLocked ); // And, reschedule if(isCurThread)