Locking scheme a hack

Question

How's this look:

class HugeHack
{
    HugeHack() : m_flag( false ) { }

    void Logout( )
    {
        boost::lock_guard< boost::mutex > lock( m_lock );
        m_flag = true;

        // do stuff that in a perfect world would be atomic with library call and onLogout

        // call a library function that waits for a thread to finish, that thread calls my onLogout() function before it dies

        m_flag = false;
    }


    void onLogout()
    {
        boost::unique_lock< boost::mutex > l( m_lock, boost::defer_lock_t );
        if( ! m_flag )
            l.lock();

        // do stuff

    }



    boost::mutex m_lock;
    bool m_flag;
};

The flag is true ONLY while Logout is running, Logout legitimately waits for a thread to die that calls onLogout, so unless someone else can call onLogout... (can't be totally sure not my library I'm using - QuickFix)

I'm not sure I'm using the unique lock correctly there, if not, the goal is only to conditionally lock the lock (while maintaining scoped locking semantics).

Answer 1

The problem is that if you read m_flag without locking the mutex, you might see m_flag be false even if in reality it is true, and Logout is in the middle of operation. Locking the mutex issues a memory fence, which is essential to ensure proper memory visibility. And BTW stijn is right - if this is all you are after, you can ditch m_flag and use try_lock instead, like this:

boost::mutex::scoped_try_lock l( m_lock );
if ( l )
    // lock succeeded

Answer 2

It sounds like what you want to do is release the lock in Logout at the point you are waiting for the thread to terminate (and call onLogout). So do that instead, then unconditionally take the lock in onLogout.