Passing 2 pointers to 2 threads but they end up sharing the same

Question

I guess this problem has appeared already and it surely shows my beginner level in the world of threads, but I haven't been able to find any previous question nor other resource addressing it. I've gone through the most common introductions to C++11 threads (such as this, this and this) but it didn't help.

Here is my code:

mutex mtx;
vector<thread> threads;

for(vcit = vc.begin(); vcit != vc.end(); ++vcit) {
    const std::shared_ptr<Graph> g = graphs.at(*vcit);

    cout << "Graph (outside thread): " << g->name << endl;
    threads.push_back(thread(
        [&g, &mtx] () {
            lock_guard<mutex> guard(mtx);
            cout << "Graph (inside thread): " << g->name << endl;
        }
    ));
}

for(thread& t : threads) {
    t.join();
}

I would expect each thread to receive a different pointer, but instead the output of the program is as follows (for 2 elements in vector vc):

Graph (outside thread): ABC
Graph (outside thread): DEF
Graph (inside thread): DEF
Graph (inside thread): DEF

Occasionally the program would "work" and output:

Graph (outside thread): ABC
Graph (inside thread): ABC
Graph (outside thread): DEF
Graph (inside thread): DEF

(notice the mixed order of output from outside and inside now). I tried to move from a lambda to a functor object, but that was of no help and the program exhibited the same behaviour.

I'd like to know where the problem lies with the code and also where my understanding of how threads (or shared_ptr's perhaps) work is flawed, if this is deducible from the code.

I don't understand all the C++11 stuff, but it seems that you are passing `&g` to the thread, but `g` is an object that is destroyed at the end of the loop. Therefore undefined behaviour. — john, Nov 29 '13 at 13:00
Impressive how the error was very basic, but I got carried away by this threads novelty and was focussing somewhere else! :) — Alberto Santini, Nov 29 '13 at 13:23
"...the normal style is to give a *new* heap object to a shared_ptr when it is created" — SChepurin, Nov 29 '13 at 13:39

score 10 · Accepted Answer · answered Nov 29 '13 at 13:00

10

The lambda is capturing g by reference, which in effect is storing a pointer to memory that only exists inside the for loop. Although it's not defined behaviour since the memory is on the stack, the odds are the two addresses will be the same. Therefore, sometimes the two threads will read the same value, and sometimes they'll read different values - and sometimes they could even read garbage values.

answered Nov 29 '13 at 13:00

Jonathan Potter

36,172
4
64
79

Your explanation makes perfect sense. Thanks. – Alberto Santini Nov 29 '13 at 13:21

score 3 · Answer 2 · answered Nov 29 '13 at 13:04

3

You are passing pointer to g to each thread. That way all threads receive pointer to the same variable. Because you modify g concurrently to their execution you get non-deterministic response. Simply change [&g,&mgx] to [g,&mtx] to fix it.

answered Nov 29 '13 at 13:04

NonNumeric

1,079
7
19

Passing 2 pointers to 2 threads but they end up sharing the same

2 Answers2