Default move assignment and destruction order of members versus the rule-of-zero

Question

Members must frequently be destroyed in the correct order. As member creation is in forward order and destruction is in reverse order this will usually work fine. However, when assignment operators and non-copyable objects are involved, things can start to break down, due to the forward order of move-assignment in the assignment operator.

In the following example, I have a registry-like object and items that register/unregister themselves. Due to this circular dependency, both items and registry are non-copyable/non-movable, and items must be destroyed before the registry is destroyed.

Instances of the holder class, own a registry and an item via unique_ptrs, as the holder itself must be movable.

Example code:

#include <algorithm>
#include <iostream>
#include <memory>
#include <vector>

struct item;

/// registry that knows about items that register/unregister themselves on creation/destruction (stand-in for external code)
struct registry
{
    registry()
    {
        std::cout << " creating registry " << this << "\n";
    }

    ~registry()
    {
        std::cout << " destroying registry " << this << "\n";
    }

    void register_item(item * i)
    {
        is.emplace_back(i);
    }

    void unregister_item(item * i)
    {
        using std::begin; using std::end;
        is.erase(
            std::remove(begin(is), end(is), i),
            end(is)
        );
    }

private:
    std::vector<item *> is;
};

/// item that knows about a registry to which it is registered (stand-in for external code)
struct item
{
    explicit item(registry & r)
        : r{&r}
        , sth{0}
    {
        std::cout << " creating item " << this << " with registry " << &r << "\n";
        r.register_item(this);
    }

    ~item()
    {
        std::cout << " destroying item " << this << " and unregistering from " << r << "\n";
        r->unregister_item(this);
    }

    void do_something()
    {
        ++sth;
        std::cout << sth << "\n";
    }

private:
    registry * r;
    int sth;
};

/// class that owns a registry and one (or more) items
struct holder
{
    holder()
        : r{std::make_unique<registry>()}
        , i{std::make_unique<item>(*r)}
    {}

private:
    std::unique_ptr<registry> r;
    std::unique_ptr<item> i;
};

int main(int, char **)
{
    {
        // creation and destruction work, due to reverse order of destruction
        std::cout << "create/destroy\n";
        holder h;
    }
    {
        // move-assign fails due to forward order of assignment
        std::cout << "move-assign\n";
        holder h;
        h = std::move(holder{});
    }
}

As seen here, the order of destruction is violated during move-assignment (doesn't crash in this minimal example for me, however the real code does and this one could as well, as a member function of a destroyed object is called):

create/destroy
 creating registry 0xdc6f30
 creating item 0xdc6f50 with registry 0xdc6f30
 destroying item 0xdc6f50 and unregistering from 0xdc6f30
 destroying registry 0xdc6f30
move-assign
 creating registry 0xdc6f30
 creating item 0xdc6f50 with registry 0xdc6f30
 creating registry 0xdc1550
 creating item 0xdc1570 with registry 0xdc1550
 destroying registry 0xdc6f30
 destroying item 0xdc6f50 and unregistering from 0xdc6f30
 destroying item 0xdc1570 and unregistering from 0xdc1550
 destroying registry 0xdc1550

By adding the following to the holder class, the issue is resolved

holder & operator=(holder && o) & noexcept
{
    i = std::move(o.i);
    r = std::move(o.r);
}

holder(holder && o) noexcept = default;

as can be seen here

create/destroy
 creating registry 0x7a6f30
 creating item 0x7a6f50 with registry 0x7a6f30
 destroying item 0x7a6f50 and unregistering from 0x7a6f30
 destroying registry 0x7a6f30
move-assign
 creating registry 0x7a6f30
 creating item 0x7a6f50 with registry 0x7a6f30
 creating registry 0x7a1550
 creating item 0x7a1570 with registry 0x7a1550
 destroying item 0x7a6f50 and unregistering from 0x7a6f30
 destroying registry 0x7a6f30
 destroying item 0x7a1570 and unregistering from 0x7a1550
 destroying registry 0x7a1550

However, as we now define our own move assignment operator, we violate the rule of zero (invoking the rule of five) and must write our own move constructor as well (explicitly defaulted in this case).

Is there any cleaner way (that doesn't involve changing the external code) to ensure that the destruction order is maintained in such a scenario?

Answer 1

I think it is an excellent observation that default copy and move assignment do work in the opposite order of the destructor when "destroying" members. I think it's worth keeping in mind when looking for bugs in that area.

However,

Members must frequently be destroyed in the correct order. ...

I would disagree here. I would rather say, most of the time I don't care at all in what order my members are destructed! It's a brittle design to begin with, and I try to avoid it where possible. Obviously, there are cases where you cannot.

The way I see this is that IFF you need to rely on the stack-like ctor/dtor order of object members, then you build a holder_impl that is itself non-copy/non-moveable that holds the corresponding members (by whatever means) and this single object is then held by a holder through a unique_ptr.

This also ties in nicely with the Single Responsibility Principle:

• One type to manage the allocation/deallocation in the correct order
• Second type to manage the moveability of the first one.