Call a method with thread in C++

Question

How to call a class method with thread ?

My class:

using namespace std;
class series{
private:
    string filename;
    vector<double> data;
public:
    series(string _filename);
    int loaddata(const int col);
    void readdata() const;

};

series::series(string _filename):filename(_filename) {}

int series::loaddata(const int col)
{
    ifstream input(filename);
    string line;
    if (!input) {
        cout << "File failed to open" << endl;
        return 0;
    }
    while(!input.eof())
    {
        while(getline(input, line)){
            vector<string> oneline;
            boost::split(oneline, line, boost::is_any_of("|"));
            data.push_back(boost::lexical_cast<double>(oneline[col]));
        }

    }
    return 1;
}

Calling it from main, only posting the relevant part. csv is a vector of filenames, basically vector of strings.

vector<series> ts;
int col = 0;
vector<thread> th;
for (unsigned int i = 0; i != csv.size(); ++i) {
    ts.push_back(series(csv[i]));
    th.emplace_back(thread(&series::loaddata, ref(ts[i]), col));
}

Gives the error I could not understand.

/usr/include/c++/4.8/functional: In instantiation of ‘struct std::_Bind_simple<std::_Mem_fn<int (series::*)(int)>(std::reference_wrapper<series>, int)>’:
/usr/include/c++/4.8/thread:137:47:   required from ‘std::thread::thread(_Callable&&, _Args&& ...) [with _Callable = int (series::*)(int); _Args = {std::reference_wrapper<series>, int}]’
/home/d066537/ClionProjects/correlation/src/main.cpp:105:66:   required from here
/usr/include/c++/4.8/functional:1697:61: error: no type named ‘type’ in ‘class std::result_of<std::_Mem_fn<int (series::*)(int)>(std::reference_wrapper<series>, int)>’
       typedef typename result_of<_Callable(_Args...)>::type result_type;
                                                             ^
/usr/include/c++/4.8/functional:1727:9: error: no type named ‘type’ in ‘class std::result_of<std::_Mem_fn<int (series::*)(int)>(std::reference_wrapper<series>, int)>’
         _M_invoke(_Index_tuple<_Indices...>)

Solution please, Using Clion CMake for the program and yes threading works for free functions so I don't think It's matter of any compiler flag.

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -lpthread -std=c++11")

If I remove ref wrapper from object I get this error:

terminate called after throwing an instance of 'std::system_error' what(): Enable multithreading to use std::thread: Operation not permitted

Now updated to g++-5, without ref wrapper and again the error: CMakeFiles/correlation.dir/src/main.cpp.o: In function

`std::thread::thread<int (series::*)(int), series&, int>(int (series::*&&)(int), series&, int&&)':
/usr/include/c++/5/thread:137: undefined reference to `pthread_create'

Answer 1

Here’s one way to solve the immediate problems I can find in your code. I also added std:: everywhere because using namespace std is bad.

std::vector<series> ts;
ts.reserve(csv.size());  // [1]

int col = 0;
std::vector<std::thread> th;
th.reserve(csv.size());  // [1a] optional

for (unsigned int i = 0; i != csv.size(); ++i) {
    ts.push_back(series(csv[i]));
    th.emplace_back(&series::loaddata, &ts[i], col);  // [2]
}

// Lots of other code could be executed here. Joining only has to happen 
// at *some* point after creating the threads.

// [3]
for (auto& thread : th) {
    thread.join();
}

[2]

Let’s start here because this is the most straight forward problem. The pattern to call a member function with std::thread is:

std::thread(ptr-to-member-func, ptr-to-object, member-func-args...);

Essentially you have to provide the object that will become this when executing loaddata(). And it’s called the “this pointer” for a reason. ;)

Also emplace_back() is a forwarding function that only takes the constructor arguments of the object you want to put into the vector. Explicitely constructing std::thread(...) here defeats the purpose.

[3]

This one is also simple. You have to make sure that the threads can do all their work and exit properly. That means calling join() at some point, which will block until the thread is done. That’s especially important if you still have threads running when you want to exit your whole program.

Note that calling join() inside the for loop like in ΔλЛ’s answer won’t work. If you do it like that you’ll create a single thread, wait for it to finish, and then continue with the next thread. That’s a convoluted way of doing single-threaded execution.

[1]

A small reminder on how vector (and basically all other std containers) works. It allocates a block of memory and uses that for the items you push_back(). When it runs out of space it allocates a new – larger – block of memory, copies or moves the existing items there and continues to push_back(). Reallocation means that all references and pointers to the items as well as all iterators into the vector are invalidated.

Which brings us back to [2]. There you pass a pointer to an item in the vector ts. But ts will have to re-allocate at some point during the loop (at least that’s what you have to assume) – leaving the threads with dangling pointers. That’s classic undefined behaviour.

There are multiple ways to solve that problem. [1] shows a very simple one that works nicely if you know in advance how large the vector will become. reserve() allocates enough space for the given number of elements, which prevents reallocation. Your pointers stay valid.

Another solution is to introduce an additional indirection: usually by allocating the items on the heap and storing pointers in the vector. Like this:

std::vector<std::unique_ptr<series>> ts;

for (...) {
    ts.push_back(new series(csv[i]));
    // In C++14 you would use std::make_unique instead of a raw new.

    th.emplace_back(
        &series::loaddata, 
        ts[i].get(),  // notice the difference
        col);
}

Now the vector can reallocate as much as it likes. It only needs to copy some pointers to its new memory block. The addresses of the actual series objects are not affected anymore.

[1a]

Why did I say this is optional? th is going to reallocate the same as ts. However, you don’t hold on to any refs or pointers to your thread objects. It doesn’t matter if they get invalidated. And std::thread is movable, i.e. it will survive reallocation just fine. On the other hand memory allocation is a potentially expensive operation and it’s trivial to avoid in this case.

P.S.

Have a look at std::async for a more high-level approach to concurrency. Threads are a very low-level construct. If you can, better avoid dealing with them directly.

Answer 2

You are creating you pointer as a temporary, stack-allocated variable and emplace_back it into your vector. When you get out of the for loop iteration, the desctrutor is executed and std::terminate will be called to terminate the running thread.

One way to fix this would be to use std::vector<std::thread*> instead:

vector<thread*> th;
for (unsigned int i = 0; i != csv.size(); ++i) {
    ts.push_back(series(csv[i]));
    thread t = new thread(&series::loaddata, ref(ts[i]), col)
    th.emplace_back(t);
    th[i]->join();
}

Remember to delete your threads after you finish loading data.