CODEWITHSUNDEEP
c++multithreadingboostvisual-studio-2013
serhii.syrotynin
serhii.syrotynin

Reputation: 259

Why only last thread executing?

I need to create console application that counts files in folders. Each folder executes parallel. I get directories paths from .txt file and put them to threads.

I'm using std::thread and boost::filesystem.

It works fine with one directory, but crashes or returns wrong results with many. What is interesting that last thread always gets correct result, but those before it are wrong.

Here is my code:

DirHandler.h

#include <iostream>
#include <fstream>
#include <string>
#include <thread>
#include <windows.h>
#include <boost/filesystem.hpp>
using namespace std;
using namespace boost::filesystem;

class DirHandler{

public:
DirHandler();

void getPaths(ifstream file);
void output();
static void run_thread(pair<string, int> * dir, string cur_path, void *args)
{
    DirHandler *prunnable = static_cast<DirHandler*>(args);
    prunnable->some_counting(dir, cur_path);
}

private:

vector<thread> threads;
vector<pair<string, int>> paths; // current directory name and files amount

void some_counting(pair<string, int> * dir, string cur_path);
void escape();
};

DirHandler.cpp

void DirHandler::getPaths(ifstream file)
{
// parse pathes and create separate thread for each
string line;
if (file.is_open())
{
    while (getline(file, line))
    {
        cout << line << endl;
        // add thread for path
        pair<string, int> dir = make_pair(line, 0);
        paths.push_back(dir);
        threads.push_back(thread(&DirHandler::run_thread, &paths.back(), line, this));
    }
    for (auto& thr : threads){
        thr.join();
    }

    file.close();
}
}

void DirHandler::some_counting(pair<string, int> * dir, string cur_path){...}

main.cpp

#include <iostream>
#include <windows.h>
#include "DirHandler.h"

int main(int argc, char* argv[])
{

    DirHandler dirHandler = DirHandler();
    dirHandler.getPaths(ifstream("strings.txt")); //ifstream(argv[1])

    dirHandler.output();



    return 0;
}

Note : While debugging I've found that all streams before last have id=0 in the end

Also I've read that the problem can be caused by a reference to single object. (In my case it's vector<pair<string, int>> paths)

So my question is how to get few threads work correctly ?

Upvotes: 0

Views: 128

Answers (2)

Some programmer dude
Some programmer dude

Reputation: 409196

The problem is that you get pointers from the paths vector, and if the vector is resized all previous pointers (and iterators) to elements in the vector will become invalid. Using an invalid pointer leads to undefined behavior which often leads to crashes.

And one common way of causing a vector is to adding new elements to it, exactly what you do in your loop.

One solution is to either pass a reference to the vector and an index of the element to the thread (the index will not change on reallocation). Another solution is to have two loops, one to add all elements to the vector and the second to create the threads.

There are other solutions too, like not passing a pointer to the thread, but instead pass the pair by value. This is actually the solution I recommend.

After checking your code a little, I also see that since you use std::thread you don't need the static member function wrapper at all. Instead you can call the non-static actual thread function directly:

threads.emplace_back(&DirHandler::some_counting, this, paths.back(), line);

[Note the use of this as the second argument, as well as the change to emplace_back for the threads vector]

Upvotes: 2

ixSci
ixSci

Reputation: 13698

Well, the most obvious problem is here: threads.push_back(thread(&DirHandler::run_thread, &paths.back(), line, this)); you are using pointer to the object(&paths.back()) which might not be there anymore after any push_back. So this error should be fixed, you can't pass pointer to the vector item unless you can guarantee that it will not reallocate its internal structure.

Besides that and ugly code I don't see anything. But more problems might be lurking in the some_counting method.

Upvotes: 0

Related Questions