Including std::lock_guard in extra scope

Question

Does is make sense to do something like putting a std::lock_guard in an extra scope so that the locking period is as short as possible?

Pseudo code:

// all used variables beside the lock_guard are created and initialized somewhere else
...// do something

{ // open new scope
    std::lock_guard<std::mutex> lock(mut);
    shared_var = newValue;  
} // close the scope

... // do some other stuff (that might take longer)

Are there more advantages besides having a short lock duration?

What might be negative side effects?

Answer 1

Yes, it certainly makes sense to limit the scope of lock guards to be as short as possible, but not shorter.

The longer you hold a lock, the more likely it is that a thread will block waiting for that lock, which impacts performance as is thus usually considered a bad thing.

However, you must make sure that the program is still correct and that the lock is held at all times when it must be, i.e. when the shared resource protected by the lock is accessed or modified.

There may be one more point to consider (I do not have enough practical experience here to speak with certainty). Locking/releasing a mutex can potentially be an operation with nontrivial performance costs itself. Therefore, it may turn out that keeping a lock for a slightly longer period instead of unlocking & re-locking it several times in the course of one operation can actually improve overall performace. This is something which profiling could show you.

Answer 2

Using an extra scope specifically to limit the lifetime of an std::lock_guard object is indeed good practice. As the other answers point out, locking your mutex for the shortest period of time will reduce the chances that another thread will block on the mutex.

I see one more point that was not mentioned in the other answers: transactional operations. Let's use the classical example of a money transfer between two bank accounts. For your banking program to be correct, the modification of the two bank account's balance must be done without unlocking the mutex in between. Otherwise, it would be possible for another thread to lock the mutex while the program is in a weird state where only one of the accounts was credited/debited while the other account's balance was untouched!

With this in mind, it is not enough to ensure that the mutex is locked when each shared resource is modified. Sometimes, you must keep the mutex locked for a period of time spanning the modification of all the shared resources that form a transaction.

EDIT:

If for some reason keeping the mutex locked for the whole duration of the transaction is not acceptable, you can use the following algorithm:
1. Lock mutex, read input data, unlock mutex.
2. Perform all needed computations, save results locally.
3. Lock mutex, check that input data has not changed, perform the transaction with readily available results, unlock the mutex.

If the input data has changed during the execution of step 2, throw away the results and start over with the fresh input data.

Answer 3

Yes, it makes sense.

There are no other advantages, and there are no side-effects (it is a good way to write it).

An even better way, is to extract it into a private member function (if you have an operation that is synchronized this way, you might as well give the operation its own name):

{
    // all used variables beside the lock_guard are created and initialized somewhere else
    ...// do something

    set_var(new_value);

    ... // do some other stuff (that might take longer)
}

void your_class::set_value(int new_value)
{
    std::lock_guard<std::mutex> lock(mut);
    shared_var = new_value;
}

Answer 4

I don't see the reason to do it. If you do something so simple as "set one variable" - use atomic<> and you don't need mutex and lock at all. If you do something complicated - extract this code into new function and use lock in its first line.

Answer 5

There might be a disadvantage: you cannot protect initializations this way. For example:

{
    std::lock_guard<std::mutex> lock(mut);
    Some_resource var{shared_var};
} // oops! var is lost

you have to use assignment like this:

Some_resource var;
{
    std::lock_guard<std::mutex> lock(mut);
    var = shared_Var;
}

which may be suboptimal for some types, for which default initialization followed by assignment is less efficient than directly initializing. Moreover, in some situations, you cannot change the variable after initialization. (e.g. const variables)

---

user32434999 pointed out this solution:

// use an immediately-invoked temporary lambda
Some_resource var {
    [&] {
        std::lock_guard<std::mutex> lock(mut);
        return shared_var;
    } () // parentheses for invoke
};

This way, you can protect the retrieval process, but the initialization itself is still not guarded.