Initialising an anonymous mutex-lock-holding class instance in the LHS of a comma operator

Question

Suppose I have code something like this:

#include "boost/thread/mutex.hpp"

using boost::mutex;
typedef mutex::scoped_lock lock;

mutex mut1, mut2;

void Func() {
 // ...
}

void test_raiicomma_1() {
 lock mut1_lock(mut1);
 Func();
}

void test_raiicomma_2() {
 (lock(mut1)), Func();
}

void test_raiicomma_3() {
 (lock(mut1)), (lock(mut2)), Func(); // Warning!
}

int main()
{
 test_raiicomma_1();
 test_raiicomma_2();
 test_raiicomma_3();
 return 0;
}

If the function test_raiicomma_1() were called from multiple threads, it locks a mutex to prevent any other thread also calling Func() at the same time. The mutex is locked when the variable mut1_lock is constructed, and released when it goes out of scope and is destructed.

This works perfectly fine, but as a matter of style, needing to give a name to the temporary object holding the lock irked me. The function test_raiicomma_2() attempts to avoid this, by inialising the lock object and calling the function Func() within one expression.

Is it correct that the temporary object destructor will not be called until the end of the expression, after Func() has returned? (If so, do you think it's ever worthwhile to use this idiom, or is it always clearer to declare the lock in a separate statement?)

If the function test_raiicomma_3() needs to lock two mutexes, is it correct that the mutexes will be locked in order before calling Func(), and released afterwards, but may unfortunately be released in either order?

Answer 1

While not having to give meaningful names frees you of a burden, it will add the task to find out what that code is supposed to do to the burdens of readers of your code -- atop the task to decide whether test_raiicomma_3 actually works the way its writer seems to have supposed.

Given that a piece of code is written once, but is read 10, 100, or 1000 times, is it really so hard to write all the locks?

Answer 2

Mutexes will be created in left-to-right order and released in the end of full expression. You could write:

 // parentheses tells that it is full expression and set order explicitly
 ( ( lock(mut1), lock(mut2) ), Func() );

Mutexes will be destroyed in proper order according to C++ Standard 5.18/1:

The comma operator groups left-to-right.
expression:
assignment-expression
expression , assignment-expression

A pair of expressions separated by a comma is evaluated left-to-right and the value of the left expression is discarded. The lvalue-to-rvalue (4.1), array-to-pointer (4.2), and function-to-pointer (4.3) standard conver- sions are not applied to the left expression. All side effects (1.9) of the left expression, except for the destruction of temporaries (12.2), are performed before the evaluation of the right expression. The type and value of the result are the type and value of the right operand; the result is an lvalue if its right operand is.

Answer 3

Is it correct that the temporary object destructor will not be called until the end of the expression, after Func() has returned?

It is guaranteed that both constructor and destructor are called, as they have side effects, and that destruction will only happen at the end of a full expression.

I believe it should work

If the function test_raiicomma_3() needs to lock two mutexes, is it correct that the mutexes will be locked in order before calling Func(), and released afterwards, but may unfortunately be released in either order?

Comma is always evaluated left to right, and automatic variables within a scope are always destroyed in reverse order of creation, so I think it's even guaranteed they are released in the (correct) order too

As litb points out in the comments, you need braces or your expression will be parsed as a declaration.

(If so, do you think it's ever worthwhile to use this idiom, or is it always clearer to declare the lock in a separate statement?)

I don't think so no, to confusing for very, very little gain... I always use very explicit locks, and very explicit scopes (often extra {} within a block), decent threadsafe code is hard enough without 'special' code, and warrants very clear code in my opinion.

YMMW of course :)