Does this code need synchronization?

Question

I plan on writing a multithreaded part in my game-project:

Thread A: loads bunch of objects from disk, which takes up to several seconds. Each object loaded increments a counter.

Thread B: a game loop, in which I either display loading screen with number of loaded objects, or start to manipulate objects once loading is done.

In code I believe it will look as following:

Counter = 0;
Objects;

THREAD A:
    for (i = 0; i < ObjectsToLoad; ++i) {
        Objects.push(LoadObject());
        ++Counter;
    }
    return;

THREAD B:
    ...
    while (true) {
        ...
        C = Counter;
        if (C < ObjectsToLoad)
            RenderLoadscreen(C);
        else
            WorkWithObjects(Objects)
        ...
    }
    ...

Technically, this can be counted as a race condition - the object may be loaded but counter is not incremented yet, so B reads an old value. I also need to cache counter in B so its value won't change between check and rendering.

Now the question is - should I implement any synchronization mechanics here, like making counter atomic or introducing some mutex or conditional variable? The point here is that I can safely sacrifice an iteration of loop until the counter changes. And from what I get, as long as A only writes the value and B only checks it, everything is fine.

I've been discussing this question with a friend but we couldn't get to agreement, so we decided to ask for opinion of someone more competent in multithreading. The language is C++, if it helps.

You also have to consider memory visibility / caching. – 1000ml Jan 04 '17 at 15:07 — 1000ml, Jan 04 '17 at 15:07

score 2 · Accepted Answer · edited May 23 '17 at 12:33

You have to consider memory visibility / caching. Without memory barriers this can very well lead to delays of several seconds until the data is visible to Thread B⁽¹⁾.

This applies to both kind of data: The Counter and the Objects list.

The C++11 standard⁽²⁾ guarantees that multithreaded programs are executed correctly only if you don't introduce race conditions. Without synchronization your program basically has undefined behaviour⁽³⁾. However, in practice it might work without.

Yes, use a mutex and synchronize access to Counter and Objects.

⁽¹⁾ This is because each CPU core has its own registers and cache. If you don't tell the CPU Core A that some other Core B might be interested in the data, it can do optimizations by e.g. leaving the data in a register. Core A has to write the data to a higher level memory region (L2/L3 Cache or RAM) so Core B can load the changes.

⁽²⁾ Any version before C++11 did not care about multithreading. There was support for mutexes, atomics etc. through third-party libraries but the language itself was thread-agnostic.
See: C++11 introduced a standardized memory model. What does it mean? And how is it going to affect C++ programming?

⁽³⁾ The problem is that your code can be reordered (for more efficient execution) at different stages: At the compiler, the assembler and also at the CPU. You must tell the computer which instructions need to stay in that order by adding memory barriers through atomics or mutexes. This works the same in most languages.

I'd recommend watching these very interesting videos about the C++11 memory model:
atomic<> weapons by Herb Sutter

IMO: If you identify data that is accessed by multiple threads, use synchronization. Multithreading-bugs are hard to track and reproduce, so better avoid them all together.

Could you please elaborate more on this topic applied to C++? From quick googling it looks like some crazy mindblowing stuff that silently substitues UB in places where it isn't even possible by sane logic. Also, can't I use `volatile` to forbid such reordering? — Artalus, Jan 04 '17 at 15:42
I'll see if I can find the references I have in mind. The whole topic is quite language-independent, though. Regarding `volatile`: http://stackoverflow.com/questions/2484980/why-is-volatile-not-considered-useful-in-multithreaded-c-or-c-programming — 1000ml, Jan 04 '17 at 15:53
@Artalus: Edited. Also note that `volatile` in C++ means something different than in Java. — 1000ml, Jan 04 '17 at 16:48

score 0 · Answer 2 · answered Jan 04 '17 at 15:06

Race condition is typically only when two threads try to non-atomically read-modify-write concurrently the same datum. In this case, only one thread writes (thread A), while the other thread reads (thread B).

The only "incorrectness" you'll encounter is, as you said, if the object has been loaded but the counter hasn't been incremented. This causes B to read stale data, as the load-and-increment operation was not executed atomically.

If you don't mind this innocent anomaly, then it works just fine. :)

If this annoys you, then you need to execute all of the load-and-increment statements in one go (by using locks or any other synchronization primitive).

Does this code need synchronization?

2 Answers2