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

Question

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 1

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
}

Answer 2

Yes, that will skip the code inside the if block if some other thread has already set the flag and nobody has cleared it. If no other code messes with the flag, that means that some thread is currently executing that block.

Atomic flags are pretty low level, though; consider using atomic_bool instead. Also, since this is C++, you can use member functions for the set and the clear.

EDIT:

Nope, atomic_bool doesn't easily do what you want. Stick with atomic_flag...