CODEWITHSUNDEEP
c++multithreading
Dess
Dess

Reputation: 2659

Trying to minimize checks of atomics on every iteration

From a multithreading perspective, is the following correct or incorrect?

I have an app which has 2 threads: the main thread, and a worker thread.

The main thread has a MainUpdate() function that gets called in a continuous loop. As part of its job, that MainUpdate() function might call a ToggleActive() method on the worker objects running on the worker thread. That ToggleActive() method is used to turn the worker objects on/off.

The flow is something like this.

// MainThread
while(true) {
    MainUpdate(...);
}

void MainUpdate(...) {
    for(auto& obj: objectsInWorkerThread) {
        if (foo()) 
          obj.ToggleActive(getBool());
    }
}

// Worker thread example worker ------------------------------
struct SomeWorkerObject {

    void Execute(...) {
       if(mIsActive == false) // %%%%%%% THIS!
          return;
       Update(...);
    } 

    void ToggleActive(bool active) {
        mIsActiveAtom = active;       // %%%%%%% THIS!
        mIsActive = mIsActiveAtom;    // %%%%%%% THIS!
    }

private:
    void Update(...) {...}

    std::atomic_bool mIsActiveAtom = true;
    volatile bool mIsActive = true;
};

I'm trying to avoid checking the atomic field on every invocation of Execute(), which gets called on every iteration of the worker thread. There are many worker objects running at any one time, and thus there would be many atomic fields checks.

As you can see, I'm using the non-atomic field to check for activeness. The value of the non-atomic field gets its value from the atomic field in ToggleActive().

From my tests, this seems to be working, but I have a feeling that it is incorrect.

Upvotes: 0

Views: 54

Answers (2)

user1643723
user1643723

Reputation: 4212

@hgminh is right, your code is not safe.

Synchronization is two way road — if you have a thread perform thread-safe write, another thread must perform thread-safe read. If you have a thread use a lock, another thread must use the same lock.

Think about inter-thread communication as message passing (incidentally, it works exactly that way in modern CPUs). If both sides don't share a messaging channel (mIsActiveAtom), the message might not be delivered properly.

Upvotes: 1

hgminh
hgminh

Reputation: 1268

volatile variable only guarantees that it is not optimized out and reorder by compiler and has nothing to do with multi-thread execution. Therefore, your program does have race condition since ToggleActive and Execute can modify/read mIsActive at the same time.

About performance, you can check if your platform support for lock-free atomic bool. If that is the case, checking atomic value can be very fast. I remember seeing a benchmark somewhere that show std::atomic<bool> has the same speed as volatile bool.

Upvotes: 2

Related Questions