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.
This commit is contained in:
Augustin Cavalier 2018-10-29 00:38:30 -04:00
parent d787141342
commit adb4e6e8c5

View File

@ -572,19 +572,7 @@ dump_rw_lock_info(int argc, char** argv)
void void
mutex_init(mutex* lock, const char *name) mutex_init(mutex* lock, const char *name)
{ {
lock->name = name; mutex_init_etc(lock, name, 0);
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);
} }
@ -618,7 +606,7 @@ mutex_destroy(mutex* lock)
#if KDEBUG #if KDEBUG
if (lock->waiters != NULL && thread_get_current_thread_id() if (lock->waiters != NULL && thread_get_current_thread_id()
!= lock->holder) { != lock->holder) {
panic("mutex_destroy(): there are blocking threads, but caller doesn't " panic("mutex_destroy(): there are blocking threads, but caller doesn't "
"hold the lock (%p)", lock); "hold the lock (%p)", lock);
if (_mutex_lock(lock, &locker) != B_OK) if (_mutex_lock(lock, &locker) != B_OK)
@ -636,6 +624,12 @@ mutex_destroy(mutex* lock)
} }
lock->name = NULL; lock->name = NULL;
lock->flags = 0;
#if KDEBUG
lock->holder = 0;
#else
lock->count = INT16_MIN;
#endif
locker.Unlock(); locker.Unlock();
@ -717,7 +711,7 @@ _mutex_lock(mutex* lock, void* _locker)
panic("_mutex_lock(): double lock of %p by thread %" B_PRId32, lock, panic("_mutex_lock(): double lock of %p by thread %" B_PRId32, lock,
lock->holder); lock->holder);
} else if (lock->holder == 0) } else if (lock->holder == 0)
panic("_mutex_lock(): using unitialized lock %p", lock); panic("_mutex_lock(): using uninitialized lock %p", lock);
#else #else
if ((lock->flags & MUTEX_FLAG_RELEASED) != 0) { if ((lock->flags & MUTEX_FLAG_RELEASED) != 0) {
lock->flags &= ~MUTEX_FLAG_RELEASED; lock->flags &= ~MUTEX_FLAG_RELEASED;