Breaking a circular dependency between a shared_ptr and a unique_ptr

Question

Given this code:

#include <iostream>
#include <memory>

class Controller;

class View {
public:
    ~View() {
        std::cout << "Disposing View" << std::endl;
    }

    void SetObserver(std::shared_ptr<Controller> a_observer) {
        observer = a_observer;
    }
private:
    std::shared_ptr<Controller> observer;
};

class Controller : public std::enable_shared_from_this<Controller> {
public:
    static std::shared_ptr<Controller> Create(std::unique_ptr<View> view) {
        //Can't use std::make_shared due to visibility rules :(
        auto controller = std::shared_ptr<Controller>(new Controller(std::move(view)));

        controller->Init();

        return controller;
    }

    ~Controller() {
        std::cout << "Disposing Controller" << std::endl;
    }
private:
    std::unique_ptr<View> view;

    explicit Controller(std::unique_ptr<View> a_view) : view(std::move(a_view)) {}

    Controller(const Controller&) = delete;

    void Init() {
        view->SetObserver(shared_from_this());
    }
};

int main() {
    auto view = std::make_unique<View>();

    auto controller = Controller::Create(std::move(view));

    return 0;
}

I think that the controller object will never be disposed (confirmed by running it).

In order to alleviate this issue, is it sufficient to make the observer variable a weak_ptrinstead of a shared_ptr?

Besides this, is there any other potential issue that I should be aware of, given the above design?

Answer 1

Yes, as you state about std::weak_ptr:

In addition, std::weak_ptr is used to break circular references of std::shared_ptr.

Changing the member to std::weak_ptr and running, yields

$ ./a.out 
Disposing Controller
Disposing View

When you need it, just call lock (checking the return value), to obtain a std::shared_ptr (which you should not store as a member):

void doSomethingWithView() {
    auto obs = observer.lock();                                                                                                                                                                          
    if(obs) {
        // Still valid
    }   
}   

---

A possible caveat has to do with std::weak_ptr and multithreading.