deadlock on c++ std::thread even with mutex lock

Question

I try to get a better understanding of c++ threads. I implemented a small example where I want four threads to work on a index region within a std::vector.

Of course I ran into deadlocks and read into the use of mutexes and locks. As I understand, the general assumption is that all variables are shared by the threads, unless explicitly stated otherwise (thread_local). If a thread alters any global data it is wise to lock the recourse first, do the work on the data to avoid data races, then unlock the data again so that other threads may use it.

In my example the locks on std::cout work fine, the threads are created fine, the function is called fine, but the program still hangs even though i implemented the data_lock just before and after the data is manipulated. It also works fine if I comment out the data manipulation and display a message instead. The output is different from run to run, so I refrain from posting it.

My feeling is that I miss some concept of c++ threads that I am unaware of (I worked with MPI before).

My Questions:

1. Is there a concept I am missing? What else do I need to know/read?
2. What are the tools other than using mutexes, locks and thread_local to proper execution?

compiler instruction:

g++ -std=c++1y -O0 -g3 -Wall -c -fmessage-length=0 -pthread -MMD -MP -MF"src/main.d" -MT"src/main.d" -o "src/main.o" "../src/main.cpp"

code:

#include <iostream>
#include <thread>
#include <vector>
#include <mutex>
using namespace std;

std::mutex data_lock;
std::mutex cout_lock;

void output(std::string message){
    cout_lock.lock();
    cout << message << endl;
    cout_lock.unlock();
}

void work(std::vector<double>& data, const int s_ind, const int e_ind)     {
    thread_local int i = 0;
    for (i = s_ind; i <= e_ind; i++) {
        data_lock.lock();
        data[i] = 1.0;
        data_lock.unlock();
        //msg("work");
    }
}

int main() {
    const int size = 1000;
    const int cpus = 4;
    const int chunksize = size / cpus;

    //create Data vector
    std::vector<double> dat { (size) };

    //thread vector
    std::vector<std::thread> threads;

    //create and start threads with proper ranges (ranges tested)
    for (int cpu = 0; cpu < cpus; cpu++) {
        threads.push_back(std::thread(work, ref(dat), (cpu * chunksize),(((cpu + 1) * chunksize) - 1)));
        output("thread created");
    }

    //delete threads
    for (int cpu = 0; cpu < cpus; cpu++) {
        threads[cpu].join();
        output("thread joined");
    }
return 0;
}

Answer 1

I know there is a recommendation to use {} in ctor and it eliminates the problem with immplicit function declaration. But you have problem if the class has constructor overload with std::initializer_list<T> as a parameter and std::vector does. So this line:

std::vector<double> dat { (size) };

creates vector with one element size in it.

As your statement that you run into deadlock and have to use mutexes, that's hardly possible. Most probably your program hangs, but that is not considered a deadlock. Deadlock is a situation when your threads block each other trying to lock mutexes in wrong order (or similar situation).

Note: this is not fix but you should use RAII for mutex lock and standard library provides tool for that: std::lock_guard, std::unique_lock or std::scoped_lock if you have c++17. Reason, why RAII should be used in this case (and many others) are described here

RAII guarantees that the resource is available to any function that may access the object (resource availability is a class invariant, eliminating redundant runtime tests). It also guarantees that all resources are released when the lifetime of their controlling object ends, in reverse order of acquisition. Likewise, if resource acquisition fails (the constructor exits with an exception), all resources acquired by every fully-constructed member and base subobject are released in reverse order of initialization. This leverages the core language features (object lifetime, scope exit, order of initialization and stack unwinding) to eliminate resource leaks and guarantee exception safety. Another name for this technique is Scope-Bound Resource Management (SBRM), after the basic use case where the lifetime of an RAII object ends due to scope exit.