CODEWITHSUNDEEP
c++shared-ptrweak-ptr
xubury
xubury

Reputation: 104

How to fix this shared_ptr reference cycles?

I designed an App that holds a stack of layers and an active obj. When a Layer is attached to the App, the Layer tells App what an active object is. But my design causes a sigtrap when deallocating.

It cause sigtrap because the destruction of shared_ptr<Obj> m_obj in App happens first which reduce the use_count to 1. Then the onDetech function gets call, setting the active shared_ptr<Obj> m_obj to nullptr and reduce use_count to 0! But the layer still holds an shared_ptr<Obj>.

The code below is a minimal example to reproduce. I notice my code has a reference cycle but I have no idea how to fix this except using a raw pointer for obj in the App class.

I used shared_ptr for obj in App because it makes sense to me that App has the shared ownership of obj. Should I not use shared_ptr in this case?

class App;

class Obj {
   public:
    ~Obj() { std::cout << "obj destruct" << std::endl; }
};

class Layer {
   public:
    Layer() { m_obj = std::make_shared<Obj>(); }

    ~Layer() { std::cout << m_obj.use_count() << std::endl; }

    void onAttach(App *app);

    void onDetach();

    std::shared_ptr<Obj> m_obj;

   private:
    App *m_app;
};

class LayerStack {
   public:
    void pushLayer(App *app, std::shared_ptr<Layer> layer) {
        m_layers.push_back(layer);
        layer->onAttach(app);
    }

    ~LayerStack() {
        for (auto &layer : m_layers) {
            layer->onDetach();
        }
    }

   private:
    std::vector<std::shared_ptr<Layer>> m_layers;
};

class App {
   public:
    App() {
        m_defaultLayer = std::make_shared<Layer>();
        m_stack.pushLayer(this, m_defaultLayer);
    }

    ~App() {}
    LayerStack m_stack;
    std::shared_ptr<Layer> m_defaultLayer;
    std::shared_ptr<Obj> m_activeObj;
};

void Layer::onAttach(App *app) {
    m_app = app;
    app->m_activeObj = m_obj;
    std::cout << m_obj.use_count() << std::endl;
}

void Layer::onDetach() {
    m_app->m_activeObj = nullptr;
    std::cout << m_obj.use_count() << std::endl;
}


int main() {
 A a;
}

output:

2
obj destruct
-923414512
-923414512

Upvotes: 0

Views: 193

Answers (1)

Miles Budnek
Miles Budnek

Reputation: 30619

You're accessing m_activeObj after its lifetime has ended, and thus the behavior of your program is undefined.

The sequence of events is as follows:

  1. App object goes out of scope
  2. ~App runs
  3. m_activeObj is destroyed; after this its lifetime has ended and it can no longer be accessed
  4. m_defaultLayer is destroyed
  5. m_stack is destroyed
    1. m_layers[0].onDetach() is called
    2. onDetach sets m_app->m_activeObj to nullptr, but its lifetime has already ended, so behavior is undefined.
  6. Irrelevant other stuff; you're already screwed.

The solution is to reorder things so that you don't access m_activeObj after its lifetime has ended. Either move m_stack's declaration after m_activeObj so it gets destroyed first or clear it manually in ~App.

Upvotes: 2

Related Questions