user1795160
user1795160

Reputation:

Is it wise to lock a mutex to just return a value?

class Foo {
public:
    // ...
    const int &getBar() const noexcept;

    void doSomethingWithBar(); // (2)

private:
    std::mutex barMutex;
    int bar = 7;
};

const int &Foo::getBar() const noexcept {
    std::lock_guard<std::mutex> lock(this->barMutex); // (1)
    return this->bar;
}

void Foo::doSomethingWithBar() {
    std::lock_guard<std::mutex> lock(this->barMutex); // necessary here
    this->bar++;
}

In terms of thread-safety, is line 1 necessary, considering that another thread might interfere and call the function in line 2 and thus change the value of bar?

Note that int might be any type here.

Upvotes: 1

Views: 1126

Answers (2)

OMGtechy
OMGtechy

Reputation: 8220

Seeing as you're returning a reference, locking is entirely useless for you in this scenario. You may want a lock when you use the reference that is returned though.

However, if you were returning a value it will have more of an effect, take a look at this for an example of a torn read.

Upvotes: 4

Christophe
Christophe

Reputation: 73376

Line 1 has not the effect that you expect: apparently, you try to lock the object and keep it locked while the caller works on the reference returned.

But the lock_gard you create is a local object of getBar() and gets destroyed as soon as you return, thus unlocking the lock that you just acquired.

Several alternatives, for example:

  • You may change your function to return the value of bar, but taking into consideration that this value is a snapshot that can be obsolete when you'll use it.

  • You may also change your function to get and process bar in one shot (for example by providing a function as parameter).

  • You could also manage the lock as class member and provide a function releaseBar() to unlock the mutex as soon as it isn't needed anymore. This is however is a more dangerous, approach, because the caller may forget to release the lock.

Upvotes: 0

Related Questions