List Iterator from STL library malfunctioning

Question

The following is a snippet of a larger program and is done using Pthreads.

The UpdateFunction reads from a text file. The FunctionMap is just used to output (key,1). Here essentially UpdateFunction and FunctionMap run on different threads.

queue <list<string>::iterator> mapperpool;

void *UpdaterFunction(void* fn) {
    std::string *x = static_cast<std::string*>(fn);
    string filename = *x;
    ifstream file (filename.c_str());
    string word;
    list <string> letterwords[50];
    char alphabet = '0';
    bool times = true;

    int charno=0;

    while(file >> word) {
        if(times) {
            alphabet = *(word.begin());
            times = false;
        }
        if (alphabet != *(word.begin())) {
            alphabet = *(word.begin());
            mapperpool.push(letterwords[charno].begin());
            letterwords[charno].push_back("xyzzyspoon");
            charno++;
        }
        letterwords[charno].push_back(word);
    }

    file.close();

    cout << "UPDATER DONE!!" << endl;
    pthread_exit(NULL);
}

void *FunctionMap(void *i) {
    long num = (long)i;
    stringstream updaterword;
    string toQ;
    int charno = 0;


    fprintf(stderr, "Print me %ld\n", num);
    sleep(1);

    while (!mapperpool.empty()) {
        list<string>::iterator it = mapperpool.front();
        while(*it != "xyzzyspoon") {
            cout << "(" << *it << ",1)" << "\n";
            cout << *it << "\n";
            it++;   
        }
        mapperpool.pop();
    }

    pthread_exit(NULL);
}

If I add the while(!mapperpool.empty()) in the UpdateFunction then it gives me the perfect output. But when I move it back to the FunctionMap then it gives me a weird out and Segfaults later. Output when used in UpdateFunction: Print me 0 course cap class culture class cap course course cap culture concurrency .....

[Each word in separate line]

Output when used in FunctionMap (snippet shown above): Print me 0 UPDATER DONE!! (course%0+�0@+�0�+�05P+�0����cap%�+�0�+�0,�05�+�0����class5P?�0 ����xyzzyspoon%�+�0�+�0(+�0%P,�0,�0�,�05+�0����class%p,�0�,�0-�05�,�0����cap%�,�0�,�0X-�05�,�0����course%-�0 -�0�-�050-�0����course%-�0p-�0�-�05�-�0����cap%�-�0�-�0H.�05�-�0����culture%.�0.�0�.�05 .�0 ����concurrency%P.�0`.�0�.�05p.�0����course%�.�0�.�08/�05�.�0����cap%�.�0/�0�/�05/�0Segmentation fault (core dumped)

How do I fix this issue?

Answer 1

Here essentially UpdateFunction and FunctionMap run on different threads.

And since they both manipulate the same object (mapperpool) and neither of them uses either pthread_mutex nor std::mutex (C++11), you have a data race. If you have a data race, you have Undefined Behaviour and the program might do whatever it wants. Most likely it will write garbage all over memory until eventually crashing, exactly as you see.

How do I fix this issue?

By locking the mapperpool object.

Why is list not thread-safe?

Well, in vast majority of use-cases, a single list (or any other collection) won't be used by more than one thread. In significant part of the rest the lock will have to extend over more than one operation on the collection, so the client will have to do its own locking anyway. The remaining tiny percentage of cases where locking in the operations themselves would help is not worth adding the overhead for everyone; C++ key design principle is that you only pay for what you use.

The collections are only reentrant, meaning that using different instances in parallel is safe.

Note on pthreads

C++11 introduced threading library that integrates well with the language. Most notably, it uses RAII for locking of std::mutex via std::lock_guard, std::unique_lock and std::shared_lock (for reader-writer locking). Consistently using these can eliminate large class of locking bugs that otherwise take considerable time to debug.

If you can't use C++11 yet (on desktop you can, but some embedded platforms did not get a compiler update yet), you should first consider Boost.Thread as it provides the same benefits.

If you can't use even then, still try to find, or write, a simple RAII wrapper for locking like the C++11/Boost do. The basic wrapper is just a couple of lines, but it will save you a lot of debugging.

Note that C++11 and Boost also have atomic operations library that pthreads sorely miss.

Answer 2

list <string> letterwords[50] is local to UpdaterFunction. When UpdaterFunction finishes, all its local variables got destroyed. When FunctionMap inspects iterator, that iterator already points to deleted memory. When you insert while(!mapperpool.empty()) UpdaterFunction waits for FunctionMap completion and letterwords stays 'alive'.