c++ 11 std::atomic_flag, am I using this correctly?

Tom picture Tom · Apr 4, 2013 · Viewed 8k times · Source

I have a simple boolean value I need to test and set in a thread-safe manner. If one thread is already working, I want the second thread to exit. If I understand std::atomic_flag correctly, this should work fine. However, I'm not confident I understand std::atomic_flag correctly :) I can't seem to find many simple examples online, save for this spinlock example:

// myclass.cpp
#using <atomic>

namespace  // anonymous namespace
{
    std::atomic_flag _my_flag = ATOMIC_FLAG_INIT;
}  // ns

myclass::do_something()
{
    if ( !::_my_flag.test_and_set() ) )
    {
        // do my stuff here; handle errors and clear flag when done
        try
        {
            // do my stuff here
        }
        catch ( ... )
        {
            // handle exception
        }

        ::_my_flag.clear();  // clear my flag, we're done doing stuff
    }
    // else, we're already doing something in another thread, let's exit
}  // do_something

Update: updated code based on suggestions below, forming a decent template for proper use of std::atomic_flag. Thanks all!

Answer

bames53 picture bames53 · Apr 4, 2013

atomic_flag is a really low level construct which isn't meant to be widely used. That said, I believe you're usage works as you intend, except possibly clearing the flag in exceptional cases. If an exception other than one matched by std::exception occurs the flag does not get cleared.

Typically RAII should be used for this sort of thing. 'R' normally stands for 'resource' but I like Jon Kalb's usage of 'responsibility' instead. After setting the flag you have a responsibility to clear the flag when done, so you should use RAII to ensure that responsibility is carried out. If all of the things you need to do in exceptional cases can be done this way then the try/catch pair disappears.

if ( !std::atomic_flag_test_and_set( &::_my_flag ) )
{
    flag_clearer x(&::_my_flag);

    // do my stuff here
}

But you don't need to write a flag_clearer type yourself. Instead you can simply use higher level constructs such as a mutex and lock_guard:

namespace
{
    std::mutex my_flag;
}

myclass::do_something()
{
    if ( my_flag.try_lock() )
    {
        std::lock_guard<std::mutex> x(my_flag, std::adopt_lock);
        // do my stuff here
    }
    // else, we're already doing something in another thread, let's exit
}