Code review comment for lp:~raof/mir/fix-threaded-dispatcher-death-test-race

I don't think there *is* an ordering between setting and reading it:

By my understanding the following code is allowed to loop infinitely:

    bool stop{false};
    std::thread i_can_endless_loop{[&stop]() { while (!stop) ; }};

    stop = true;

because there's no memory barrier - the reads from stop in “while (!stop)” are neither before nor after the “stop = true” write.

However

    std::atomic<bool> stop{false};
    std::thread i_cannot_endless_loop{[&stop]() { while (!stop) ; }};

    stop = true;

because the atomic read and write operations provide an ordering.

This is similar to what's happening in the test - new ThreadedDispatcher is starting a thread, then assigning to dispatcher.

dispatchable->trigger() is *not* a memory barrier - it's a write to a file-descriptor. I think the std::atomic<md::ThreadedDispatcher*> is technically necessary.

Now, in practise I don't think this will actually be a problem, because:
1) I don't think any compiler is going to be smart enough to notice that ‘delete dispatcher’ does not happen-after a write to dispatcher, and so replace it with abort() (which would be standards-conforming)

and

2) All the platforms we care about provide cache-coherency, so the write to dispatcher will be seen by the ThreadedDispatcher thread.

Actually, now that I notice it, without std::atomic<> I don't think the compiler is even required to emit a store to dispatcher?