How do you use a std::atomic<bool> to ensure mutual exclusion in a code block?

Question

I have a function that accesses(reads and writes to) a std::atomic<bool> variable. I'm trying to understand the order of execution of instructions so as to decide whether atomic will suffice or if I've got to use mutexes here. The function is given below -

// somewhere member var 'executing' is defined as std::atomic<bool>`

int A::something(){
    
    int result = 0;
    // my intention is only one thread should enter next block
    // others should just return 0
    if(!executing){
        executing = true;

        ...
        // do some really long processing
        ...
       
        result    = processed;
        executing = false;
    }
    
    return result;
}

---

I've read this page on cppreference which mentions -

Each instantiation and full specialization of the std::atomic template defines an atomic type. If one thread writes to an atomic object while another thread reads from it, the behavior is well-defined (see memory model for details on data races)

and on Memory model page the following is mentioned -

When an evaluation of an expression writes to a memory location and another evaluation reads or modifies the same memory location, the expressions are said to conflict. A program that has two conflicting evaluations has a data race unless either

• both conflicting evaluations are atomic operations (see std::atomic)

• one of the conflicting evaluations happens-before another (see std::memory_order)

If a data race occurs, the behavior of the program is undefined.

and slight below it reads -

When a thread reads a value from a memory location, it may see the initial value, the value written in the same thread, or the value written in another thread. See std::memory_order for details on the order in which writes made from threads become visible to other threads.

---

This is slightly confusing to me, which one of above 3 statements are actually happening here?

