CODEWITHSUNDEEP
c++multithreadingc++11lambdapass-by-reference
Nimrod Morag
Nimrod Morag

Reputation: 984

Passing variable by reference to several threads corrupts heap

This is part of a test for thread safety. I'm running an anonymous lambda in different threads.

I use the variable i as thread id.

Originally I passed every variable from main scope by using [&], but this corrupts the heap.

Solved it now by passing i by value, but for the life of me I can't figure out why this would cause problems on the heap since the threads are only reading i.

Can anyone explain?

Minimal compilable example producing error:

#include <thread>
#include <vector>

using namespace std;

int main(int argc, char** argv) {
    vector<thread> threads;
    vector<string> vec1;
    vector<string> vec2;
    for (int i = 0; i < 2; i++) {
        threads.push_back(
            thread([&vec1, &vec2, &i]() {
                for (int j = 0; j < 10; j++) {
                    const string str = "foo";
                    if (i == 0) {
                        vec1.push_back(str);
                    } else {
                        vec2.push_back(str);
                    }
                }
            })
        );
    }

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

    return 0;
}

Output:

*** Error in `/vagrant/bin/TempFileTest': double free or corruption (fasttop): 0x00007f00240008c0 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7f002a0e97e5]
/lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7f002a0f237a]
/lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7f002a0f653c]
/vagrant/bin/TempFileTest(_ZNSt6vectorINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEESaIS5_EE19_M_emplace_back_auxIJRKS5_EEEvDpOT_+0x1a3)[0x4021b3]
/vagrant/bin/TempFileTest[0x401e83]
/usr/lib/x86_64-linux-gnu/libstdc++.so.6(+0xb8c80)[0x7f002a70ac80]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x76ba)[0x7f002a9db6ba]
/lib/x86_64-linux-gnu/libc.so.6(clone+0x6d)[0x7f002a17941d]
======= Memory map: ========
00400000-00403000 r-xp 00000000 08:02 2622834                            /vagrant/bin/TempFileTest
00602000-00603000 r--p 00002000 08:02 2622834                            /vagrant/bin/TempFileTest
00603000-00604000 rw-p 00003000 08:02 2622834                            /vagrant/bin/TempFileTest
02182000-021b4000 rw-p 00000000 00:00 0                                  [heap]
7f001c000000-7f001c021000 rw-p 00000000 00:00 0 
7f001c021000-7f0020000000 ---p 00000000 00:00 0 
7f0024000000-7f0024021000 rw-p 00000000 00:00 0 
7f0024021000-7f0028000000 ---p 00000000 00:00 0 
7f0028d67000-7f0028d68000 ---p 00000000 00:00 0 
7f0028d68000-7f0029568000 rw-p 00000000 00:00 0 
7f0029568000-7f0029569000 ---p 00000000 00:00 0 
7f0029569000-7f0029d69000 rw-p 00000000 00:00 0 
7f0029d69000-7f0029e71000 r-xp 00000000 00:32 313                        /lib/x86_64-linux-gnu/libm-2.23.so
7f0029e71000-7f002a070000 ---p 00108000 00:32 313                        /lib/x86_64-linux-gnu/libm-2.23.so
7f002a070000-7f002a071000 r--p 00107000 00:32 313                        /lib/x86_64-linux-gnu/libm-2.23.so
7f002a071000-7f002a072000 rw-p 00108000 00:32 313                        /lib/x86_64-linux-gnu/libm-2.23.so
7f002a072000-7f002a232000 r-xp 00000000 00:32 45                         /lib/x86_64-linux-gnu/libc-2.23.so
7f002a232000-7f002a432000 ---p 001c0000 00:32 45                         /lib/x86_64-linux-gnu/libc-2.23.so
7f002a432000-7f002a436000 r--p 001c0000 00:32 45                         /lib/x86_64-linux-gnu/libc-2.23.so
7f002a436000-7f002a438000 rw-p 001c4000 00:32 45                         /lib/x86_64-linux-gnu/libc-2.23.so
7f002a438000-7f002a43c000 rw-p 00000000 00:00 0 
7f002a43c000-7f002a452000 r-xp 00000000 00:32 314                        /lib/x86_64-linux-gnu/libgcc_s.so.1
7f002a452000-7f002a651000 ---p 00016000 00:32 314                        /lib/x86_64-linux-gnu/libgcc_s.so.1
7f002a651000-7f002a652000 rw-p 00015000 00:32 314                        /lib/x86_64-linux-gnu/libgcc_s.so.1
7f002a652000-7f002a7c4000 r-xp 00000000 00:32 311                        /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21
7f002a7c4000-7f002a9c4000 ---p 00172000 00:32 311                        /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21
7f002a9c4000-7f002a9ce000 r--p 00172000 00:32 311                        /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21
7f002a9ce000-7f002a9d0000 rw-p 0017c000 00:32 311                        /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21
7f002a9d0000-7f002a9d4000 rw-p 00000000 00:00 0 
7f002a9d4000-7f002a9ec000 r-xp 00000000 00:32 61                         /lib/x86_64-linux-gnu/libpthread-2.23.so
7f002a9ec000-7f002abeb000 ---p 00018000 00:32 61                         /lib/x86_64-linux-gnu/libpthread-2.23.so
7f002abeb000-7f002abec000 r--p 00017000 00:32 61                         /lib/x86_64-linux-gnu/libpthread-2.23.so
7f002abec000-7f002abed000 rw-p 00018000 00:32 61                         /lib/x86_64-linux-gnu/libpthread-2.23.so
7f002abed000-7f002abf1000 rw-p 00000000 00:00 0 
7f002abf1000-7f002ac17000 r-xp 00000000 00:32 42                         /lib/x86_64-linux-gnu/ld-2.23.so
7f002adf6000-7f002adfc000 rw-p 00000000 00:00 0 
7f002ae15000-7f002ae16000 rw-p 00000000 00:00 0 
7f002ae16000-7f002ae17000 r--p 00025000 00:32 42                         /lib/x86_64-linux-gnu/ld-2.23.so
7f002ae17000-7f002ae18000 rw-p 00026000 00:32 42                         /lib/x86_64-linux-gnu/ld-2.23.so
7f002ae18000-7f002ae19000 rw-p 00000000 00:00 0 
7fff30694000-7fff306b5000 rw-p 00000000 00:00 0                          [stack]
7fff30761000-7fff30763000 r--p 00000000 00:00 0                          [vvar]
7fff30763000-7fff30765000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]
Aborted (core dumped)

Minimal compilable example without error (notice no & on i):

#include <thread>
#include <vector>

using namespace std;

int main(int argc, char** argv) {
    vector<thread> threads;
    vector<string> vec1;
    vector<string> vec2;
    for (int i = 0; i < 2; i++) {
        threads.push_back(
            thread([&vec1, &vec2, i]() {
                for (int j = 0; j < 10; j++) {
                    const string str = "foo";
                    if (i == 0) {
                        vec1.push_back(str);
                    } else {
                        vec2.push_back(str);
                    }
                }
            })
        );
    }

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

    return 0;
}

I'm using:

Ubuntu 16.04

gcc 5.4.0

Upvotes: 2

Views: 219

Answers (3)

StoryTeller - Unslander Monica
StoryTeller - Unslander Monica

Reputation: 170153

As a side note, you can spare yourself a lot of grief and reduce code size if you just capture the vector itself conditionally:

for (int i = 0; i < 2; i++) {
    auto& vec = (i == 0 ? vec1 : vec2);
    threads.push_back(
        thread([&vec]() {
            for (int j = 0; j < 10; j++) {
                const string str = "foo";
                vec.push_back(str);
            }
        })
    );
}

Upvotes: 3

Jarod42
Jarod42

Reputation: 217810

Value of i changes at each iteration loop (in main thread) whereas you read it in other thread (without synchronization) -> UB.

Moreover, once the primary loop ends, you have dangling reference to i.

Upvotes: 11

Tomasz Maciejewski
Tomasz Maciejewski

Reputation: 513

Both for and the thread uses the same memory address of i (because you pass it by reference). Correct way is to let the thread have its own copy of i, which would be the same for the thread lifetime and independant from the loop changes.

Upvotes: 2

Related Questions