segmentation fault, possible reason: function pointer, multi-thread, stl map, etc

Question

I am new to c++. I don't know why the following code has segmentation fault. Doo() is a class that contain a map<>. You can call Doo::start() to start a while loop thread. And then later call Doo::turnoff() to terminate the thread. I don't know what is wrong with my code. Please help me understand.

#include <iostream>
#include <thread>
#include <map>
#include <chrono>


using namespace std;

class Doo{
    int id;
    bool _turnoff=false;
    map<int,string> msg;
public:
    Doo(int _id);
    void start(bool (*fptr)(map<int,string>&));
    void turnoff();
};

Doo::Doo(int _id){
    id = _id;
    msg[1]="hello";
    msg[2]="nihao";
    msg[4]="conichiwa";
}

void Doo::start(bool (*fptr)(map<int,string>&)){
    thread m_thr([&](){
        while(!_turnoff){
            this_thread::sleep_for(chrono::seconds(1));
            fptr(msg);
        }
    });
    m_thr.detach();
}

void Doo::turnoff(){
    _turnoff=true;
}

bool hdl(map<int,string>& greet){
    cout<<greet[2]<<endl;
    return true;
}

int main(void){
    Doo d(1);
    d.start(hdl);
    while(1){
        char x;
        cin>>x;
        if(x=='q'){
            cout<<"quit"<<endl;
            d.turnoff();
            this_thread::sleep_for(chrono::seconds(1));
            break;
        }
    }
    return 0;
}

I compiled with following command:

g++ p3.cpp -std=c++11 -pthread

It compiled without any problem.

valgrind result:

==18849== Memcheck, a memory error detector
==18849== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==18849== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==18849== Command: ./a.out
==18849== 
==18849== 
==18849== Process terminating with default action of signal 11 (SIGSEGV)
==18849==  Bad permissions for mapped region at address 0x68C1700
==18849==    at 0x68C1700: ???
==18849==    by 0x402799: void std::_Bind_simple<Doo::start(bool (*)(std::map<int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<int>, std::allocator<std::pair<int const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >&))::{lambda()#1} ()>::_M_invoke<>(std::_Index_tuple<>) (in /home/xli1989/Projects/playground/a.out)
==18849==    by 0x4026EF: std::_Bind_simple<Doo::start(bool (*)(std::map<int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<int>, std::allocator<std::pair<int const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >&))::{lambda()#1} ()>::operator()() (in /home/xli1989/Projects/playground/a.out)
==18849==    by 0x40267F: std::thread::_Impl<std::_Bind_simple<Doo::start(bool (*)(std::map<int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<int>, std::allocator<std::pair<int const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >&))::{lambda()#1} ()> >::_M_run() (in /home/xli1989/Projects/playground/a.out)
==18849==    by 0x4EF2C7F: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21)
==18849==    by 0x53D96F9: start_thread (pthread_create.c:333)
==18849==    by 0x56F5B5C: clone (clone.S:109)

Answer 1

First of all there's a data race on _turnoff, so your example will exhibit UB. Keep in mind that nothing is atomic by default in C++ so you better use std::atomic or synchronize.

---

Now, the problem you are referring to arises from the thread task taking a reference to an automatic object (Doo::msg) that is allocated by the main thread. As the worker thread is detached from it's handle in the main thread, this means that that the main thread will be able to terminate before the worker thread and destroy all of its allocated objects on the stack (main destroys its own stack). This results in the possibility of the worker thread keeping a reference to an object that is destroyed.

After sending the turnoff signal in main, you only wait/sleep the same amount of time as the wait/sleep in the worker thread loop. As a result there is a strong possibility of the worker thread "finishing last" using a dangling reference.

This is usually not a problem as the program will terminate anyway, but Valgrind will still complain.

Answer 2

Problem lies in lambda in thread m_thr. You are capturing fptr by reference, but this reference is destroyed, once function executes. But thread tries to use reference to destroyed fptr and dying because of segmentation fault.

The solution is simple - capture fptr by value, not by reference.

thread m_thr([=](){ // changed & to =
    while(!_turnoff){
        this_thread::sleep_for(chrono::seconds(1));
        fptr(msg);
    }
});