Is write guaranteed with one thread writing and another reading non-atomic

Question

Say I have

bool unsafeBool = false;

int main()
{
    std::thread reader = std::thread([](){ 
        std::this_thread::sleep_for(1ns);
        if(unsafeBool) 
            std::cout << "unsafe bool is true" << std::endl; 
    });
    std::thread writer = std::thread([](){ 
        unsafeBool = true; 
    });
    reader.join();
    writer.join();
}

Is it guaranteed that unsafeBool becomes true after writer finishes. I know that it is undefined behavior what reader outputs but the write should be fine as far as I understand.

score 1 · Answer 1 · answered Sep 12 '20 at 07:22

1

After writer.join() it guaranteed that unsafeBool == true. But in reader thread the access to it is a data race.

answered Sep 12 '20 at 07:22

ALX23z

4,456
1
11
18

Is it, since you have UB in the program, there ain't guaratees. Your program can as well have false into it thanks to the race condition. (Not sure if any processor does so) – JVApen Sep 12 '20 at 07:29
@JVApen there are always bugs and UBs in program. Data race is UB in the meaning that there is no guarantee what value would be read. But reading won't cause write to fail. It isn't like you have a complex structure that could produce corrupted memory access on read if it being modified. – ALX23z Sep 12 '20 at 08:31
1

