Stefan
Stefan

Reputation: 1309

Difference between if statement initializers taking a lock

I need to get a lock only during evaluation of the if statement condition.

The lock must be released when running the code inside the if or else statements.

I found a piece of code and I don't know if it works. I'm rather new to C++, so please forgive me if this is an obvious question.

Example 1

I found I cannot use this if statement initializer:

if (std::lock_guard lock(mutex); condition) {
  // do something
}

as it will be equivalent to the following, which will hold the lock also during execution of the if or else blocks:

{
    std::lock_guard lock(mutex);
    if (condition) {
      // do something
    }
}

Example 2

I also found something like this:

if (std::lock_guard(mutex); condition) {
  // do something
}

This will definitely not work. It will create the lock and then directly destroy it (and free the lock) as the lock_guard is not stored in a variable.

So in fact, this example will not hold the lock when condition is evaluated, and also not hold the lock during execution of the if or else blocks.

Example 3

However, I also found this code:

if (std::lock_guard{mutex}, condition) {
    // do something
}

Note that instead of a ; there is a , and also the brackets differ, {} instead of ().

This code compiles and run.

I verified that the lock is not held during execution of the if or else blocks, because the following does not dead-lock:

if (std::lock_guard{mutex}, condition) {
    std::lock_guard lock(mutex);
    // do something
}

Questions

Does the code in example 3 work as intended? That is, does it hold a lock during evaluation of condition? I cannot find out if this line actually works. I'm afraid it may be similar to example 2.

Does this type of if initializer have a name so I can read up on it?

Upvotes: 6

Views: 200

Answers (4)

KevinZ
KevinZ

Reputation: 3319

Compute the condition outside of the if statement:

bool const a_good_condition_name = [&mutex]()
{
    auto const lock = std::lock_guard{mutex};
    // ...
    return .....;   
} ();

if ( a_good_condition_name )
{
}
else
{
}

Upvotes: 1

許恩嘉
許恩嘉

Reputation: 504

Although this is the generally recommended method, lock_guard is not the only way to obtain a lock. And there is no need to cram all the code into one line.
The advantage of lock_guard is that you will never forget to unlock it and will correctly unlock when an exception is thrown.
If the above two points are not required, we can achieve the goal in other simple ways:

Mutex.lock();
if (condition()) {
    Mutex.unlock();
    std::cout << "Doing something.";
}
else{
    Mutex.unlock();// Don't forget to unlock every branch!
}

When multiple branches exist, save results locally:

Mutex.lock();
auto result = condition();
Mutex.unlock();
if (result==A) {
    std::cout << "Doing something.";
}
else if (result==B){
    std::cout << "Doing another thing.";
}
else {
    std::cout << "Doing other thing.";
}

If condition() may throw an exception, you still need to use lock_guard to ensure exception safety

bool result;
{
    std::lock_guard lock(Mutex);
    result = condition();
}
if (result) {
    std::cout << "Doing something.";
}

Upvotes: 0

Yksisarvinen
Yksisarvinen

Reputation: 22354

Yes, example 3 works as you want, but it's highly obscure

What you found is not any special initialisation, it's the comma operator. Comma operator evaluates left hand side operand, discards it (although the object is not destroyed until end of expression), then evaluates right hand side and returns it. So std::lock_guard{mutex}, condition creates temporary lock_guard, ignores it, evaluates condition and destroys temporary lock_guard.

However, comma operator is a really obscure feature and it's hard to understand. It's also hard to notice, since comma is usually used in other contexts. I'd strongly recommend to simply extract a function

bool condition() {
    std::lock_guard lock {mutex};
    return /*evaluate the condition here*/;
}

and call it in if:

if (condition()) {
    // mutex isn't held here
} else {
    // nor here
}

Important note

I verified that the lock is not held during execution of the if or else blocks because the following does not dead-lock:

if (std::lock_guard{mutex}, condition) {
    std::lock_guard lock(mutex);
// do something 
}

If mutex was locked here, you have Undefined Behaviour (because the thread that holds a lock on mutex attempts to lock it again), so this doesn't test anything.

Upvotes: 6

463035818_is_not_an_ai
463035818_is_not_an_ai

Reputation: 123114

First of all, attempting to lock an already locked mutex is undefined. It is not guaranteed to deadlock as you assume.

A better way to test your assumptions is to use a dummy RAII type:

#include <string>
#include <array>
#include <vector>

struct my_raii {
    my_raii() { std::cout << "construct\n"; }
    ~my_raii() { std::cout << "destroy\n";}
};

bool condition() {
    std::cout << "condition\n";
    return true;
}

int main() {
    if (my_raii{}, condition()) {
        std::cout << "true branch\n";
    } else {
        std::cout << "false branch\n";
    }

}

Output:

construct
condition
destroy
true branch

This works as expected, because the comma operator evaluates first the left hand operand then the right operand and its value is that of the right (left is discarded). And because the temporaries lifetime is until the end of the expression.


However, the mere fact that you have to ask signals that the code is not clear as it could be. I would suggest to refactor to (staying with same example as above):

bool condition() {
    my_raii lock{};
    std::cout << "condition\n";
    return true;
}

int main() {
    if (condition()) {
        std::cout << "true branch\n";
    } else {
        std::cout << "false branch\n";
    }   
}

In addition to being unclear code, the comma operator can be overloaded to make the above silently fail or do something unexpected. I would not rely on the comma operator unless there is no alternative.

Upvotes: 6

Related Questions