Boost thread mutex

Started by
6 comments, last by the_edd 14 years, 10 months ago
Hi there... I'm very new to multithreaded programming, and I've a question regarding the boost thread library. I have this class (only relevant code):

class cBall
{
private:
	cBall();
	static cBall* m_pGlobalBall;

        boost::shared_ptr<boost::thread> m_BallThread;
        boost::mutex	m_BallMutex;
        bool		m_bThreadStop;


        [... cut ...]
public:
	~cBall();

	void	StartUpdateThread();		
	void	JoinUpdateThread();		
	void	StopUpdateThread();

	void	Render();	
        
        [... cut ...]
};					

And there's the implementation


void cBall::UpdatePosition()
{
	do
	{
           boost::mutex::scoped_lock l(m_BallMutex);

           [do some calculation]

	    if (m_bDataRead)
	    {
                m_vPosizioneReale = m_vPosizione;
		m_bDataRead = FALSE;
	    }           

	}
	while(!m_bThreadStop);

}

void cBall::StartUpdateThread()
{
	m_BallThread = boost::shared_ptr<boost::thread>(new boost::thread(boost::bind(&cBall::UpdatePosition, this)));
	m_bThreadStop = FALSE;
}

void cBall::JoinUpdateThread()
{
	m_BallThread->join();
}

void cBall::StopUpdateThread()
{
	m_bThreadStop = TRUE;
	m_BallThread->join();
}

void cBall::Render()
{
	if (!m_bDataRead)
	{
		boost::mutex::scoped_lock l(m_BallMutex);

		m_NodeBall->setPosition(m_VPosizioneReale);
		m_bDataRead = TRUE;
	}

}

The "render" function runs on the main thread. When I get to ball set position I get wrong data in m_VPosizioneReale... I'm sure I'm doing something wrong, but I don't know where... Many thanks in advance... Juri
Advertisement
There is nothing wrong with this code.
Write an actual test case.
Try marking m_VPosizioneReale as volatile, as well as any other variables accessed from multiple threads.
In cBall::Render(), you're reading from m_bDataRead without first locking the mutex.
Locking or unlocking the mutex will cause a memory barrier, which prevents the CPU from re-ordering read/write instructions and screwing up multi-threaded data sharing.

Without a memory barrier, the CPU can re-order this code:
                m_vPosizioneReale = m_vPosizione;		m_bDataRead = FALSE;
to look like this instead:
		m_bDataRead = FALSE;                m_vPosizioneReale = m_vPosizione;
which will break your program.

[EDIT] Actually, this shouldn't be a problem for you.
Even if the wrong value of m_bDataRead is read, both threads should be placing a barrier after writing to and before reading from m_VPosizioneReale (so it's value should always be valid/up-to-date, even if m_bDataRead is invalid).
Many thanks for the answers...

The volatile keyword solved the data corruption, but the animation is not smooth as I expected.

The calculations done in the UpdatePosition worked previously well in single threaded app (the UpdatePosistion where called once per frame, just before the render).

Now the object movement is not smooth, it seems just strange, because I've checked the timings and the UpdatePosition procedure cycle takes about 0.00129 seconds, and the rendering cycle is on 50/60 fps, so ~0.02 seconds....

volatile probably isn't doing what you think it does.

Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]

Quote:Original post by swiftcoder
volatile probably isn't doing what you think it does.


Thanks for the interesting link... I was confused by MSDN too, becuase in this page MSDN LINK they say:

Quote:"The volatile keyword is a type qualifier used to declare that an object can be modified in the program by something such as the operating system, the hardware, or a concurrently executing thread."


For my problem I've now removed the volatile keyword but I've the same stutter movement of the object.

As I have one thread that writes the data and another one that reads it, maybe should I use the boost shared_mutex for read/write operations?
In case could anyone post a little example that fits my case? I've tried to read the boost documentation, but I can't find the right way to implement the shared mutex within my code...

Once again thanks
Quote:Original post by kiske1
I have this class (only relevant code):

class cBall{private:	cBall();	static cBall* m_pGlobalBall;


Try really hard to avoid class-/file-/function-static and global objects of any kind in threaded code.

Quote:
And there's the implementation

// ...void cBall::StartUpdateThread(){	m_BallThread = boost::shared_ptr<boost::thread>(new boost::thread(boost::bind(&cBall::UpdatePosition, this)));	m_bThreadStop = FALSE;}



These two statements are in the wrong order. It's entirely possible that one iteration of the loop in UpdatePosition will occur before m_bThreadStop is set to FALSE. Perhaps this is unrelated to your current problem, but it is a bug nonetheless.

You can also write the thread creation line somewhat more succinctly:

m_BallThread.reset(new boost::thread(boost::bind(&cBall::UpdatePosition, this)));


Quote:
// ...void cBall::StopUpdateThread(){	m_bThreadStop = TRUE;	m_BallThread->join();}



m_bThreadStop should be protected by the mutex as it is read from and written to by different threads. If you don't do this, old values may stay resident in one of the various layers of cache.

Quote:
// ...void cBall::Render(){	if (!m_bDataRead)	{		boost::mutex::scoped_lock l(m_BallMutex);		m_NodeBall->setPosition(m_VPosizioneReale);		m_bDataRead = TRUE;	}}



You have the same issue again here with m_bDataRead.

You need to have a very clear idea of the invariants that your mutex is protecting. All the levels of the cache must have a consistent view of your shared data for the threads to be properly coordinated.

This topic is closed to new replies.

Advertisement