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!

@BrendanLong -- I haven't fully tested it yet; just want to make sure I get the concept first before I venture forth...the 'do my stuff' part will be fairly extensive in my case. Though I recognize I could easily revise this code to use a mutex if needed... — Tom, Apr 04 '13 at 19:28
@K-ballo -- I prefer using anonymous namespaces in my cpp files as opposed to static/private methods ( http://stackoverflow.com/q/154469/882436 ), though I see how this might be confusing in the context of my question. So, basically, I put it there out of habit :) — Tom, Apr 04 '13 at 20:14

score 8 · Accepted Answer · answered Apr 04 '13 at 20:05

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
}

Thanks bames. The RAII point is well-taken. And you're right, I messed up on my exception handling. If I still wanted to avoid using a flag_clearer for simplicity reasons, could I replace `catch ( std::exception& e )` with `catch ( ... )` and call it good? BTW I'm probably going to replace my code with what you suggested, if only to make it less esoteric :) I don't need to squeeze every last drop of performance out of this section in my app. Thank you — Tom, Apr 04 '13 at 20:45
@Tom Yes, catching `...` would ensure that the clean-up code runs. — bames53, Apr 04 '13 at 21:08
Thanks for the follow up, I will update my post with the revised code...and then replace my real code with your mutex example :) Thanks again! — Tom, Apr 04 '13 at 21:12

score 1 · Answer 2 · answered Apr 04 '13 at 19:59

1

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...

answered Apr 04 '13 at 19:59

Pete Becker

74,985
8
76
165

Pete, thanks for the confirmation. I will use the member functions instead, I think those are more readable. Thanks again! – Tom Apr 04 '13 at 20:06

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

2 Answers2