0

If I have a move-only class which is initially owned by AManager, can be transferred to another owner via obtain, returned back to AManager via release.

class AManager;

class A {
public:
    A(const A& other) = delete;
    const A& operator=(const A&) = delete;

    A(A&& other);
    A& operator=(A&& other);

private:
    AManager& _manager;
    bool _isValid;
};

class AManager {
public:
    A obtain() {
        return std::move(_instance);
    }
    void release(A a) {
        _instance = std::move(a);
    }

private:
    A _instance;
};

Is it valid for A to move itself within its own destructor? i.e., is it valid for A to release itself back to its manager when it is destroyed?

A::~A() {
    if (_isValid) {
        _manager.release(std::move(*this));
    }
}

Assuming the A class has some way to know if its in a valid state. For example, I added a valid flag. I believe unique_ptr uses the pointer being null or not.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
Patrick Wright
  • 1,401
  • 7
  • 13
  • I don't see why it would be magically not allowed. It's just a perfectly normal function call, to which you pass your object by rvalue reference. – n. m. could be an AI Sep 08 '21 at 19:50
  • Is it intended for `A` to have additional functionality? So `A` is going to be responsible for its primary purpose **and** be responsible for making sure resources are returned to the `AManager`? Two tasks for one class? – JaMiT Sep 08 '21 at 22:40
  • Hmm... the idea of moving `*this` might warrant keeping this question separate from near-duplicates, but you still might want to look at [Usage of "this" in destructor](https://stackoverflow.com/questions/10979250/) and [Validity of "this" in destructor](https://stackoverflow.com/questions/25639140/). – JaMiT Sep 08 '21 at 22:45
  • Why do you want to write code full of **undefined behavior** and hard to understand. It make absolutly no sense at all to `move *this` from its destructor. – Phil1970 Sep 08 '21 at 23:34
  • @Phil1970: If there’s undefined behavior here, that’s an answer, but you’ll have to write it since I don’t see it. – Davis Herring Sep 09 '21 at 02:51
  • You are inside the destructor and you move yourself into the manager. When the manager is destroyed, it will try to destroy `_instance` which happen to be the exact same object that has already been destroyed. – Phil1970 Sep 10 '21 at 00:58
  • 2
    @Phil1970 Moving `*this` to `_instance` does not cause `_instance` to be the exact same object that has already been destroyed. Rather, it causes `_instance` to become a copy of `*this` with the option for this copy to be done very efficiently. However, you are dancing on the edge of an issue: what does happen when the manager is destroyed? After the manager is destroyed, its members are destroyed, including `_instance`. When `_instance` is destroyed, it might try to release itself to the no-longer-valid manager. – JaMiT Sep 10 '21 at 01:16
  • @JaMiT Well, I overlook some details... But in the end if one has to analyse the code 10 minutes to understand it properly, then it already proove that the code is too complicated. I probably didn't realize that it was making a copy of the data at is even make less sense. And because of the cycle, who wil destroy last copy? – Phil1970 Sep 11 '21 at 03:27
  • @DavisHerring I believe this is UB according to **[basic.life](https://timsong-cpp.github.io/cppwp/basic.life#1.4)**, which says that a class object's lifetime ends when "the destructor call starts". Inside the destructor, the `A` has already ended its lifetime. The members of `A` are still alive, but `A` itself no longer exists. – Raymond Chen Sep 11 '21 at 13:49
  • @RaymondChen: That’s formally true, but we have [class.cdtor]/4 that says that we can still use member functions *on* the object, so it seems absurd that we can’t access its members in a move constructor. I don’t know what [basic.life]/4 means to say, precisely, but I don’t think it’s supposed to prohibit this case, since that would seem to prevent *finding* those living members in the destructor from `this`. – Davis Herring Sep 11 '21 at 15:49

1 Answers1

1

is it valid for A to release itself back to its manager when it is destroyed?

"Valid" in what sense? Will it compile? Yes.

Does it make any kind of sense? No.

You are moving an object that is being destroyed. The owner of that object requested and expects the resources managed by this object to no longer exist. By giving those resources back to the manager, they will continue to exist.

Basically, what you have is pretend-shared-ownership. The manager can pretend to share ownership with you, but in reality, it always owned the object and true ownership of those resources can never be relinquished. It's like a shared_ptr, except that its use count can never exceed 2.

Also, there are no apparent safeguards in place should A outlive its manager, unlike with a proper shared pointer. The reference will simply reference a destroyed object, leading to UB when the A instance is destroyed.

If you want to share ownership, then share it. If you don't, then don't. But this half-state makes your code brittle.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
  • C++ runtime does not destroy `operator delete`d memory, but returns it to the free store for later reuse (how would you destroy memory anyway?) Other resources don't need to be different. There is nothing wrong with returning them to their respective resource managers. – n. m. could be an AI Sep 09 '21 at 19:12
  • @n.1.8e9-where's-my-sharem.: Unless the resource manager is destroyed while you have a resource that it owns. That's what shared ownership *means*: when two or more pieces of code are responsible for destroying a thing. If it's uniquely owned by the resource manager, then it should *not* be owned by users; they should just be borrowing a reference to it. – Nicol Bolas Sep 09 '21 at 19:22