Possible problem with condition releasing

[davevest]

I think this is a bug but since I have only just 
started using the threads library it could be a 
misunderstanding on my part.

Since the actual application is quite complex I've 
created an imaginary scenario that shows the problem.

Scenario: A single 'throttle' manager that only lets 
three threads at a time perform some work. When a 
thread wants to work it must successfully addRef and 
when it has finished it releases.

Problem: If all available 'slots' are used I am using 
a condition in the addRef to wait for a release before 
continuing. Therefore I would expect that after the 
wait I can successfully assert that a slot is 
available. However, sometimes this does not appear to 
be true and I can only assume that sometimes the 
condition is not working properly. 

Work around: In CThrottle::release remove the 'unlock' 
so that the notify_one occurs while the method still 
has a lock on the m_mutexChangeState.

Have I misunderstood how condition is supposed to work?
Thanks,
Dave.


The code:

#include <boost\thread\thread.hpp>
#include <boost\thread\recursive_mutex.hpp>
#include <boost/thread/condition.hpp>
#include <boost/shared_ptr.hpp>
#include <windows.h>
#include <crtdbg.h>
#include <vector>

class CThrottle
{
	int m_nThrottle;
public:
	CThrottle() : m_nThrottle(0)  {}

	void addRef()	
	{	
		boost::mutex::scoped_lock lockIncrement
( m_mutexIncrement );
		boost::recursive_mutex::scoped_lock 
lockChangeState( m_mutexChangeState );

		if ( !isAllowed() )
			m_condition.wait( 
lockChangeState );

		_ASSERT( isAllowed() ); // PROBLEM

		++m_nThrottle;
	}

	void release()
	{
		boost::recursive_mutex::scoped_lock 
lockChangeState( m_mutexChangeState );
		--m_nThrottle;
		lockChangeState.unlock();

		m_condition.notify_one();
	}
		
	bool isAllowed()
	{
		boost::recursive_mutex::scoped_lock 
lockChangeState( m_mutexChangeState );
		return m_nThrottle < 3;
	}
private:
	boost::mutex m_mutexIncrement;
	boost::recursive_mutex m_mutexChangeState;
	boost::condition m_condition;
};

class CTestThread
{
public:
	CTestThread( CThrottle * pThrottle ) : 
m_pThrottle( pThrottle ) {}
	void operator()()	
	{	
		m_pThrottle->addRef();
		std::cout << "Do something\n";
		m_pThrottle->release();
	}
	CThrottle * m_pThrottle;
};

typedef boost::shared_ptr<CTestThread> _TestThreadPtr;

int main(int argc, _TCHAR* argv[])
{
	CThrottle * pThrottle = new CThrottle();
	boost::thread_group groupThreads;
	std::vector<_TestThreadPtr> list;

	for( int n=0; n < 10000; ++n )
	{
		_TestThreadPtr ptrTest( new CTestThread
( pThrottle ) );
		groupThreads.create_thread( 
*ptrTest.get() );
		list.push_back( ptrTest );
	}

	groupThreads.join_all();

	delete pThrottle;

	return 0;
}

[Anthony Williams]

Logged In: YES 
user_id=60756

This is not a bug, but a property of conditions. If you are
waiting on a predicate, use the predicate version of wait:

cond.wait(some_lock,some_predicate)

Alternatively, test isAllowed() yourself in a loop, and keep
waiting on the condition until isAllowed is true.

The docs for pthread_cond_wait say:

"When using condition variables there is always a Boolean
predicate involving shared variables associated with each
condition wait that is true if the thread should proceed.
Spurious wakeups from the pthread_cond_timedwait() or
pthread_cond_wait() functions may occur. Since the return
from pthread_cond_timedwait() or pthread_cond_wait() does
not imply anything about the value of this predicate, the
predicate should be re-evaluated upon such return."

and the same applies to boost::condition::wait.