How to correctly manage resources with move semantics?

Question

struct foo{
  int* i;
  foo(): i(new int(42)){}
  foo(const foo&) = delete;
  foo(foo&&) = default;
  ~foo(){
    std::cout << "destructor: i" << std::endl;
    delete(i);
  }
};
int main()
{
  foo f;
  auto sp_f = std::make_shared<foo>(std::move(f));
}

This is bad because it seems that the destructor of f will be called once it moves into the shared_ptr. The shared_ptr will have a deleted pointer and will delete it after it goes out of scope, which means the pointer will be deleted two times.

How do I avoid this problem?

Yeah, actually define the move ctor and swap pointers, e.g. That or simply use something which has move semantics defined for it already, like `std::unique_ptr`. — , Dec 05 '15 at 03:41
Instead of a raw array, just use a `std::vector`. There. All OK. The key to managing resources in C++, is to **use the standard resource managers**. At least as far as possible. — Cheers and hth. - Alf, Dec 05 '15 at 03:53
Possible duplicate of [How to actually implement the rule of five?](http://stackoverflow.com/q/5976459/3425536) — Emil Laine, Dec 05 '15 at 04:19
@zenith: maybe your eyes were fooled by the unconventional naming. the member `i` points to a raw array. XXX oh, it's my eyes fooled by unconventional everything: it's a `new` of a single `int`. very silly. — Cheers and hth. - Alf, Dec 05 '15 at 04:26
well, the solution then is pretty simple, namely to use an `int` instead of `int*`. — Cheers and hth. - Alf, Dec 05 '15 at 04:30
In this case you could just use `unique_ptr i` and the Rule of Zero. You can also use `unique_ptr` with a custom deleter, if the resource requires other release code. It's good to avoid reinventing the wheel ! — M.M, Dec 05 '15 at 04:54

score 7 · Accepted Answer · edited Dec 05 '15 at 03:46

7

You need to define the move constructor to prevent deletion from the moved-from object:

foo(foo&& f): i(f.i) {
  f.i = nullptr;
}

Now when to destructor of the old object is run, it won't delete the i, since deleting a null pointer is a no-op.

You should also define a move assignment operator and delete the copy assignment operator too.

edited Dec 05 '15 at 03:46

Aaditya Kalsi

1,039
8
16

answered Dec 05 '15 at 03:41

Emil Laine

41,598
9
101
157

What if my use case wasn't so trivial? What if I would need to call `delete_something(some_handle)` and there would be no "noop" for some default value and calling it two times would be a problem? – Maik Klein Dec 05 '15 at 03:48
You could always fall back to keeping a boolean flag to mark whether to clean the resource or not. – Aaditya Kalsi Dec 05 '15 at 03:53
@MaikKlein I didn't quite understand. Can you provide a more elaborate example, possibly in the original question? – Emil Laine Dec 05 '15 at 04:02
`class foo` could have a member `bool shouldDestroy` which would be checked at destruction time. If it is `true`, destroy the resource. – Aaditya Kalsi Dec 05 '15 at 12:15

score 6 · Answer 2 · edited May 23 '17 at 11:59

The rule of three is really the rule of five now. If you have a class that can be moved from, you should define the move semantics yourself (plus the copy, destructor, etc).

As to how to do that, to quote cppreference's page on std::move, "... objects that have been moved from are placed in a valid but unspecified state." The unspecified state is typically what the object would look like if it had been default initialized, or what would happen if the objects had swap called on them.

A straightforward way is, as @zenith has answered, is to have the move constructor (or assignment operator) set the original pointer to nullptr. This way the data isn't freed, and the original object is still in a valid state.

Another common idiom is, as mentioned, to use swap. If a class needs its own copy and move semantics, a swap method would be handy as well. The move constructor would delegate initialization to the default constructor, and then call the swap method. In a move assignment operator, just call swap. The object being moved into will get the resources, and the other object's destructor will free the original ones.

It would usually look like this:

struct Foo
{
    void* resource; //managed resource
    Foo() : resource(nullptr) {} //default construct with NULL resource
    Foo(Foo&& rhs) : Foo() //set to default value initially
    {
        this->swap(rhs); //now this has ownership, rhs has NULL
    }
    ~Foo()
    {
        delete resource;
    }
    Foo& operator= (Foo&& rhs)
    {
        this->swap(rhs); //this has ownership, rhs has previous resource
    }
    void swap(Foo& rhs) //basic swap operation
    {
        std::swap(resource, rhs.resource); //thanks @M.M
    }
};

Instead of the three-line swap, you can write `std::swap(resource, rhs.resource);` — M.M, Dec 05 '15 at 04:52

How to correctly manage resources with move semantics?

2 Answers2