Reputation:
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
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
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