Addressed a deadlock race condition: Acquiration of condition variable

and thread spinlock was reverse in Wait() and Notify(). The thread lock
is now the outer lock -- this way it is still possible to call Notify()
with the thread lock being held.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@22401 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/private/kernel/condition_variable.h

@@ -82,8 +82,7 @@ protected:
 			void				Notify(bool all, bool threadsLocked);
 
 private:
-			void				_Notify(bool all, bool threadsLocked,
+			void				_Notify(bool all, status_t result);
-									status_t result);
 
 protected:
 			const void*			fObject;

src/system/kernel/condition_variable.cpp

@@ -134,6 +134,7 @@ PrivateConditionVariableEntry::Wait(uint32 flags)
 	}
 
 	InterruptsLocker _;
+	SpinLocker threadLocker(thread_spinlock);
 	SpinLocker locker(sConditionVariablesLock);
 
 	// get first entry for this thread
@@ -159,10 +160,9 @@ PrivateConditionVariableEntry::Wait(uint32 flags)
 	thread->condition_variable_entry = firstEntry;
 	thread->sem.blocking = -1;
 
-	GRAB_THREAD_LOCK();
 	locker.Unlock();
 	scheduler_reschedule();
-	RELEASE_THREAD_LOCK();
+	threadLocker.Unlock();
 
 	return firstEntry->fResult;
 }
@@ -289,6 +289,7 @@ PrivateConditionVariable::Unpublish(bool threadsLocked)
 	ASSERT(fObject != NULL);
 
 	InterruptsLocker _;
+	SpinLocker threadLocker(threadsLocked ? NULL : &thread_spinlock);
 	SpinLocker locker(sConditionVariablesLock);
 
 #if KDEBUG
@@ -304,7 +305,7 @@ PrivateConditionVariable::Unpublish(bool threadsLocked)
 	fObjectType = NULL;
 
 	if (fEntries)
-		_Notify(true, threadsLocked, B_ENTRY_NOT_FOUND);
+		_Notify(true, B_ENTRY_NOT_FOUND);
 }
 
 
@@ -314,6 +315,7 @@ PrivateConditionVariable::Notify(bool all, bool threadsLocked)
 	ASSERT(fObject != NULL);
 
 	InterruptsLocker _;
+	SpinLocker threadLocker(threadsLocked ? NULL : &thread_spinlock);
 	SpinLocker locker(sConditionVariablesLock);
 
 #if KDEBUG
@@ -325,18 +327,17 @@ PrivateConditionVariable::Notify(bool all, bool threadsLocked)
 #endif
 
 	if (fEntries)
-		_Notify(all, threadsLocked, B_OK);
+		_Notify(all, B_OK);
 }
 
 
-//! Called with interrupts disabled and the condition variable spinlock held.
+/*! Called with interrupts disabled and the condition variable spinlock and
+	thread lock held.
+*/
 void
-PrivateConditionVariable::_Notify(bool all, bool threadsLocked, status_t result)
+PrivateConditionVariable::_Notify(bool all, status_t result)
 {
 	// dequeue and wake up the blocked threads
-	if (!threadsLocked)
-		GRAB_THREAD_LOCK();
-
 	while (PrivateConditionVariableEntry* entry = fEntries) {
 		fEntries = entry->fVariableNext;
 		entry->fVariableNext = NULL;
@@ -370,36 +371,18 @@ PrivateConditionVariable::_Notify(bool all, bool threadsLocked, status_t result)
 		if (!all)
 			break;
 	}
-
-	if (!threadsLocked)
-		RELEASE_THREAD_LOCK();
 }
 
 
 // #pragma mark -
 
 
-/*!	Interrupts must be disabled, thread lock must be held. Note, that the thread
+/*!	Interrupts must be disabled, thread lock must be held.
-	lock may temporarily be released.
 */
 status_t
 condition_variable_interrupt_thread(struct thread* thread)
 {
-	if (thread == NULL || thread->state != B_THREAD_WAITING
-		|| thread->condition_variable_entry == NULL) {
-		return B_BAD_VALUE;
-	}
-
-	thread_id threadID = thread->id;
-
-	// We also need the condition variable spin lock, so, in order to respect
-	// the locking order, we must temporarily release the thread lock.
-	RELEASE_THREAD_LOCK();
 	SpinLocker locker(sConditionVariablesLock);
-	GRAB_THREAD_LOCK();
-
-	// re-get the thread and do the checks again
-	thread = thread_get_thread_struct_locked(threadID);
 
 	if (thread == NULL || thread->state != B_THREAD_WAITING
 		|| thread->condition_variable_entry == NULL) {

src/system/kernel/signal.cpp

@@ -274,8 +274,7 @@ is_signal_blocked(int signal)
 
 
 /*!	Tries to interrupt a thread waiting for a semaphore or a condition variable.
-	Interrupts must be disabled, the thread lock be held. Note, that the thread
+	Interrupts must be disabled, the thread lock be held.
-	lock may temporarily be released.
 */
 static status_t
 signal_interrupt_thread(struct thread* thread)