Is return atomic and should I use temporary in getter to be thread safe?

Question

Is it necessary to use a temporary here to be thread-safe?

 int getVal() {
       this->_mutex.lock();
       int result = this->_val;
       this->_mutex.unlock();
       return result;
 }

I'll give you disassembly of simple RAII test function

int test()
{
    RAIITest raii; //let's say it's a scoped lock
    return 3;
}


 {
     0x004013ce <_Z4testv>:    push  %ebp
     0x004013cf <_Z4testv+1>:  mov   %esp,%ebp
     0x004013d1 <_Z4testv+3>:  sub   $0x28,%esp
     return 3;
     0x004013d4 <_Z4testv+6>:  lea   -0x18(%ebp),%eax
     0x004013d7 <_Z4testv+9>:  mov   %eax,(%esp)
     0x004013da <_Z4testv+12>: call  0x4167a0 <_ZN8RAIITestD1Ev>  //here destructor is called
     0x004013df <_Z4testv+17>: mov   $0x3,%eax //here result is pushed onto the stack
 }
 0x004013e4 <_Z4testv+22>: leave 
 0x004013e5 <_Z4testv+23>: ret   

the compiler is gcc/g++ 3.4.5

Answer 1

If access to this->_val is synchronized by this->_mutex, then you don't have a choice the way the code is written currently. You need to read this->_val before you unlock the mutex and you have to unlock the mutex before you return. The result variable is necessary to get this order of actions.

If you use a lock_guard (or scoped_lock in Boost), then you don't need to use the temporary because the lock on the mutex will be released when the function returns. For example, using the C++0x threads library:

int getVal() {
    std::lock_guard<std::mutex> lock(_mutex);
    return this->_val;
}   // lock is released by lock_guard destructor 

Answer 2

Yes if you use explicit lock()/unlock(). No if you construct a lock object on the stack and have its destructor do the unlock.

Answer 3

No -- the compiler creates a temporary for the return value automatically. You don't normally need to protect a read with a mutex either, so even though its multithreaded, just return _val; should be sufficient.

As an aside, I'd get rid of the leading underscore though -- the rules about what variable names you can and cannot use when they start with an underscore are sufficiently complex that it's better to just avoid them entirely.

Answer 4

You can do this cleanly is your mutex is encapsulated in a scoped_lock that unlocks on destruction:

 int getVal() {
       scoped_lock lockit(_mutex);
       return _val;
 }

And yes, you do need to hold the lock until it's returned.