lockless thread/process handle counting and id assignment

kernel/memory.cpp

@@ -1678,7 +1678,7 @@ void MMBalanceThread() {
 			pmm.nextProcessToBalance = process->allItem.nextItem ? process->allItem.nextItem->thisItem : nullptr;
 
 			if (process->handles) {
-				process->handles++;
+				OpenHandleToObject(process, KERNEL_OBJECT_PROCESS, ES_FLAGS_DEFAULT);
 				break;
 			}
 		}

kernel/module.h

@@ -196,7 +196,7 @@ enum KernelObjectType : uint32_t {
 
 // TODO Rename to KObjectReference and KObjectDereference?
 void CloseHandleToObject(void *object, KernelObjectType type, uint32_t flags = 0);
-bool OpenHandleToObject(void *object, KernelObjectType type, uint32_t flags = 0, bool maybeHasNoHandles = false);
+bool OpenHandleToObject(void *object, KernelObjectType type, uint32_t flags = 0);
 
 // ---------------------------------------------------------------------------------------------------------------
 // Module loading.

kernel/objects.cpp

@@ -103,7 +103,7 @@ KMutex objectHandleCountChange;
 
 // TODO Use uint64_t for handle counts, or restrict OpenHandleToObject to some maximum (...but most callers don't check if OpenHandleToObject succeeds).
 
-bool OpenHandleToObject(void *object, KernelObjectType type, uint32_t flags, bool maybeHasNoHandles) {
+bool OpenHandleToObject(void *object, KernelObjectType type, uint32_t flags) {
 	bool hadNoHandles = false, failed = false;
 
 	switch (type) {
@@ -116,20 +116,11 @@ bool OpenHandleToObject(void *object, KernelObjectType type, uint32_t flags, boo
 		} break;
 
 		case KERNEL_OBJECT_PROCESS: {
-			KSpinlockAcquire(&scheduler.lock);
+			hadNoHandles = 0 == __sync_fetch_and_add(&((Process *) object)->handles, 1);
-			Process *process = (Process *) object;
-			if (!process->handles) hadNoHandles = true;
-			else process->handles++; // NOTE Scheduler::OpenProcess and MMBalanceThread also adjust process handles.
-			KernelLog(LOG_VERBOSE, "Scheduler", "open process handle", "Opened handle to process %d; %d handles.\n", process->id, process->handles);
-			KSpinlockRelease(&scheduler.lock);
 		} break;
 
 		case KERNEL_OBJECT_THREAD: {
-			KSpinlockAcquire(&scheduler.lock);
+			hadNoHandles = 0 == __sync_fetch_and_add(&((Thread *) object)->handles, 1);
-			Thread *thread = (Thread *) object;
-			if (!thread->handles) hadNoHandles = true;
-			else thread->handles++;
-			KSpinlockRelease(&scheduler.lock);
 		} break;
 
 		case KERNEL_OBJECT_SHMEM: {
@@ -208,11 +199,7 @@ bool OpenHandleToObject(void *object, KernelObjectType type, uint32_t flags, boo
 	}
 
 	if (hadNoHandles) {
-		if (maybeHasNoHandles) {
+		KernelPanic("OpenHandleToObject - Object %x of type %x had no handles.\n", object, type);
-			return false;
-		} else {
-			KernelPanic("OpenHandleToObject - Object %x of type %x had no handles.\n", object, type);
-		}
 	}
 
 	return !failed;
@@ -225,15 +212,12 @@ void CloseHandleToObject(void *object, KernelObjectType type, uint32_t flags) {
 		} break;
 
 		case KERNEL_OBJECT_THREAD: {
-			KSpinlockAcquire(&scheduler.lock);
 			Thread *thread = (Thread *) object;
-			if (!thread->handles) KernelPanic("CloseHandleToObject - All handles to thread %x have been closed.\n", thread);
+			uintptr_t previous = __sync_fetch_and_sub(&thread->handles, 1);
-			thread->handles--;
-			bool removeThread = thread->handles == 0;
-			// EsPrint("Thread %d has %d handles\n", thread->id, thread->handles);
-			KSpinlockRelease(&scheduler.lock);
 
-			if (removeThread) {
+			if (previous == 0) {
+				KernelPanic("CloseHandleToObject - All handles to thread %x have been closed.\n", thread);
+			} else if (previous == 1) {
 				scheduler.RemoveThread(thread);
 			}
 		} break;

kernel/scheduler.cpp

@@ -232,26 +232,28 @@ struct Scheduler {
 
 	// Variables:
 
-	KSpinlock lock;
+	KSpinlock lock; // The general lock. TODO Break this up!
+	KMutex allThreadsMutex; // For accessing the allThreads list.
 
 	KEvent killedEvent; // Set during shutdown when all processes have been terminated.
 	uintptr_t blockShutdownProcessCount;
-
+	size_t activeProcessCount;
-	LinkedList<Thread> activeThreads[THREAD_PRIORITY_COUNT], pausedThreads;
-	LinkedList<KTimer> activeTimers;
 
 	Pool threadPool, processPool, mmSpacePool;
+
+	LinkedList<Thread> activeThreads[THREAD_PRIORITY_COUNT];
+	LinkedList<Thread> pausedThreads;
+	LinkedList<KTimer> activeTimers;
 	LinkedList<Thread> allThreads;
 	LinkedList<Process> allProcesses;
-	EsObjectID nextThreadID;
-	EsObjectID nextProcessID;
-	size_t activeProcessCount;
 
 	volatile bool started, panic, shutdown;
 
 	uint64_t timeMs;
 
-	uint32_t currentProcessorID;
+	EsObjectID nextThreadID;
+	EsObjectID nextProcessID;
+	EsObjectID nextProcessorID;
 
 #ifdef DEBUG_BUILD
 	EsThreadEventLogEntry *volatile threadEventLog;
@@ -371,15 +373,19 @@ void Scheduler::MaybeUpdateActiveList(Thread *thread) {
 }
 
 void Scheduler::InsertNewThread(Thread *thread, bool addToActiveList, Process *owner) {
+	KMutexAcquire(&allThreadsMutex);
+	allThreads.InsertStart(&thread->allItem);
+	KMutexRelease(&allThreadsMutex);
+
 	KSpinlockAcquire(&lock);
-	EsDefer(KSpinlockRelease(&lock));
 
 	// New threads are initialised here.
 	thread->id = __sync_fetch_and_add(&nextThreadID, 1);
 	thread->process = owner;
 
-	owner->handles++; // Each thread owns a handles to the owner process.
+	// Each thread owns a handles to the owner process.
-			  // This makes sure the process isn't destroyed before all its threads have been destroyed.
+	// This makes sure the process isn't destroyed before all its threads have been destroyed.
+	OpenHandleToObject(owner, KERNEL_OBJECT_PROCESS, ES_FLAGS_DEFAULT);
 
 	thread->item.thisItem = thread;
 	thread->allItem.thisItem = thread;
@@ -395,7 +401,8 @@ void Scheduler::InsertNewThread(Thread *thread, bool addToActiveList, Process *o
 		// Some threads (such as idle threads) do this themselves.
 	}
 
-	allThreads.InsertStart(&thread->allItem);
+	KSpinlockRelease(&lock);
+	// The thread may now be terminated at any moment.
 }
 
 Thread *Scheduler::SpawnThread(const char *cName, uintptr_t startAddress, uintptr_t argument1, uint32_t flags, Process *process, uintptr_t argument2) {
@@ -490,10 +497,11 @@ void _RemoveProcess(KAsyncTask *task) {
 
 void CloseHandleToProcess(void *_process) {
 	KSpinlockAcquire(&scheduler.lock);
+
 	Process *process = (Process *) _process;
-	if (!process->handles) KernelPanic("CloseHandleToProcess - All handles to the process have been closed.\n");
+	uintptr_t previous = __sync_fetch_and_sub(&process->handles, 1);
-	process->handles--;
+	if (!previous) KernelPanic("CloseHandleToProcess - All handles to process %x have been closed.\n", process);
-	bool removeProcess = !process->handles;
+	bool removeProcess = previous == 1;
 
 	KernelLog(LOG_VERBOSE, "Scheduler", "close process handle", "Closed handle to process %d; %d handles remain.\n", process->id, process->handles);
 
@@ -561,8 +569,11 @@ void KillThread(KAsyncTask *task) {
 	Thread *thread = EsContainerOf(Thread, killAsyncTask, task);
 	GetCurrentThread()->SetAddressSpace(thread->process->vmm);
 
-	KSpinlockAcquire(&scheduler.lock);
+	KMutexAcquire(&scheduler.allThreadsMutex);
 	scheduler.allThreads.Remove(&thread->allItem);
+	KMutexRelease(&scheduler.allThreadsMutex);
+
+	KSpinlockAcquire(&scheduler.lock);
 	thread->process->threads.Remove(&thread->processItem);
 
 	KernelLog(LOG_INFO, "Scheduler", "killing thread", 
@@ -954,7 +965,7 @@ void Scheduler::CreateProcessorThreads(CPULocalStorage *local) {
 	idleThread->terminatableState = THREAD_IN_SYSCALL;
 	idleThread->cName = "Idle";
 	local->currentThread = local->idleThread = idleThread;
-	local->processorID = __sync_fetch_and_add(&currentProcessorID, 1);
+	local->processorID = __sync_fetch_and_add(&nextProcessorID, 1);
 
 	if (local->processorID >= K_MAX_PROCESSORS) { 
 		KernelPanic("Scheduler::CreateProcessorThreads - Maximum processor count (%d) exceeded.\n", local->processorID);
@@ -1375,7 +1386,7 @@ Process *Scheduler::OpenProcess(uint64_t id) {
 		if (process->id == id 
 				&& process->handles /* if the process has no handles, it's about to be removed */
 				&& process->type != PROCESS_KERNEL /* the kernel process cannot be opened */) {
-			process->handles++;
+			OpenHandleToObject(process, KERNEL_OBJECT_PROCESS, ES_FLAGS_DEFAULT);
 			break;
 		}
 

kernel/syscall.cpp

@@ -1311,7 +1311,7 @@ SYSCALL_IMPLEMENT(ES_SYSCALL_SYSTEM_TAKE_SNAPSHOT) {
 }
 
 SYSCALL_IMPLEMENT(ES_SYSCALL_PROCESSOR_COUNT) {
-	SYSCALL_RETURN(scheduler.currentProcessorID, false);
+	SYSCALL_RETURN(scheduler.nextProcessorID, false);
 }
 
 SYSCALL_IMPLEMENT(ES_SYSCALL_PROCESS_OPEN) {