Threads and mutable variables

[ottawaDan]

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;
}

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;
}

To copy to clipboard, switch view to plain text mode 

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;
}

private:
    mutable bool    m_targetIsAligned;
    mutable bool    m_overrideTargetAlignment;
}

To copy to clipboard, switch view to plain text mode 

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]

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]

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();

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

To copy to clipboard, switch view to plain text mode 

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;
};

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;
};

To copy to clipboard, switch view to plain text mode 

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

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

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

To copy to clipboard, switch view to plain text mode 

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;
        }
...
}

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

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

To copy to clipboard, switch view to plain text mode 

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; }
...
}

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

To copy to clipboard, switch view to plain text mode 

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]

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]

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