Make the mutex global/static. The way you have it, the mutex exists in the scope of this function only, which is not shared between different threads, meaning there is no actual locking happening at all cross-threads.
Also, unless this is an example for how to produce this kind of errors - having a lock like this makes threads literally worthless. Any one thread that grabs the lock the first time will push its 10000 elements, while the other one has to wait. You could have literally used sequential coding in this case. Just though I add, don't know if you are aware of this.
To further explain, mutex is not a magical thing that exists to solve all threading problems, but a simple language construct. Essentially, mutex is equivalent to:
void doit(std::vector<int>& d){
static bool locked = false;
{
while(locked)
{
sleep(1);
}
locked = true; // atomic
for (int i = 0; i < 10000;i++)
d.push_back(15);
locked = false;
}
Only that it is implemented more efficiently, for example by having the lock be atomic, so that two threads cannot interfere. This should make it pretty clear already why first of all you have to share the mutex explicitely between threads. Also you have to make sure you do not deadlock your other threads by locking too often or in the wrong places. I'm not too much of an expert on threading, but essencially it should be better in your above example to handle the lock like this:
void doit(std::vector<int>& d){
static std::mutex m;
{
std::vector<int> dTemp;
for (int i = 0; i < 10000;i++)
dTemp.push_back(15);
std::lock_guard<std::mutex> guard(m);
d += dTemp;
}
}
This allows all threads to do their work in parallel, and only write their output once the thread is finished. It might not be better, depending on how the appending of the array is implemented, but in more real-world applications (ie. the more complicated the work you do inside the threads is becoming), from my understanding this should yield better results. Someone correct me if I'm wrong.