Reputation: 1309
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.
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
}
}
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.
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
}
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
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
Reputation: 22354
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
}
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
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";
}
}
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