PDA

View Full Version : Threads and mutable variables



ottawaDan
3rd October 2012, 16:53
Good morning,

I am doing some refactoring and I came across the following issue. I am running a multi-thread program, where a pair of threads (call the writingThread and readingThread) are accessing a shared object. One thread is processing an image and sets a data member in the shared object when the desired condition occurs while the second thread (readingThread) waits for the given condition (data member set to continue its process).

The shared object has the following declaration:


class TargetTracking : public QObject
{
...
public:
bool isTargetAligned() { return m_targetIsAligned; }
void overrideTargetAlignment( const bool action) { m_overrideTargetAlignment = action; }
void waitForAlign();

private:
bool m_targetIsAligned;
bool m_overrideTargetAlignment;
}

Both threads have a pointer to TargetTracking object as data member.

Prior to the refactoring, the code was working correctly. However, the data member m_targetIsAligned was declared as a member of writingThread, and readingThread was calling and waiting for writingThread::waitForAlign() to return. The refactoring process included moving code form writingThread to TargetTracking class as shown above.

After some tests, I was only able to make the code work if the mutable keyword was added to the declaration of the two booleans



private:
mutable bool m_targetIsAligned;
mutable bool m_overrideTargetAlignment;
}

I have done some reading about the use of mutable; ie 'casting away const', however, I am lost as for why it is required in this case. Before using mutable, I tried wrapping the access to the booleans using mutex but that did not work either.

So my question is why the mutable attribute required here?

Thanks in advance
Daniel

wysota
3rd October 2012, 17:47
The code above doesn't enforce the use of mutable. It is the code that you didn't post that requires it. Somewhere in your code you have a method declared as const or accepting the TargetTracking instance as a const object and some code is trying to change this const (through inheritance from the class) member which is forbidden. Therefore rather than using "mutable", you should pay attention to your const methods and const objects. Use mutable only as a means of last resort or when you know what you are doing (e.g. when implementing a cache that needs to be built from within a const method). And of course this is plain C++, nothing Qt specific.

ottawaDan
4th October 2012, 21:42
Thanks for the reply.

The shared object (TargetTracking) is instantiated in the gui thread and passed as a constructor argument for the reading and writing threads



m_targetTracking = new TargetTracking( ...);
m_targetTrackingThread = new TargetTrackingThread( m_targetTracking); // writing thread
m_targetTrackingThread->start();


The TargetTrakingThread is declared as

class TargetTrakingThread : public QThread {
Q_OBJECT

public:
TargetTrakingThread( TargetTracking* targetTracking);
~TargetTrakingThread();

void autoTargetAlign();
void stopTargetAlign() { m_stop = true; }

protected:
void run();

private:
TargetTracking* targetTracking;
bool m_stop;
};

The waitForAlign method is essentially polling the data member and is defined as


void TargetTracking::waitForAlign()
{
while (!m_targetIsAligned && m_autoTargetAlign) {
usleep( 100000);
}
}

This method is called by the readingThread

The data member m_targetIsAligned is set in a different method TargetTracking::alignTarget as shown


void TargetTracking::alignTarget()
{
...
locateTarget();

// check if the target is centred
if (!m_overrideTargetlAlignment) {
m_targetIsAligned = checkIfTargetCentred();
} else {
m_targetIsAligned = true;
}
...
}

This method is called by the writing thread

I reviewed the code, and the only instance where const appears is in the different get methods for the TargetTracking data members. There are two methods setting the control booleans and are defined as


class TargetTracking : public QObject
{
...
void setAutoTargetAlign( const bool align) { m_autoTargetAlign = align; }
void overrideTargetAlignment( const bool action) { m_overrideTargetAlignment = action; }
...
}


Could these instance of const be the root of my problem?
Also, there is no inheritance for the classes mentioned here. Note that TargetTracking has some static methods declared

So the addition of the mutable attribute still puzzle me.

Daniel

wysota
4th October 2012, 22:38
Why aren't you just using a wait condition? Using such a spinlock as yours waitForAlign is very heavy for your cpu and it's prone to race conditions.

ottawaDan
9th October 2012, 23:12
This was to be my next step in refactoring the code.

I did replace polling by a QWaitCondition in the waitForAlign() method, removed the mutable attribute and now the code is working as expected.

Thanks again,
Daniel