2

In the GotW article #45, Herb states the following:

void String::AboutToModify(
  size_t n,
  bool   bMarkUnshareable /* = false */
) {
  if( data_->refs > 1 && data_->refs != Unshareable ) {
    /* ... etc. ... */

This if-condition is not thread-safe. For one thing, evaluating even "data_->refs > 1" may not be atomic; if so, it's possible that if thread 1 tries to evaluate "data_->refs > 1" while thread 2 is updating the value of refs, the value read from data_->refs might be anything -- 1, 2, or even something that's neither the original value nor the new value.

Additionally, he points out that data_->refs may be modified in between comparing with 1 and comparing with Unshareable.

Further down, we find a solution:

void String::AboutToModify(
  size_t n,
  bool   bMarkUnshareable /* = false */
) {
  int refs = IntAtomicGet( data_->refs );
  if( refs > 1 && refs != Unshareable ) {
    /* ... etc. ...*/

Now, I understand that the same refs is used for both comparisons, solving problem 2. But why the IntAtomicGet? I have turned up nothing in searches on the topic - all atomic operations focus on Read, Modify, Write operations, and here we just have a read. So can we just do...

int refs = data_->refs;

...which should probably just be one instruction in the end anyway?

mkobierski
  • 59
  • 3
  • 1
    **"...while thread 2 is updating the value of refs, the value read from data_->refs might be anything..."** i.e. even reading the value is not safe unless it's atomic. – Jonathan Potter Oct 20 '15 at 20:46

2 Answers2

1

Different platforms make different promises about atomicity of read/write operations. x86 for example guarantees that reading a double word (4 bytes) will be an atomic operation. However, you cannot assume that this will be true for any architecture and it will probably not be.

If you plan to port your code for different platforms, such assumptions could put you in troubles and lead to strange race conditions in your code. Therefore, it's better to protect yourself and make read/write operations explicitly atomic.

dbajgoric
  • 1,447
  • 11
  • 17
1

Reading from shared memory (data_->refs) while another thread writes to it is the definition of a data race.

What happens when we non-atomically read from data_->refs while another thread is trying to write to it at the same time?

Imagine that thread A is doing ++data_->refs (write) while thread B is doing int x = data_->refs (read). Imagine that thread B reads the first few bytes from data_->refs and that thread A finishes writing its value to data_->refs before thread B is done reading. Thread B then reads the rest of the bytes at data_->refs.

You will get neither the original value, nor the new value; you will get a completely different value! This scenario is just to illustrate what is meant by:

[...] the value read from data_->refs might be anything -- 1, 2, or even something that's neither the original value nor the new value.

The purpose of atomic operations is to ensure that an operation is indivisible: it is either observed as done or not done. Therefore, we use an atomic read operation to ensure that we get the value of data_->refs either before it is updated, or after (this depends on thread timings).

user2296177
  • 2,807
  • 1
  • 15
  • 26
  • I guess here we are implicitly assuming that IntAtomicGet is taking its parameter by reference, and not by value? Otherwise, the function would receive a copy of the (potentially corrupted) value, in other words we are still evaluating data_->refs in a non-atomic section? (No mention of the function signature is given in the article) – mkobierski Oct 22 '15 at 00:54
  • @mkobierski Yes, because we want to atomically obtain the value of that variable (at that memory location). Therefore, passing the value of `data->refs` wouldn't make sense. You can view `IntAtomicGet( data->refs )` as: go to the memory location of `data->refs` and atomically get a copy of the value and return it. – user2296177 Oct 22 '15 at 05:15