CODEWITHSUNDEEP
c++c++11scopelockingconditional-statements
Peter VARGA
Peter VARGA

Reputation: 5186

Conditional use of std::lock_guard

I have a function where the statement foo should be executed under lock_guard but only when a pointer to a mutex object has been provided to the function as a parameter. Otherwise foo does not have to be protected by lock_guard.

I cannot use the lock_guard within an if because the lock will be released immediately when the if block ends.

so, this code is nonsense:

bar( std::mutex * optionalMutex = nullptr )
{
   ...
   if ( nullptr != optionalMutex ) {
      std::lock_guard<std::mutex> lockScope( *optionalMutex );
   }          <- Here the lock ends

   foo...     <- foo is not protected when optionalMutex was provided
}

I tried something like this:

bar( std::mutex * optionalMutex = nullptr )
{
   ...
   nullptr == optionalMutex ? 0 : std::lock_guard<std::mutex> lockScope( *optionalMutex );

   // this scope should be protected by lock_guard when optionalMutex was provided

   foo...
}

More or less, the only one possible solution for me is to repeat foo:

bar( std::mutex * optionalMutex = nullptr )
{
   ...
   if ( nullptr != optionalMutex ) {
      std::lock_guard<std::mutex> lockScope( *optionalMutex );
      foo...
   }  else {
      foo...
   }
}

The compiler gcc 4.9.3 does not compile the 2nd example and complains: error: expected primary-expression before 'lockScope'. Update: Superlokkus explained in his answer why.

But I do want to avoid any code duplicates and therefore also the duplicate foo.

My question:

Is there an elegant way how to implement this problem and not to use duplicate foo. I know, I could use a lambda function to group foo but I am curious if there is an another solution.

Upvotes: 22

Views: 6969

Answers (6)

x64
x64

Reputation: 11

It's fairly trivial to implement your own version of lock_guard which takes a pointer to a mutex rather than a reference.

template <typename MutexT>
class conditional_lock_guard
{
private:

    MutexT* const m_mtx;

public:

    explicit conditional_lock_guard(MutexT* mtx) :
        m_mtx{ mtx }
    {
        if (m_mtx != nullptr)
        {
            m_mtx->lock();
        }
    }

    ~conditional_lock_guard() noexcept
    {
        if (m_mtx != nullptr)
        {
            m_mtx->unlock();
        }
    }

    conditional_lock_guard(const conditional_lock_guard&) = delete;
    conditional_lock_guard& operator=(const conditional_lock_guard&) = delete;

    bool owns_lock() const noexcept
    {
        return m_mtx != nullptr;
    }

    explicit operator bool() const noexcept
    {
        return owns_lock();
    }

    MutexT* mutex() const noexcept
    {
        return m_mtx;
    }
};

Upvotes: 1

Superlokkus
Superlokkus

Reputation: 5049

How about this one? 

void bar(std::mutex * optionalMutex = nullptr)
{
        auto lockScope = (optionalMutex == nullptr) ? 
                           std::unique_lock<std::mutex>() 
                         : std::unique_lock<std::mutex>(*optionalMutex);
}

Explanation: Your compiler had trouble with your prior statement because, you can not suddenly change the type of the ternary ? expression; i.e. the literal 0 is not a std::lock_guard and vice versa. So I changed the two branches to the same type, here std::unique_lock<std::mutex> because lock_guard isn't designed be used without a valid mutex. But still prefer std::lock_guard over std::unique_lock in the simpler cases, because it will make your code more readable.

Also your statement wasn't viable for the compiler, i.e. even syntactical correct, because the variable lockScope would only have existed in one branch.

Upvotes: 19

1stCLord
1stCLord

Reputation: 880

It's a minor gripe but you can avoid passing the the raw pointer by letting the caller pass a std::unique_lock instead:

bar( std::unique_lock<std::mutex> lockScope )
{
    if(lockScope.mutex())
    {
        lockScope.lock();
        //do things
    }
}

This seems like a more clear expression of the interface and reduces potential for abuse.

Upvotes: 1

Adrien Hamelin
Adrien Hamelin

Reputation: 395

The answer from Superlockus is good enough, but I am wondering why you did not simply write it like that:

bar( std::mutex * optionalMutex = nullptr )
{
   if (optionalMutex)
      optionalMutex->lock():

   foo...

   if (optionalMutex)
      optionalMutex->unlock():
}

lock_guard and unique_lock are convenient but not the only way.

Upvotes: 0

Pete Becker
Pete Becker

Reputation: 76315

What you really have is two functions, one that locks, and one that doesn't. The first one can call the second:

void bar() {
    // whatever
}

void bar(std::mutex* mtx) {
    std::lock_guard<std::mutex> lockScope(*mtx);
    bar();
}

Upvotes: 14

Peter VARGA
Peter VARGA

Reputation: 5186

I have only this solution. Using a dummy mutex object:

The code is:

bar( std::mutex * optionalMutex = nullptr )
{
   ...
   std::mutex dummyMutex;
   std::lock_guard<std::mutex> lockScope( optionalMutex ? *optionalMutex, dummyMutex );

   foo...     <- NOW foo is protected when optionalMutex was provided
}

Upvotes: 1

Related Questions