C++11 rvalue calls destructior twice

Question

I'm trying to make a class runner (run a class at a fixed time freq), which runs a class in another thread, and can be controlled (like pause, resume, stop) from main thread.

So I want to take advantage of C++11's Functor and other features. But I have a strange problem, the Functor's destructor passed into Runner has been called twice.

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

using namespace std;

class Runner {
 public:
  typedef function<bool()> fn_t;
  Runner(fn_t &&fn) : fn_(move(fn)), thread_(Thread, ref(*this)) {
    cout << "Runner" << endl;
  }
  ~Runner() {
    cout << "~Runner" << endl;
    thread_.join();
  }
 private:
  fn_t fn_;
  thread thread_;
  static void Thread(Runner &runner) {
    while (runner.fn_()) {
      cout << "Running" << endl;
      this_thread::sleep_for(chrono::milliumseconds(1));
    }
  }
};

class Fn {
 public:
  Fn() : count(0) {
    cout << "Fn" << endl;
  }
  ~Fn() {
    cout << "~Fn" << endl;
  }
  bool operator()() {
    return (++count < 5);
  }
 private:
  int count;
};

int main (int argc, char const* argv[])
{
  Fn fn;
  Runner runner(move(fn));
  return 0;
}

outpus:

Fn
Runner
~Fn
~Runner
Running
Running
Running
Running
Running
~Fn
~Fn

and if I change

Fn fn;
Runner runner(move(fn));

to

Runner runner(Fn());

the program outpus nothing and stalls. I have tried to disable compiling optimization, nothing changes. Any explanation?

How can I fix this or do the samething in other method? Should I implement this class like std::async / std::thread?

Update to Runner runner(Fn())

This statement was interrupted as a function declaration.

Runner runner((Fn())) solved problem.

Thanks for all comments and answers. After look into rvalue, seems I have misunderstand the meaning of rvalue reference from ground 0. I will try some other ways.

Final Solution for this problem

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

using namespace std;

template<typename T, typename... Args>
class Runner {
 public:
  Runner(Args&&... args) : 
      t(forward<Args>(args)...), 
      thread_(Thread, ref(*this)) {
    cout << "Runner" << endl;
  }
  ~Runner() {
    cout << "~Runner" << endl;
    thread_.join();
  }
 private:
  T t;
  thread thread_;
  static void Thread(Runner &runner) {
    while (runner.t()) {
      cout << "Running" << endl;
      this_thread::sleep_for(chrono::milliseconds(100));
    }
  }
};

class Fn {
 public:
  Fn() : count(0) {
    cout << "Fn" << endl;
  }
  ~Fn() {
    cout << "~Fn" << endl;
  }
  bool operator()() {
    return (count++ < 5);
  }
 private:
  int count;
};

int main (int argc, char const* argv[])
{
  //vector<Fn> fns;
  //fns.emplace_back(Fn());
  Runner<Fn> runner;
  return 0;
}

outpus:

Fn
Runner
~Runner
Running
Running
Running
Running
Running
~Fn

Add `Fn(Fn const&) { cout << "Fn" << endl; }` and you'll see. — R. Martinho Fernandes, May 16 '12 at 18:50
The problem here is, if the Functor holds some resource (char* for example) and will automatically free when destruct, those copy and destruct will leave a invalid memory pointer in copied Functor. How to fix this? — xiaoyi, May 16 '12 at 19:02
Where have you used std::move? And even if you did, moving does not mean that an additional object is not created and subsequently destroyed. — Benjamin Lindley, May 16 '12 at 19:06
@xiaoyi If Functor holds a resource that must be freed it should also define a copy constructor (and assignment operator) to manage that resource. That solves your problem, aside from having an extra copy operation. — Praetorian, May 16 '12 at 19:07
@Prætorian: Yes, I know this method. Is it possible to hide this from user? — xiaoyi, May 16 '12 at 19:10
You've copied the solution I provided in my answer and pasted it in your question's code snippet. Why would you do that? Didn't my answer solve you problem? (I assume it did, since you marked it as accepted). — mfontanini, May 16 '12 at 19:36
I edited it before you answered it. After Benjamin Lindley mentioned move problem, I updated my code. The problem remains, still destruct twice, and before using move, it destructed 3 times. I think this is a design pattern problem instead of method usage problem. You answer is correct in solving this problem which my program is in a wrong pattern. — xiaoyi, May 16 '12 at 19:48
As it was said, there is nothing wrong with the fact that d-tor gets called twice after move operation (tho I see three ~Fn in your output). Your class should handle it if it supports move semantics. For classes in STL it is guaranteed that after a move the objects are in a valid but unspecified state (more here: http://stackoverflow.com/questions/7930105/does-moving-leave-the-object-in-a-usable-state). — marcinj, May 16 '12 at 20:14

score 4 · Accepted Answer · answered May 16 '12 at 19:12

Use std::move:

Runner(fn_t &&fn) : fn_(std::move(fn)), thread_(Thread, ref(*this)) {
    /*....*/
}

You need to explicitly use std::move, otherwise it will be treated as a const reference. You could also use std::forward:

Runner(fn_t &&fn) : fn_(std::forward<fn_t>(fn)), thread_(Thread, ref(*this)) {
    /*....*/
}

Benjamin Lindley · Answer 2 · 2012-05-16T20:29:08.973

First of all, you shouldn't be taking r-value reference arguments for the most part, except in your own move constructors. As you have it, there is no way to pass l-values of std::function<bool()> into the constructor of Runner.

int main()
{
    Fn fn;
    std::function<bool()> func(fn);
    Runner runner(func); // this is illegal
}

Maybe I'm just not creative enough, but I can't imagine any valid reason which you would want to prevent such a thing.

You should let std::function take care of its own copying/moving. When you need a copy of an object, take your parameter by value. If the function is passed an r-value, then it will be move constructed. If it is passed an l-value, then it will be copy constructed. Then, in your Runner constructor, you can move the value into the member object, as fontanini showed.

None of this is guaranteed to reduce destructor calls though, because when you move an object, you're still creating a second object, and will have to destroy a second object. In order to see fewer destructions, copy elision would have to happen, which actually does avoid the creation of multiple objects. But unlike moving, that's an implementation issue that's not guaranteed to come into effect in all the situations where you would hope.

C++11 rvalue calls destructior twice

2 Answers2