How can I modify a string from a thread?

Question

I am trying to modify a some strings from threads (each thread would have its own string) but all strings are stored in a vector, because i need to be able to access them after the threads have done their thing.

I haven't used threads in c++, so if this is a terrible thing to do, all suggestions welcome :)

Basically the only thing the program does now is:

• create some threads
• send a string and an id to each thread
• thread function modifies the string to add the id to it
• end

This gives a segfault :(

Is this just a bad approach? How else could I do this?

static const int cores = 8;

void bmh_t(std::string & resr, int tid){
    resr.append(std::to_string(tid));
    resr.append(",");
    return;
}        

std::vector<std::string> parbmh(std::string text, std::string pat){

    std::vector<std::string> distlists;
    std::thread t[cores]; 
    //Launch a group of threads
    for (int i = 0; i < cores; ++i) {
        distlists.push_back(" ");
        t[i] = std::thread(bmh_t,std::ref(distlists[i]), i);
    }

    for (int i = 0; i < cores; ++i) {
        t[i].join();
    }

    return distlists;
}

Answer 1

Your basic approach is fine. The main thing you need to consider when writing parallel code is that any data shared between threads is done so in a safe way. Because your algorithm uses a different string for each thread, it's a good approach.

The reason you're seeing a crash is because you're calling push_back on your vector of strings after you've already given each thread a reference to data stored within the vector. This is a problem because push_back needs to grow your vector, when its size reaches its capacity. That growth can invalidate the references that you've dispatched to each thread, causing them to write to freed memory.

The fix is very simple: just make sure ahead of time that your vector doesn't need to grow. This can be accomplished with a constructor argument specifying an initial number of elements; a call to reserve(); or a call to resize().

Here's an implementation that doesn't crash:

static const int cores = 8;

void bmh_t(std::string & resr, int tid){
    resr.append(std::to_string(tid));
    resr.append(",");
    return;
}

std::vector<std::string> parbmh(){

    std::vector<std::string> distlists;
    std::thread t[cores];
    distlists.reserve(cores);

    //Launch a group of threads
    for (int i = 0; i < cores; ++i) {
        distlists.push_back(" ");
        t[i] = std::thread(bmh_t, std::ref(distlists[i]), i);
    }

    for (int i = 0; i < cores; ++i) {
        t[i].join();
    }

    return distlists;
}

Answer 2

The vector of strings is being destructed before the threads can act on the contained strings. You'll want to join the threads before returning so that the vector of strings isn't destroyed.