C++ shared mutex with return by reference

Question

I have a question that is related to, but not answered by, this question:

Example for boost shared_mutex (multiple reads/one write)?

I understand how the exclusive locks work when the operations are scoped within the member function. My question is, what about return-by-reference functions? Consider the following (psuedo-code):

class A {
    shared_mutex _mutex;
    std::string _name;

public:
    const std::string& name() const {shared_lock(_mutex); return _name;}
}

Then assume I do something like this in code:

A obj;
if (obj.name().length() >0) { ... };

My gut tells me this may not be thread safe, since the mutex will already have gone out of scope by the time the length() function is called, but I don't know.

I guess I'm also asking in the big picture, if I'm trying to make the object thread-safe, should I avoid returning by reference altogether? Wouldn't it enable someone to do something like this:

A obj;
std::string& s = obj.name();
(at this point lock is out of scope)
s = "foo";  // does this change the original object's member since it is a reference?

Answer 1

It is not threadsafe as the mutex would be released after name() has returned and another thread could begin modification of _name before or during the call to std::string::length() or after and the state of _name would have changed before entering the {...} true branch of the if.

To make the object threadsafe, ensure that all accesses to the _name occur when the mutex is locked. This means ensuring that a reference to _name is not returned to a caller or is not passed to a callback provided to (an imaginary member function not posted in the original code) a member function of A as this callback could cache the address of _name for future, unsynchronized, use.

Instead, return, or pass, by value (and create a copy of _name when the mutex is locked):

// Must be mutable to be modifiable
// in a const member function.
mutable shared_mutex _mutex;

std::string name() const
{
    // Must not create a temporary shared_lock,
    // otherwise the lock will be released immediately.
    // So give the shared_lock a name.
    shared_lock lk(_mutex);
    return _name;
}

Answer 2

Just change this function delaration:

const std::string& name() const ...

To this.

std::string name() const ...

You'll then be returning a copy of the string. (And since it's a copy, it doesn't need to be const).

That code will be thread-safe, and any compiler optimizations would preserve that safety.