When I perform if(!executing){ is this instruction an atomic instruction here? and more important - is it guaranteed that no other thread will enter that if loop if one two threads will enter that if body since first one will set executing to true?

And if something's wrong with the mentioned code, how should I rewrite it so that it reflects original intention..

Netherlands

if(!executing){ executing = true; another thread can get in between these 2 statements. Have a look at std::atomic<>.compare_exchange_XXX – Disturbance

Your use case is strange. Why fire multiple threads if only one of them can do some really long processing at a time? – Brief

@Brief user might click on the button several times and I don't want to disable button for the time being, if the result hasn't been computed yet the return value 0 will be checked and user will be notified to wait.. is there a better way to do this? – Netherlands

Yes, disable the button, or change its state so that it tells the user what's going on. – Brief

Agreed with @Mat. A disabled button signals that the user actually hit it, the program noticed it and it's working on it. – Benedetto

@Brief I explicitly mentioned I cannot disable the button :/ even if I would some user will fire the api himself and will do whatever the button was doing at it's first place. This isn't a native application where I can hide my api. – Netherlands

No, you said you didn't want to, not that you couldn't, and you said nothing about an API. If you want advice about how to do whatever it is you're trying to achieve, you'll need to provide more accurate details about what that is exactly. (In your question.) – Brief

@Brief my question isn't about hiding or showing button or any other interface to the user. – Netherlands

Got that. But what is it exactly then? The code you posted is broken in terms of synchronization. If you want to know how to do something correctly, you need to describe that thing correctly too, especially when threads are involved. These things are hard, and missing details or misunderstood requirements usually lead to pain and tears in production. – Brief

@Brief I've written it as comment inside the code. // my intention is only one thread should enter next block // others should just return 0. I apologize if it wasn't clear, I just want other incoming functions to just return 0 without doing that computation part, otherwise they'd be all doing same thing and return multiple times. And I'm the one who has recieved this code from someone else to maintain, the tests are passing but I somehow caught it and was suspicious that this might be wrong. – Netherlands

Answer 1

Can you use a std::atomic_bool to ensure mutual exclusion in a code block?

Yep. std::atomic_bool is sufficient to implement a spinlock, which is simpler (although also less general and a worse default) than the std::mutex in the other answer, and still more complicated than you actually need.

Your problem is here:

When I perform if(!executing){ ... is it guaranteed that no other thread will enter that if loop if one two threads will enter that if body since first one will set executing to true?

Both the load (for comparison) and the store are individually atomic, but they're not a single atomic operation, which literally means they're divisible (by another operation in a different thread).

However, there is an atomic load+compare+store for exactly this purpose: compare_exchange.

Your code should be something like:

int A::something() {
    
    int result = 0;

    // if executing is false,
    // atomically set it to true
    bool expect_executing = false;
    if(executing.compare_exchange_strong(
        expect_executing, true)) {

        // enter the branch only if the exchange succeeded
        // (executing was false and is now true)
        // ... do some really long processing ...
       
        result = processed;
        executing = false;
    }
    
    return result;
}

You can do something similar with std::atomic_flag::test_and_set, but I stuck with your existing type.

You may also be able to weaken the default (sequential consistency) ordering.

Sterner

Yes, mutual exclusion only needs acq/rel, not seq_cst. acquire ordering on the CAS (or exchange) that takes the lock, and executing.store(false, std::memory_order_release). But yes, you do need an atomic RMW to take the lock, or a different algorithm like en.wikipedia.org/wiki/Dekker%27s_algorithm to achieve mutual exclusion (between two threads, or can be generalized to more) with only pure-load and pure-store with sequential consistency. – Lefthand

This answer doesn't seem to add much beyond amit's answer, other than mentioning that weaker ordering is possible but without saying what strength is necessary. I guess some explanatory text at the top to more directly answer the question? – Lefthand

I think that's about the size of it. I'd started answering before I realized Amit had beaten me to it. Probably their answer's presentation just wasn't what I was expecting to see. – Sterner

Answer 2

If I understand correctly, you are trying to ensure that only one thread will ever execute a stretch of code at the same time. This is exactly what a mutex does. Since you mentioned that you don't want threads to block if the mutex is not available, you probably want to take a look at the try_lock() method of std::mutex. See the documentation of std::mutex.

Now to why your code does not work as intended: Simplifying a little, std::atomic guarantees that there will be no data races when accessing the variable concurrently. I.e. there is a well defined read-write order. This doesn't suffice for what you are trying to do. Just imagine the if branch:

if(!executing) {
   executing = true;

Remember, only the read-write operations on executing are atomic. This leaves at least the negation ! and the if itself unsynchronized. With two threads, the execution order could be like this:

1. Thread 1 reads executing (atomically), value is false
2. Thread 1 negates the value read from executing, value = true
3. Thread 1 evaluates the condition and enters the branch
4. Thread 2 reads executing (atomically), value is false
5. Thread 1 set executing to true
6. Thread 2 negates the value, which was read as false and is now true again
7. Thread 2 enters the branch...

Now both threads have entered the branch.

I would suggest something along these lines:

std::mutex myMutex;

int A::something(){

    int result = 0;
    // my intention is only one thread should enter next block
    // others should just return 0
    if(myMutex.try_lock()){

        ...
        // do some really long processing
        ...

        result    = processed;
        myMutex.unlock();
    }

    return result;
}

Cheekbone

std::atomic<>.compare_exchange_XXX will also do what the OP requires. No need for a full mutex. – Disturbance

Whilst mutex is definitely the right answer to this, you can achieve similar functionality to this example by using compare_exchange_strong which will atomically read and conditionally update the variable in a single atomic operation – Hairstyle

@RichardCritten. Yes it can work but that doesn't mean its the correct answer to the implied question. – Hairstyle

This really seems a good answer but I ain't sure about this part - This function is allowed to fail spuriously and return false even if the mutex is not currently locked by any other thread. – Netherlands

It's true that try_lock() can fail spuriously in rare cases. This is in the standard to allow some very fast implementations. However, if you call it multiple times from various threads, it is safe to assume that one of them will acquire the lock. For more details, see for example here and here – Cheekbone

Answer 3

The issue in your code is that the two atomic operations, load (for comparison) and save, are separated, allowing for a lot to happen between them:

if(!executing) { // <-- Load (for compare).
   // (Several threads can reach here, before one of them will execute the next line.)
   executing = true; // <-- Save.
.
.
.

}

--

The proper way to use, is to replace the two with a single atomic operation:

bool expected{ false };
if (executing_.compare_exchange_strong(expected, true)) { // expected: false, desired: true.
.
.
.

}

compare_exchange_strong always accesses the contained value to read it, and if the comparison is true (contained value == expected) it then also exchanges it with the desired value and returns true. But the entire operation is atomic.

or:

if (!executing_.exchange(true)) { // If previously false, it wons the race to take the lock
.
.
.

}

exchange atomically replaces the underlying value with desired (a read-modify-write operation) and returns the value of the atomic variable before the call.

--

Example:

#include <atomic>
#include <mutex>

class A final
{
public:

    int DoSomething() // Only one thread should enter next block; others should just return 0
    {
        int result{ 0 };

        bool expected{ false };
        if (executing_.compare_exchange_strong(expected, true)) { // expected: false, desired: true.

            // Do some really long processing...
            result = 5; // 5 for example. In reality, the result of the processing.

            executing_ = false;
        }

        return result;
    }

protected:
    std::atomic_bool executing_{ false };
};

Isotone

The OP's current code doesn't involve and exchange, just a load and a separate store. That's the problem; if they did if (!executing.exchange(true) ), that would work as well as CAS. (Of course, if their code matched your comments, with a separate load and then an exchange which didn't use the return value, that would have the same race condition. But it's better to accurately describe what they're doing. An = assignment to an atomic var isn't an exchange. If compiling for x86, an xchg instruction merely happens to be a more efficient way to get the seq_cst semantics than mov+mfence) – Lefthand