How to Use std::atomic_bool or std::atomic_flag?

Hi I need to use boolean flags in callback functions in my program to control program behavior. The callback threads are executed in different threads than the threads that set them up. In one of my callback functions the program sets the boolean flag and in the other it checks a different unrelated boolean flag. Currently I am using std::atomic_bool's for this purpose.

I've read about the possibility of deadlock with std::atomic variables. Is this something to worry about in my case? Is there anything else to worry about?

Am I doing things correctly in using the atomic_bool's like normal variables? If not how should I do things?

Should I be using std::atomic_flag instead?

And the compiler didn't throw an error initializing the standbyResumeTimerDelayBool in as below in the class member list not sure if this is correct or not.

Thanks!

//some code omitted to keep things simple
//first callback function, this boolean flag is checked at one other point in the program
static ULONG Replicator::standbyResumeCallbackFunction(PVOID context, ULONG type, PVOID setting);
ULONG Replicator::standbyResumeCallbackFunction(PVOID context, ULONG type, PVOID setting)
{
    if (type == PBT_APMRESUMEAUTOMATIC)
    {
        Replicator* replicator = reinterpret_cast<Replicator*>(context);
        replicator->standbyResumeSetTimerDelayBool();
    }
}

//Replicator class members
void standbyResumeSetTimerDelayBool() { standbyResumeTimerDelayBool = true; }
std::atomic_bool standbyResumeTimerDelayBool = false;

//behavior modified here
void Replicator::autoBackup()
{
    if (standbyResumeTimerDelayBool)
    {
        standbyResumeTimerDelayBool = false;
        //do stuff
    }
}


//second callback function
static DWORD CALLBACK copyProgress(
             LARGE_INTEGER totalSize, LARGE_INTEGER totalTransferred,
             LARGE_INTEGER streamSize, LARGE_INTEGER streamTransferred,
             DWORD streamNo, DWORD callbackReason, HANDLE src, HANDLE dst,
             LPVOID data);

DWORD Worker::copyProgress(LARGE_INTEGER totalSize, LARGE_INTEGER totalTransferred, LARGE_INTEGER, LARGE_INTEGER, DWORD, DWORD, HANDLE, HANDLE, LPVOID data)
{
    Worker *worker = static_cast<Worker *>(data);

    if (worker->getStop())
        return PROGRESS_CANCEL;
}

//Worker class members
bool getStop() { return cancel; }
std::atomic_bool cancel;

//cancel is set to false when backup begins
void Worker::startBackup()
{
    cancel = false;
}
//if user clicks cancel button cancel flag is set to false
void stop() { cancel = true; }

I've read about the possibility of deadlock with std::atomic variables. Is this something to worry about in my case?

std::atomic can never deadlock on trivial types, see Can atomic read operations lead to deadlocks?, technically no, because atomic types are almost all trivial, and the only non-trivial std::atomic<shared_ptr> shouldn't cause a deadlock, unless your code has a logical bug that causes this deadlock, like deadlocking on resources.

Should I be using std::atomic_flag instead?

why ? std::atomic_flag has a harder interface, it is designed as a building block for mutexes and spinlocks. for an atomic bool you should use std::atomic_bool. and before C++20 you couldn't even read or set std::atomic_flag without doing an RMW with test_and_set which is a slow operation, which made std::atomic_flag slower than std::atomic_bool when used as an atomic bool. (i guess the naming gives it away).

---

the only concern i have is over this line.

if (standbyResumeTimerDelayBool)
{
    standbyResumeTimerDelayBool = false;
    //do stuff
}

If more than one thread is executing this code concurrently then more than one thread may end up executing this block, instead you should be doing an exchange here

if (standbyResumeTimerDelayBool && standbyResumeTimerDelayBool.exchange(false))
{
   // only 1 thread can enter this block
   // other threads will get back false from exchange, and won't enter
}

note that this exchange is slower than a normal write, so if only 1 thread was allowed to execute this code anyway then your original code is fine.

as pointed by Peter Cordes, you can also use a different memory order to improve performance.

if (standbyResumeTimerDelayBool.load(std::memory_order::memory_order_relaxed) && 
standbyResumeTimerDelayBool.exchange(false, std::memory_order::memory_order_acquire))

Personally, I wouldn't recommend you play with memory ordering unless you really know what you are doing.

---

And the compiler didn't throw an error initializing the standbyResumeTimerDelayBool in as below in the class member list not sure if this is correct or not.

It is correct if passing the containing struct to the other threads is synchronized. that is if the struct existed before the thread was created or it was passed to the other threads through an atomic or a lock or some thread-safe container that will establish synchronization between threads.