0

I am not sure why even I called the object's member function, even it is still in it's scope, the destructor is still called. A simple example is shown as below:

#include<thread>
#include<iostream>
class Obj1
{
private:

public:
    ~Obj1();
    void testFunc();
};

Obj1::~Obj1()
{
    std::cout<<"destory\n";
}
void Obj1::testFunc(){
    std::cout<<"testfun\n";
}
#include "Obj1.hpp"
#include <thread>
#include <chrono>
int main()
{
    using namespace std::chrono_literals;
    Obj1 obj1 = Obj1();
    for(int i=0;i<100;i++){
        std::thread th =  std::thread(&Obj1::testFunc,obj1);
        std::this_thread::sleep_for(1s);
        std::cout<<"we wait\n";
    }
}

When I tried to run it, I can see the output:

destory
testfun
destory
we wait
terminate called without an active exception
Aborted (core dumped)

I wonder why obj1 is destroyed every time the thread ends? p.s. the reason for that 1s delay is because this is used in a realtime system, the main loop has a lower frequency, the task will be done before the next loop.

HAO LEE
  • 153
  • 3
  • 13
  • 1
    Just FYI. this is overkill: `Obj1 obj1 = Obj1();` should simply be `Obj1 obj1;`. Second, thread args are by-value (so in your case, copied). If you want to reference the same object, then use a ref-wrapper. I'd use the address of obj1. – WhozCraig Jul 20 '19 at 08:52
  • Why do you sleep for 1s ? To allow the thread to finish work? Bad practise. Also you don't call th.join() or th.detach(); – Michael Chourdakis Jul 20 '19 at 08:53
  • You're copying the object and you don't know it, because you're not logging all the calls to Big Five (only the destructor). See https://stackoverflow.com/questions/56799469/why-is-emplace-back-behaving-like-this/56799500#56799500 and here's [what happens when you fix the logging](http://coliru.stacked-crooked.com/a/1b368459f57728d4) – milleniumbug Jul 20 '19 at 09:03
  • @WhozCraig std::ref(obj1) ??? – HAO LEE Jul 20 '19 at 09:16
  • @HAOLEE for member fn ptrs, its going to take an obj address or a bind setup. the former is in my answer below. – WhozCraig Jul 20 '19 at 09:17

1 Answers1

3

The two biggest problems in your code:

  • You're making copies for each std::thread you launch.
  • You're not waiting for your threads to terminate.

A std::thread requires a callable, and if required, appropriate arguments. In your case the callable is pointer-to-member-function, which requires an object instance or address (std::thread will use either). You're giving it the former by way of making copies of obj1. If the intent is for all threads to access the same object, you should pass the address instead.

And then wait for the threads to terminate, of course

Code (with added messaging to detect copy-construction)

#include <iostream>
#include <vector>
#include <thread>

class Obj1
{
public:
    Obj1() { std::cout << "construct\n"; }
    Obj1(const Obj1&) { std::cout << "copy\n"; }
    ~Obj1() { std::cout << "destroy\n"; }

    void testFunc();
};

void Obj1::testFunc() {
    std::cout << "testfun\n";
}

int main()
{
    Obj1 obj1;

    std::vector<std::thread> threads;
    for (int i = 0; i < 10; ++i)
        threads.emplace_back(&Obj1::testFunc, &obj1); // <<== HERE

    for (auto& t : threads)
        t.join();
}

Output (can vary)

construct
testfun
testfun
testfun
testfun
testfun
testfun
testfun
testfun
testfun
testfun
destroy
WhozCraig
  • 65,258
  • 11
  • 75
  • 141
  • A similar sample (with a little bit more noise): [**Live Demo on coliru**](http://coliru.stacked-crooked.com/a/b6a20de1c5e83d5a). – Scheff's Cat Jul 20 '19 at 09:34