0

Is the following function thread-safe (in C++) or do I have to add a mutex?

int example() {
    return g_maxValue++;
}

where int g_maxValue is some global integer. If yes, does the same hold true for all integer types such as std::uint64_t?

digito_evo
  • 3,216
  • 2
  • 14
  • 42
Yeladia
  • 131
  • 4
  • 12
    No, it's not thread-safe. – Ted Lyngmo Aug 17 '22 at 10:19
  • 2
    Oh boy, global variables and threads.. you are asking for trouble are you not? Better have a an interface with getters (and threadsafe implementation) and pass that around to classes that need it. (dependency injection) – Pepijn Kramer Aug 17 '22 at 10:28
  • You could have at least two requirements for thread safety: The returned number should be unique over all calls and the variable should (in the end, possibly with delay) contain the added number of all function calls over all threads. For both requirements the code is not thread-safe. – Sebastian Aug 17 '22 at 10:28
  • Mutex/critical section or atomic variable (or semaphore). Depends on, whether the variable is changed only on this or multiple locations in the program, you only increment, read or also decrement or more complicated operations. Does this line stand for itself or is it a part of a more complicated transaction. – Sebastian Aug 17 '22 at 10:31
  • That is essentially what I am doing right now. I was just wondering if I could get away with replacing the class with a simple function. @PepijnKramer – Yeladia Aug 17 '22 at 10:33
  • 1
    No keep the class, globals are not maintainable and problematic for unit testing. The most "convenient" code (least amount of lines) is not always the best code. – Pepijn Kramer Aug 17 '22 at 10:35
  • See this example, https://en.cppreference.com/w/cpp/atomic/atomic/fetch_add You can write safe code, which is nearly as short as the code in your question. It is available for uint64_t. – Sebastian Aug 17 '22 at 10:37
  • 1
    This is why [`std::atomic`](https://en.cppreference.com/w/cpp/atomic/atomic) was added to the library. With `std::atomic j;` you can write `j++` and `++j` without causing data races. – Pete Becker Aug 17 '22 at 12:35
  • 1
    Does this answer your question? [Are incrementers / decrementers (var++, var--) etc thread safe?](https://stackoverflow.com/questions/443500/are-incrementers-decrementers-var-var-etc-thread-safe) – Luis Moita Aug 17 '22 at 14:40

2 Answers2

2

Thread safety is guaranteed only for atomic variables (std::atomic).

From C++ standard:

The execution of a program contains a data race if it contains two conflicting actions in different threads, at least one of which is not atomic, and neither happens before the other. Any such data race results in undefined behavior.

The compiler doesn't have to consider thread safety for non-atomic variables, so it is allowed to translate ++ to multiple operations (pseudo code):

  1. Read g_maxValue to a register
  2. Increment the value in the register
  3. Store the value to g_maxValue
VLL
  • 9,634
  • 1
  • 29
  • 54
0

No, it is not. The increment operation is internally computed as three different operations:

load operator

add 1

write operator

It is possible for one write operation to overwrite another if they are executed in parallel .

Luis Moita
  • 47
  • 1
  • 8
  • This is not always the case. There is an operation for incrementing a value at a memory location, which I imagine the compiler would use in most cases. https://docs.oracle.com/cd/E19455-01/806-3773/instructionset-24/index.html – VLL Aug 17 '22 at 14:20
  • @VLL I think the used processor architecture was not mentioned. But your comment is true, if there is at least one. – Sebastian Aug 17 '22 at 14:23
  • @VLL according to Rob Kennedy on this post https://stackoverflow.com/a/443530/15096546 this function still do the load and store operations, but in your link there is no mention of it. Do you know where I can find more info on this subject? – Luis Moita Aug 17 '22 at 14:44
  • found evidence that the INC operation just calls an ADD operation with fixed value of 1. source: https://www.intel.com/content/www/us/en/architecture-and-technology/64-ia-32-architectures-software-developer-vol-2a-manual.html So I belive that the INC operation is functionally the same as the instructions described in my answer. Love to learn those things :) – Luis Moita Aug 17 '22 at 15:03
  • 1
    You have to distinguish, what the compiler and optimizer produce, what the CPU reorders and how the memory architecture propagates changes. Especially for the first and third point you can take care with relevant modifiers and instructions or constructs, which combine both. So even with a single inc there is no guarantee. Some memory architectures can do simple calculations within the hierarchy (instead of the CPU), some others need bus locks or cache flushes or memory fences. Sometimes you need or get operating system support to prevent other threads or interrupts from interrupting. – Sebastian Aug 17 '22 at 15:05
  • 1
    Newer CPUs (and C++ with a TS) support transactional memory, where several memory accesses together are seen as one transaction comparable with a simple variant of the transactions available in databases, which can be rolled back in case of conflicts and are seen outside only as finished group of operations. https://en.m.wikipedia.org/wiki/Transactional_memory and https://en.cppreference.com/w/cpp/language/transactional_memory – Sebastian Aug 17 '22 at 15:09
  • 2
    For x86 specifically, `inc dword [mem]` isn't an atomic RMW, although if it's aligned, the load and store are separately atomic so you can't get tearing. So yeah, it's effectively the same as 3 separate instructions to load/add/store. For atomic RMW (like `std::atomic::fetch_add(1)`) you need `lock inc [mem]`. See [Can num++ be atomic for 'int num'?](https://stackoverflow.com/q/39393850) for how that works efficiently without actually taking a bus lock, just having this core hang on to the cache line (in MESI exclusive or modified state) for the duration. (@Sebastian) – Peter Cordes Aug 17 '22 at 19:51