Enforce concurrent modification of a variable (C++)

Question

I'm trying to unit test an atomic library (I am aware that an atomic library is not suitable for unit testing, but I still want to give it a try)

For this, I want to let X parallel threads increment a counter and evaluate the resulting value (it should be X).

The code is below. The problem is that is it never breaks. The Counter always nicely ends up being 2000 (see below). What I also notice is that the cout is also printed as a whole (instead of being mingled, what I remember seeing with other multithreaded couts)

My question is: why doesn't this break? Or how can I let this break?

#include <iostream>
#include <thread>
#include <vector>
#include <mutex>
#include <condition_variable>

std::mutex m;
std::condition_variable cv;
bool start = false;

int Counter = 0;

void Inc() {

    // Wait until test says start
    std::unique_lock<std::mutex> lk(m);
    cv.wait(lk, [] {return start; });

    std::cout << "Incrementing in thread " << std::this_thread::get_id() << std::endl;
    Counter++;
}

int main()
{
    std::vector<std::thread> threads;

    for (int i = 0; i < 2000; ++i) {
        threads.push_back(std::thread(Inc));
    }

    // signal the threads to start
    {
        std::lock_guard<std::mutex> lk(m);
        start = true;
    }
    cv.notify_all();

    for (auto& thread : threads) {
        thread.join();
    }

    // Now check whether value is right
    std::cout << "Counter: " << Counter << std::endl;
}

The results looks like this (but then 2000 lines)

Incrementing in thread 130960
Incrementing in thread 130948
Incrementing in thread 130944
Incrementing in thread 130932
Incrementing in thread 130928
Incrementing in thread 130916
Incrementing in thread 130912
Incrementing in thread 130900
Incrementing in thread 130896
Counter: 2000

Any help would be appreciated

UPDATE: Reducing the nr of threads to 4, but incrementing a million times in a for loop (as suggested by @tkausl) the cout of thread id appear to be sequential..

UPDATE2: Turns out that the lock had to be unlocked to prevent exclusive access per thread (lk.unlock()). An additional yield in the for-loop increased the race condition effect.

Answer 1

cv.wait(lk, [] {return start; }); only returns with the lk acquired. So it's exclusive. You might want to unlock lk right after:

void Inc() {
    // Wait until test says start
    std::unique_lock<std::mutex> lk(m);
    cv.wait(lk, [] {return start; });
    lk.unlock();

    Counter++;
}

And you must remove std::cout, because it potentially introduces synchronization.