From adb4e6e8c5f5fbf0638ade3ef1b7baef7c8e4cef Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Mon, 29 Oct 2018 00:38:30 -0400 Subject: [PATCH] kernel: Reset lock holder and count on mutex_destroy(). Previously, there were a number of circumstances where these were not getting reset properly, leading to some destroyed mutexes having holders of the last thread which locked them, and some with "-1", which meant that the next call to "mutex_lock" just behaved as if the lock was still valid (!), and so the unlucky caller would deadlock forever. Now we properly reset these fields, which means from now on attempts to lock or unlock destroyed mutexes will lead to "PANIC: uninitialized mutex" on KDEBUG kernels, and (as before) an infinite deadlock on non-KDEBUG kernels (perhaps we should store the thread_id of the locker on non-KDEBUG kernels also?). As the next commits will show, this already uncovered a number of bugs, and there are of course potentially more strange deadlocks caused by this. --- src/system/kernel/locks/lock.cpp | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/system/kernel/locks/lock.cpp b/src/system/kernel/locks/lock.cpp index 2108874f53..f8381a9aa7 100644 --- a/src/system/kernel/locks/lock.cpp +++ b/src/system/kernel/locks/lock.cpp @@ -572,19 +572,7 @@ dump_rw_lock_info(int argc, char** argv) void mutex_init(mutex* lock, const char *name) { - lock->name = name; - lock->waiters = NULL; - B_INITIALIZE_SPINLOCK(&lock->lock); -#if KDEBUG - lock->holder = -1; -#else - lock->count = 0; - lock->ignore_unlock_count = 0; -#endif - lock->flags = 0; - - T_SCHEDULING_ANALYSIS(InitMutex(lock, name)); - NotifyWaitObjectListeners(&WaitObjectListener::MutexInitialized, lock); + mutex_init_etc(lock, name, 0); } @@ -618,7 +606,7 @@ mutex_destroy(mutex* lock) #if KDEBUG if (lock->waiters != NULL && thread_get_current_thread_id() - != lock->holder) { + != lock->holder) { panic("mutex_destroy(): there are blocking threads, but caller doesn't " "hold the lock (%p)", lock); if (_mutex_lock(lock, &locker) != B_OK) @@ -636,6 +624,12 @@ mutex_destroy(mutex* lock) } lock->name = NULL; + lock->flags = 0; +#if KDEBUG + lock->holder = 0; +#else + lock->count = INT16_MIN; +#endif locker.Unlock(); @@ -717,7 +711,7 @@ _mutex_lock(mutex* lock, void* _locker) panic("_mutex_lock(): double lock of %p by thread %" B_PRId32, lock, lock->holder); } else if (lock->holder == 0) - panic("_mutex_lock(): using unitialized lock %p", lock); + panic("_mutex_lock(): using uninitialized lock %p", lock); #else if ((lock->flags & MUTEX_FLAG_RELEASED) != 0) { lock->flags &= ~MUTEX_FLAG_RELEASED;