4

What is the best approach to achieve thread-safety for rather simple operations?

Consider a pair of functions:

void setVal(int val)
{
    this->_val = val;
}

int getVal() {
     return this->_val;
}

Since even assignments of primitive types aren't guaranteed to be atomic, should I modify every getter and setter in the program in the following way to be thread-safe?

void setVal(int val)
{
    this->_mutex.lock();
    this->_val = val;
    this->_mutex.unlock();
}

int getVal() {
     this->_mutex.lock();
     int result = this->_val;
     this->_mutex.unlock();
     return result;
}
mip
  • 8,355
  • 6
  • 53
  • 72
  • 5
    Note that in general, for exception safety and code clarity, it's better to use a `scoped_lock` of some kind than to manually lock and unlock the mutex. – James McNellis Sep 01 '10 at 13:42
  • @James: I am quite noobish with practical aspects... is it necessary to copy to a temporary and return it rather than directly returning here ? (which would necessitates a scoped lock) I would guess most compiler implementations would apply NRVO and thus it would not matter, but I find the `result` spurious. – Matthieu M. Sep 01 '10 at 13:55
  • @Matthieu: If you use a `scoped_lock` you can just `return this->_val`. If you use manual locking and unlocking, then you don't have much choice (unless you like deadlocks :-P). – James McNellis Sep 01 '10 at 14:15
  • Matthieu M.: Scoped lock isn't necessary, you can place `mutex.unlock()` right after `mutex.lock()` to get the same result. But another question is if assignment during return is atomic... I have used temporary to put the result on the stack so that another thread won't harm the value during return. – mip Sep 01 '10 at 14:22

4 Answers4

6

Are you using _val in multiple threads? If not, then no, you don't need to synchronize access to it.

If it is used from multiple threads, then yes, you need to synchronize access, either using a mutex or by using an atomic type (like std::atomic<T> in C++0x, though other threading libraries have nonstandard atomic types as well).

James McNellis
  • 348,265
  • 75
  • 913
  • 977
  • I wonder if it's really necessary if another thread doesn't matter what's returned by `getVal()`. For example if it only prints the result to the user in real time. – mip Sep 01 '10 at 13:51
  • @Piotr @doc: In that case, you still need to synchronize access to the object in order to prevent torn reads. – James McNellis Jan 14 '11 at 20:40
2

Mutexes are very costly, as they are able to be shared across processes. If the state that you're limiting access to is only to be constrained to threads within your current process then go for something much less heavy, such as a Critical Section or Semaphore.

OJ.
  • 28,944
  • 5
  • 56
  • 71
2

On 32-bit x86 platforms, reads and writes of 32-bit values aligned on 4-byte boundary are atomic. On 64-bit platforms you can also rely on 64-bit loads and stores of 8-byte aligned values to be atomic as well. SPARC and POWER CPUs also work like that.

C++ doesn't make any guarantees like that, but in practice no compiler is going to mess with it, since every non-trivial multi-threaded program relies on this behaviour.

Igor Nazarenko
  • 2,184
  • 13
  • 10
-3
int getVal() { 
     this->_mutex.lock(); 
     int result = this->_val; 
     this->_mutex.unlock(); 
     return result; 
}

What exactly are you hoping to accomplish with this? Sure, you've stopped this->_val from changing before you saved into result but it still may change before result is returned, -- or between the return and the assignment to whatever you assigned it -- or a microsecond later. Regardless of what you do, you are just going to get a snapshot of a moving target. Deal with it.

void setVal(int val)          
{          
    this->_mutex.lock();          
    this->_val = val;          
    this->_mutex.unlock();          
} 

Similarly, what is this buying you? If you call setVal(-5) and setVal(17) from separate threads at the same time, what value should be there after both complete? You've gone to some trouble to make sure that the first to start is also the first to finish, but how is that help to get the "right" value set?

James Curran
  • 101,701
  • 37
  • 181
  • 258
  • 3
    It's not about the value in this->_val. It's to protect _val from being rubish, because an assignment isn't atomic. I have used temporary in return to put the result on the stack so that another thread won't harm the value during return. Because I don't know if return's push is atomic(?). Since assignment isn't atomic I might get potentially a rubish in return. When the `result` variable is on the stack it won't be modified by another thread during return. – mip Sep 01 '10 at 14:48
  • In other words, without locks getVal() may return `-123232` between `setVal(-5)` and `setVal(17)` calls. – mip Sep 01 '10 at 14:50