4

I have a code at work that starts multiple threads that doing some operations and if any of them fail they set the shared variable to false.

Then main thread joins all the worker threads. Simulation of this looks roughly like this (I commented out the possible fix which I don't know if it's needed):

#include <thread>
#include <atomic>
#include <vector>
#include <iostream>
#include <cassert>

using namespace std;

//atomic_bool success = true;
bool success = true;

int main()
{
    vector<thread> v;
    for (int i = 0; i < 10; ++i)
    {
        v.emplace_back([=]
        {
            if (i == 5 || i == 6)
            {
                //success.store(false, memory_order_release);
                success = false;
            }
        });
    }
    for (auto& t : v)
        t.join();

    //assert(success.load(memory_order_acquire) == false);
    assert(success == false);

    cout << "Finished" << endl;
    cin.get();
    return 0;
}

Is there a possibility that main thread will read the success variable as true even though one of the workers set it to false?

I found that thread::join() is a full memory barrier (source) but does that imply synchronized-with relationship with the following read of success variable from the main thread, so that we're guaranteed to get newest value?

Is the fix I posted (in the commented code) necessary in this case (or maybe another fix if this one is wrong)?

Is there a possibility that read of success variable will be optimized away (since it's not volatile) and we will get old value regardless of suppossed to exist implicit memory barrier on thread::join?

The code is suppossed to work on multiple architectures (cannot remember all of them, I don't have makefile in front of me) but there are atleast x86, amd64, itanium, arm7.

Thanks for any help with this.

Edit: I've modified the example, because in real situation more then one thread can try to write to success variable.

Community
  • 1
  • 1
Mikaka
  • 83
  • 5
  • 1
    Isn't the source clear about that? "Since thread creation and structure is defined as a synchronizing operation, joining a thread necessarily happens after that thread terminates. Thus you will see anything that necessarily happened before the thread terminated." – Jean-Baptiste Yunès Feb 11 '17 at 11:20
  • I'm not sure since as you can see here http://en.cppreference.com/w/cpp/atomic/atomic_thread_fence reference only mentions synchronization between fences and atomic objects, there is nothing about synchronization between fences and ordinary objects. I think join uses such fence internally, but I may be wrong. – Mikaka Feb 11 '17 at 11:25
  • 2
    [Yes](https://timsong-cpp.github.io/cppwp/thread.thread.member#4). Otherwise `join` would be rather useless. – T.C. Feb 11 '17 at 12:29
  • 1
    @T.C. isn't the multiple unsynchronised overwrite of `success` between the 5 threads technically UB? – Richard Hodges Feb 11 '17 at 13:57
  • @RichardHodges Only one thread writes to it in the example...? – T.C. Feb 11 '17 at 14:06
  • @T.C. ah yes. you're right. – Richard Hodges Feb 11 '17 at 14:17
  • @RichardHodges Hi I'm sorry for this error, I've modified the example, because in real program more than one thread can try to write to *success* variable, does this change the situation? – Mikaka Feb 11 '17 at 15:14
  • 1
    @T.C. what about now it's been edited. Although I expect it would work, I think it's UB. – Richard Hodges Feb 11 '17 at 15:55
  • 1
    @RichardHodges Yep, clear cut nasal demons now. – T.C. Feb 11 '17 at 15:56

1 Answers1

2

The code above represents a data race, and the use of join cannot change that fact. If only one thread wrote to the variable, it would be fine. But you have two threads writing to it, with no synchronization between them. That's a data race.

join simply means "all side effects of that thread's operation have completed and are now visible to you." That does not create ordering or synchronization between that thread and any thread other than your own.

If you used an atomic_bool, then it wouldn't be UB; it would be guaranteed to be false. But because there is a data race, you get pure UB. It might be true, false, or nasal demons.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
  • Ok I get the undefined behavior part. But assuming we're on the platform where write to bool is atomic, and my thread (the one which joins all workers) is the only one reading the *success* flag, will *join* be enough to guarantee proper ordering and I will always see *false* if some worker set it or can I still get *true*? – Mikaka Feb 11 '17 at 15:50
  • 1
    @Mikaka: "*But assuming we're on the platform where write to bool is atomic*" No. You asked about C++. I'm telling you what C++ guarantees. How "arbitrary platform that I hope does something reasonable" behaves is a matter between you and your platform of choice. If you want this to be *guaranteed* to work, you use an actual `atomic`. If that is a platform that behaves in the way you think it does, then `atomic` will just be a thin wrapper around a `bool`. And if it doesn't, then it will still be correct. – Nicol Bolas Feb 11 '17 at 16:03