Why is this piece of C++ code not synchronized

Question

I am learning to write multithreading applications. So share I run into trouble anytime I want my threads to access even the simples shared resources, despite using mutex.

For example, consider this code:

using namespace std;
mutex mu;
std::vector<string> ob;

void addSomeAValues(){
    mu.lock();    
    for(int a=0; a<10; a++){
        ob.push_back("A" + std::to_string(a));
        usleep(300);    
    }
    mu.unlock();
}

void addSomeBValues(){    
    mu.lock();    
    for(int b=0; b<10; b++){            
        ob.push_back("B" + std::to_string(b));        
        usleep(300);    
    }   
    mu.unlock();
}

int main() {    
    std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now();        
    thread t0(addSomeAValues);        
    thread t1(addSomeBValues);    
    std::chrono::steady_clock::time_point end = std::chrono::steady_clock::now();

    t0.join();
    t1.join();

    //Display the results
    cout << "Code Run Complete; results: \n";    
    for(auto k : ob){
        cout << k <<endl;
    }
    //Code running complete, report the time it took
    typedef std::chrono::duration<int,std::milli> millisecs_t;
    millisecs_t duration(std::chrono::duration_cast<millisecs_t>(end-start));
    std::cout << duration.count() << " milliseconds.\n";
    return 0;
}

When I run the program, it behaves unpredictably. Sometimes, the values A0-9 and B0-9 is printed to console no problem, sometimes there is a segmentation fault with crash report, sometimes, A0-3 & B0-5 is presented.

If i am missing a core synchronization issue, pleasee help

Edit: after alot of useful feed back i changed the code to

#include <iostream>
#include <string>
#include <vector>
#include <mutex>
#include <unistd.h>
#include <thread>
#include <chrono>

using namespace std;
mutex mu;
std::vector<string> ob;

void addSomeAValues(){

for(int a=0; a<10; a++){
    mu.lock();
        ob.push_back("A" + std::to_string(a));
    mu.unlock();
    usleep(300);
}
}

void addSomeBValues(){
for(int b=0; b<10; b++){
    mu.lock();
        ob.push_back("B" + std::to_string(b));
    mu.unlock();

    usleep(300);
}
}

int main() {

std::chrono::steady_clock::time_point    start = std::chrono::steady_clock::now() ;

    thread t0(addSomeAValues);
    thread t1(addSomeBValues);

std::chrono::steady_clock::time_point       end = std::chrono::steady_clock::now() ;

t0.join();
t1.join();

//Display the results
cout << "Code Run Complete; results: \n";
for(auto k : ob){
    cout << k <<endl;
}
//Code running complete, report the time it took
    typedef std::chrono::duration<int,std::milli> millisecs_t ;
    millisecs_t duration( std::chrono::duration_cast<millisecs_t>(end-start) ) ;
    std::cout << duration.count() << " milliseconds.\n" ;
return 0;

}

however I get the following output sometimes:

*** Error in `/home/soliduscode/eclipse_workspace/CppExperiment/Debug/CppExperiment':
double free or corruption (fasttop): 0x00007f19fc000920 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x80a46)[0x7f1a0687da46]
/home/soliduscode/eclipse_workspace/CppExperiment/Debug/CppExperiment[0x402dd4]
/home/soliduscode/eclipse_workspace/CppExperiment/Debug/CppExperiment[0x402930]
/home/soliduscode/eclipse_workspace/CppExperiment/Debug/CppExperiment[0x402a8d]
/home/soliduscode/eclipse_workspace/CppExperiment/Debug/CppExperiment[0x402637
/home/soliduscode/eclipse_workspace/CppExperiment/Debug/CppExperiment[0x402278]
/home/soliduscode/eclipse_workspace/CppExperiment/Debug/CppExperiment[0x4019cf]
/home/soliduscode/eclipse_workspace/CppExperiment/Debug/CppExperiment[0x4041e3]
/home/soliduscode/eclipse_workspace/CppExperiment/Debug/CppExperiment[0x404133]
/home/soliduscode/eclipse_workspace/CppExperiment/Debug/CppExperiment[0x404088]
/usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xb29f0)[0x7f1a06e8d9f0]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x7f8e)[0x7f1a060c6f8e]
/lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)[0x7f1a068f6e1d]

---

Update & Solution

With the problem I was experiencing (namely: unpredictable executing of the program with intermittent dump of corruption complaints), all was solved by including -lpthread as part of my eclipse build (under project settings).

I am using C++11. It's odd, at least to me, that the program would compile without issuing a complaint that I have not yet linked against pthread.

So to anyone using C++11, std::thread, and linux, make sure you link against pthread otherwise your program runtime will be VERY unpredictable, and buggy.

Answer 1

If you're going to use threads, I'd advise doing the job at least a little differently.

Right now, one thread gets the mutex, does all it's going to do (including sleeping for 3000 microseconds), then quits. Then the other thread does essentially the same thing. This being the case, threads have accomplished essentially nothing positive and a fair amount of negative (synchronization code and such).

Your current code is almost unsafe with respect to exceptions -- if an exception were to be thrown inside one of your thread functions, the mutex wouldn't be unlocked, even though that thread could no longer execute.

Finally, right now, you're exposing a mutex, and leaving it to all code that accesses the associated resource to use the mutex correctly. I'd prefer to centralize the mutex locking so its exception safe, and most of the code can ignore it completely.

// use std::lock_guard, if available.
class lock { 
    mutex &m
public:
    lock(mutex &m) : m(m) { m.lock(); }
    ~lock() { m.unlock(); }
};

class synched_vec { 
    mutex m;
    std::vector<string> data;
public:
    void push_back(std::string const &s) { 
        lock l(m);
        data.push_back(s);
    }
} ob;

void addSomeAValues(){
    for(int a=0; a<10; a++){
        ob.push_back("A" + std::to_string(a));
        usleep(300);    
    }
}

This also means that if (for example) you decide to use a lock-free (or minimal locking) structure in the future, you should only have to modify the synched_vec, not all the rest of the code that uses it. Likewise, by keeping all the mutex handling in one place, it's much easier to get the code right, and if you do find a bug, much easier to ensure you've fixed it (rather than looking through all the client code).