Derived member states during destruction

Question

This is a similar question to this, but that focused on virtual methods (both in the question and answers) and I'm more interested in non-virtual methods and data members of the derived class and how it interacts with a related type hierarchy. In this test code:

#include <iostream>
#include <vector>

using namespace std;

struct ItemBase
{
    explicit ItemBase(int v) : value(v) {}
    virtual ~ItemBase() {}

    virtual void f() { cout << "In ItemBase::f() for " << value << endl; }
    int value;
};

struct ListBase
{
    virtual ~ListBase() { cout << "In ~ListBase" << endl; iterate(); }

    void add(int v) { items.push_back(new ItemBase(v)); }

    void iterate()
    {
        for (vector<ItemBase*>::iterator it = items.begin(); it != items.end(); ++it)
        {
            (*it)->f();
        }
        items.clear();
    }

    vector<ItemBase*> items;
};

struct ListDerived : public ListBase
{
    struct ItemDerived : public ItemBase
    {
        ItemDerived(int v, ListDerived& p) : ItemBase(v), owner(p) {}
        virtual void f()
        {
            cout << "In ItemDerived::f() for " << value << endl;
            owner.g();
        }
        ListDerived& owner;
    };

    void addSpecial(int v) { items.push_back(new ItemDerived(v, *this)); }

    ListDerived() : destroyed(false) {}
    ~ListDerived() { cout << "In ~ListDerived" << endl; destroyed = true; }
    void g() { cout << "In ListDerived::g(): " << (destroyed ? "dead" : "alive") << endl; }

    bool destroyed;
};

int main()
{
    ListDerived list;
    list.add(1);
    list.addSpecial(2);
    list.iterate();
    list.add(3);
    list.addSpecial(4);
    return 0;
}

(There are a number of known bugs with this code due to it being simplified for this question -- I know it leaks memory and makes too much stuff public, among other things; that's not the point.)

The output of this test program is as follows:

In ItemBase::f() for 1
In ItemDerived::f() for 2
In ListDerived::g(): alive
In ~ListDerived
In ~ListBase
In ItemBase::f() for 3
In ItemDerived::f() for 4
In ListDerived::g(): dead

In particular note that the call to iterate() in the base class destructor has resulted in a call to ListDerived::g() after ~ListDerived() has executed its body but before it has actually exited -- so the ListDerived instance is on the way out but is still partially alive. And note that g() itself is not virtual and not in ListBase's methods.

I suspect that this output relies on UB, so that's the first question: is that the case or is this well-defined (albeit perhaps dodgy style)?

(The behaviour in question is the call to g() on a partially-destroyed ListDerived and its subsequent accessing of its actually-destroyed destroyed member.)

The second question is that if this is not UB simply because destroyed has a trivial destructor, does it become UB if that were something more complex (eg. a shared_ptr) instead?

The third question is (assuming that this is UB), what is a good way to still have the same flow but avoid the UB? I have some constraints on this from the Real Code™:

• It's C++03 code (VS2008), so sadly no C++11 allowed. (Having said that, I'm still interested in hearing if C++11 would improve things in some way.)
• It is acceptable to skip the call to g() once ListDerived's destructor has started executing.
• ListDerived's destructor is not allowed to access anything in items (nor to keep a separate copy of its special items), so it can't mark the item in some way to tell it to avoid the call to g().
• ListDerived itself can't assume it's in a shared_ptr, so can't use shared_from_this.

(There might be more constraints which would complicate an alternate solution that I haven't thought of -- these were inspired by solutions that I considered and rejected while writing this.)

Answer 1

This is my own attempt at resolving this (under the assumption that it was indeed UB), but I'm interested in hearing if I'm wrong and the original code was ok, or if there's a better solution than the below:

struct ListDerived : public ListBase
{
    struct ItemDerived : public ItemBase
    {
        ItemDerived(int v, boost::shared_ptr<ListDerived> const& p)
            : ItemBase(v), owner(p) {}
        virtual void f()
        {
            cout << "In ItemDerived::f() for " << value << endl;
            if (boost::shared_ptr<ListDerived> p = owner.lock())
            {
                p->g();
            }
        }
        boost::weak_ptr<ListDerived> owner;
    };
    struct null_deleter
    {
        void operator()(void const *) const {}
    };

    void addSpecial(int v) { items.push_back(new ItemDerived(v, token)); }

    ListDerived() : destroyed(false) { token.reset(this, null_deleter()); }
    ~ListDerived() { cout << "In ~ListDerived" << endl; destroyed = true; }
    void g() { cout << "In ListDerived::g(): " << (destroyed ? "dead" : "alive") << endl; }

    bool destroyed;
    boost::shared_ptr<ListDerived> token;
};

This introduces token, which exists for the life of the ListDerived instance (sort of like shared_from_this) while not actually owning the instance (hence the null_deleter), but can still be used to make a weak_ptr that the items use to access their parent instead of directly using a reference. The resulting output:

In ItemBase::f() for 1
In ItemDerived::f() for 2
In ListDerived::g(): alive
In ~ListDerived
In ~ListBase
In ItemBase::f() for 3
In ItemDerived::f() for 4

So the first call to g() occurs as expected but the second one is never made because the token has already been destroyed (in ~ListDerived()) before it gets there (in ~ListBase()). I think this is now safe.

(Barring concurrent calls, of course, especially since p isn't an owning pointer inside f(). It's also not safe if ListDerived is copied or moved, but then neither was the original code; pretend that's been blocked via the usual ways.)

destroyed is redundant now, but I left it in to avoid changing the code too much. (Also using boost::shared_ptr due to C++03; feel free to swap in std::tr1::shared_ptr or std::shared_ptr if you have them, it shouldn't make a difference.)

Making a non-owning shared_ptr feels a bit wrong (even if it's covered in the official cookbook) but AFAIK there aren't any other standard types that support lifetime tracking.