From d6cf9622a46065819348a26b4d44add2a9dffc26 Mon Sep 17 00:00:00 2001 From: Randell Jesup Date: Thu, 24 Mar 2022 16:38:57 +0000 Subject: [PATCH] Bug 1761107: Make Monitor.Wait() require the monitor be locked r=nika Differential Revision: https://phabricator.services.mozilla.com/D141893 --- xpcom/threads/Monitor.h | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/xpcom/threads/Monitor.h b/xpcom/threads/Monitor.h index 5a9135810091..4a6636e080ca 100644 --- a/xpcom/threads/Monitor.h +++ b/xpcom/threads/Monitor.h @@ -32,8 +32,8 @@ class CAPABILITY Monitor { [[nodiscard]] bool TryLock() TRY_ACQUIRE(true) { return mMutex.TryLock(); } void Unlock() CAPABILITY_RELEASE() { mMutex.Unlock(); } - void Wait() { mCondVar.Wait(); } - CVStatus Wait(TimeDuration aDuration) { return mCondVar.Wait(aDuration); } + void Wait() REQUIRES(this) { mCondVar.Wait(); } + CVStatus Wait(TimeDuration aDuration) REQUIRES(this) { return mCondVar.Wait(aDuration); } void Notify() { mCondVar.Notify(); } void NotifyAll() { mCondVar.NotifyAll(); } @@ -126,8 +126,17 @@ class SCOPED_CAPABILITY MOZ_STACK_CLASS BaseMonitorAutoLock { } ~BaseMonitorAutoLock() CAPABILITY_RELEASE() { mMonitor->Unlock(); } - void Wait() { mMonitor->Wait(); } - CVStatus Wait(TimeDuration aDuration) { return mMonitor->Wait(aDuration); } + // It's very hard to mess up MonitorAutoLock lock(mMonitor); ... lock.Wait(). + // The only way you can fail to hold the lock when you call lock.Wait() is to + // use MonitorAutoUnlock. For now we'll ignore that case. + void Wait() { + mMonitor->AssertCurrentThreadOwns(); + mMonitor->Wait(); + } + CVStatus Wait(TimeDuration aDuration) { + mMonitor->AssertCurrentThreadOwns(); + return mMonitor->Wait(aDuration); + } void Notify() { mMonitor->Notify(); } void NotifyAll() { mMonitor->NotifyAll(); } @@ -243,9 +252,13 @@ class SCOPED_CAPABILITY MOZ_STACK_CLASS ReleasableMonitorAutoLock { } } - void Wait() { mMonitor->Wait(); } + // See BaseMonitorAutoLock::Wait + void Wait() { + mMonitor->AssertCurrentThreadOwns(); // someone could have called Unlock() + mMonitor->Wait(); + } CVStatus Wait(TimeDuration aDuration) { - MOZ_ASSERT(mLocked); + mMonitor->AssertCurrentThreadOwns(); return mMonitor->Wait(aDuration); }