@ALX23z As far as I understood in any modern(or even existing) compiler the memory will be written correctly and the cache in the other thread will eventually be updated, but there is no guarantee that a fictional compiler which is compliant with the c++ standard might just decide not to write that memory. For example adding a compiler option to GCC which detects data races and invalidates the write commands will technically still follow the standard. – A.Hristov Sep 12 '20 at 09:39
@A.Hristov detecting data races is neigh impossible - except trivial cases. Generally, I believe standard should classify UB more appropriately. It is puts too much stuff under one name and telling that all of them might have same theoretically unpredictable consequences is stretching it too far. – ALX23z Sep 12 '20 at 10:27
@ALX23z it is certainly possible to detect data races - [thread sanitizer](https://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual) does exactly that (among other things)! _Data race is UB in the meaning that there is no guarantee what value would be read_ No, a data race can cause all kinds of problems! It is also possible that the read does not happen at all, because the compiler removed it. In this trivial example the data race might be benign, but in general a data race and its effects is something you definitely should not underestimate! – mpoeter Sep 13 '20 at 16:45
@mpoeter whether data was read or presumed to be read - it is an optimization thing. Another thing entirely that write doesn’t happen just because and in a completely unrelated thread. – ALX23z Sep 13 '20 at 17:19
@ALX23z yes, it is an optimization thing. But the point is that a data race results in UB, which means that _anything_ is possible - full stop! For example, the compiler might remove a read, which in turn might destroy the termination condition of a loop, causing it to become infinite - which in itself is again UB, so the compiler is well in its right to remove the whole function that contains the loop! Quintessence: don't take data race lightly! – mpoeter Sep 13 '20 at 17:26
@mpoeter you again speak about optimising reads. What about the specific case in the post? And infinite loop is not UB. – ALX23z Sep 13 '20 at 18:05
@ALX23z I should have been more precise - infinite loops _without side effects_ are UB (see http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1528.htm for example). It doesn't matter whether the issue is caused to to compiler optimizations or lack of memory barriers to ensure memory consistency. Data races are UB, full stop! There is no point in arguing whether in "some cases they are ok" - they are not! – mpoeter Sep 14 '20 at 10:17
@mpoeter: You're referring to a part of the Standard that uses the term "assume" differently from anything else in the Standard; no other usage would, in and of itself, entitle an implementation to behave in completely unbounded fashion if the assumption were violated. Contrast with N1570 K3.7.4.1: "That is, any call to the memset_s function shall *assume* that the memory indicated by s and n may be accessible in the future and thus must contain the values indicated by c." Would it make sense to say that calling `memset_s` on storage that was about to become inaccessible invoked UB? – supercat Sep 15 '20 at 17:06
@mpoeter: IMHO, the Standard should have been more explicit about things implementations may do on a presumption that a loop will terminate, such as saying that if the exit of a loop is statically reachable from within it, the execution of the loop as a whole is only sequenced before an action that would be statically reachable via normal loop termination when *some individual action within the loop is likewise sequenced*, and saying that implementations may terminate a program or raise a signal when an execution time limit is reached, *or if it would be impossible for code not to exceed it*. – supercat Sep 15 '20 at 17:15
@supercat The final wording in the C++11 standard was different - here is the C++17 standard which has some minor changes compared to C++11: https://timsong-cpp.github.io/cppwp/n4659/intro.progress#1 I cannot comment on the usage of the term "assume" and the consequences of what should happen if the assumption is not fulfilled. IMHO the forward progress definition (see link) is sensible and sufficient. Either way, the fact is that compilers _do_ rely on this definition to remove empty loops. – mpoeter Sep 17 '20 at 07:26
@mpoeter: I have no beef with compilers removing or deferring the execution of loops whose exit is statically reachable, and which do not contain any actions that would be sequenced before any code that's moved "before" them. Treating a loop whose exit isn't statically reachable as a "do not execute any actions after this loop, because some are worse than useless" indicator wouldn't impair useful optimizations of cases where it wouldn't affect program behavior, since such loops would never be executed in such cases. Further, assuming "If a program doesn't receive input X, it will hit an... – supercat Sep 17 '20 at 14:47
...endless loop, and thus the program may behave as though it receives X regardless what is actually received" is a recipe for disaster except in those rare cases where nothing a program could possibly do would be worse than useless. – supercat Sep 17 '20 at 14:48

score 1 · Accepted Answer · answered Sep 12 '20 at 07:35

UB is and stays UB, there can be reasoning about why things are happinging the way they happen, though, you ain't allowed to rely on that.

You have a race condition, fix it by either: adding a lock or changing the type to an atomic.

Since you have UB in your code, your compiler is allowed to assume that doesn't happen. If it can detect this, it can change your complete function to a noop, as it can never be called in a valid program.

If it doesn't do so, the behaviour will depend on your processor and the caching linked to it. There, if the code after the joins uses the same core as the thread that read the boolean (before the join), you might even still have false in there without the need to invalidate the cache.

In practice, using Intel X86 processors, you will not see a lot of side effects from the race conditions as it has been made to invalidate the caches on write.

supercat · Answer 3 · 2020-09-15T17:37:35.020

Some implementations guarantee that any attempt to read the value of a word-size-or-smaller object that isn't qualified volatile around the time that it changes will either yield an old or new value, chosen arbitrarily. In cases where this guarantee would be useful, the cost for a compiler to consistently uphold it would generally be less than the cost of working around its absence (among other things, because any ways by which programmers could work around its absence would restrict a compiler's freedom to choose between an old or new value).

In some other implementations, however, even operations that would seem like they should involve a single read of a value might yield code that combines the results from multiple reads. When ARM gcc 9.2.1 is invoked with command-line arguments -xc -O2 -mcpu=cortex-m0 and given:

#include <stdint.h>
#include <string.h>
#if 1
uint16_t test(uint16_t *p)
{
    uint16_t q = *p;
    return q - (q >> 15);
}

it generates code which reads from *p and then from *(int16_t*)p, shifts the latter right by 15, and adds that to the former. If the value of *p were to change between the two reads, this could cause the function to return 0xFFFF, a value which should be impossible.

Unfortunately, many people who design compilers so that they will always refrain from "splitting" reads in such fashion think such behavior is sufficiently natural and obvious that there's no particular reason to expressly document the fact that they never do anything else. Meanwhile, some other compiler writers figure that because the Standard allows compilers to split reads even when there's no reason to (splitting the read in the above code makes it bigger and slower than it would be if it simply read the value once) any code that would rely upon compilers refraining from such "optimizations" is "broken".

Is write guaranteed with one thread writing and another reading non-atomic

3 Answers3