1

I have seen c++ code like below in some production code. Assume x is aligned to uint64 and the code runs on X86 systems. My understanding is that read/write to aligned uint64 values are atomic on x86 systems. So what's the purpose of this additional mutex here?

While the mutex is held while calling GetX(), some other threads can grab the mutex and make changes to X right before/after GetX(). So I don't feel the additional mutex makes the code safer.

uint64 GetX()
{
   Mutex l(&mutex); (1)
   return x;
}
user17732522
  • 53,019
  • 2
  • 56
  • 105
yacc45
  • 121
  • 5
  • In general, writing code that has defined behavior is often considered preferable to writing code that relies on undefined behavior that _currently_ resolves the way you want it to. – Nathan Pierson Dec 19 '22 at 00:32
  • Related: [is \`int\` an atomic type?](https://stackoverflow.com/q/74836832) - yes for signals, but not usefully for concurrency in C++. See also [Who's afraid of a big bad optimizing compiler?](https://lwn.net/Articles/793253/) for more about the pitfalls of rolling your own shared variables with plain types, not even `volatile`. Also [Why is integer assignment on a naturally aligned variable atomic on x86?](https://stackoverflow.com/a/36685056). – Peter Cordes Dec 19 '22 at 01:46
  • Sort of a duplicate of [Which types on a 64-bit computer are naturally atomic in gnu C and gnu C++? -- meaning they have atomic reads, and atomic writes](https://stackoverflow.com/q/71866535), although this question takes a different angle. A Mutex is one way to get the necessary safety ([MCU programming - C++ O2 optimization breaks while loop](https://electronics.stackexchange.com/a/387478)), but an expensive way. – Peter Cordes Dec 19 '22 at 01:47

1 Answers1

5

It is not relevant whether or not the equivalent access on the ISA level is atomic. You can't violate the C++ memory model. The compiler will optimize under the assumption that it is not violated.

In particular only std::atomic specializations are atomic types in the C++ model. Any unsynchronized access to any other object type (with at least one a write) is a data race in the C++ model and therefore causes undefined behavior.

Therefore it is required here to lock the mutex before reading the variable if x's type is not a std::atomic specialization. However if so, this could be avoided by making x an std::atomic instead. Then it would not cause a data race, regardless of whether the equivalent access on the ISA level is atomic. (The compiler has to implement atomicity with locks inside std::atomic if the hardware doesn't support it directly.)

That being said, as you are hinting at, it doesn't seem that the function is particularly useful. As soon as it returns, the value of x might have already changed, so the caller can not assume it to still hold that value. However, without the locking the code will have undefined behavior, which is a worse condition than just providing a useless value.

user17732522
  • 53,019
  • 2
  • 56
  • 105
  • 1
    If you want it to be efficient like a plain load across various ISAs, you'd use `memory_order_relaxed`. (Or maybe release for stores and acquire for loads, if you still want the happens-before synchronization with previous critical sections like you'd get from acquiring a mutex. Hmm, mutex I guess uses different rules than a release-sequence for std::atomic, since an acquire load actually would only synchronize with the last release-store of `x`, not all previous holders of the lock.) – Peter Cordes Dec 19 '22 at 01:14