Search code examples
c++qtshared-ptr

std::shared_ptr deleted in a getter function with Qt


I'm using shared pointers for my Qt project, but when I call setLayout() through a shared pointer I get a segmentation fault.

I'm calling it like this:

 model->getBody()->setLayout(layoutTemplate.get());

The function getBody() returns a std::shared_ptr:

std::shared_ptr<QWidget> MainTemplate::getBody()
{
   return std::shared_ptr<QWidget>(ui->body);
}

I tried using QSharedPointer instead of std::shared_ptr, but that gave me the same result.

Why is my pointer deleted?


Solution

  • Smart pointers represent ownership of objects. shared_ptr in particular represents an object that is owned by multiple entities and has to live until everyone is done using it. Whenever the last shared_ptr pointing to an object is destroyed, the object it points to is also destroyed. The proper use of shared_ptr is:

    std::shared_ptr<T> ptr_a(new T); // Create a T and immediately pass it to shared_ptr
    std::shared_ptr<T> ptr_b = std::make_shared<T>(); // Same effect but more efficient
    
    ptr_a = ptr_b; // smart pointer to smart pointer assignment (never existing raw to smart)
    ptr_a.reset(new T);            // This is fine.
    ptr_a = std::make_shared<T>(); // Same as previous line. Again more efficient
    
    //... eventually ptr_a and ptr_b go out of scope and handle deleting their objects
    

    Note that in all the examples of reassignment, ptr_a checked the reference count and deleted the held object if it was the last shared_ptr pointing to it.

    Generally, you only ever want to pass raw pointers to smart pointers at the exact moment of their creation. What you did is pass ownership of an object to a temporary smart pointer. After the statement, the temporary shared_ptr was destroyed and as no copy of it was ever made, it assumed it should destroy ui->body. Generally what you want to do is simply return body by non-const reference, or just not return it and provide setter functions for its parameters.

    QWidget& MainTemplate::getBody()
    {
       return ui->body;
    }
    

    If however, some reason dictates that body needs to be able to outlive its class (doubtful, unless questionable design decisions have been made), you need to make body itself a shared_ptr and return it by value. This will create copies of shared_ptr and you are guaranteed that the last one of them will destroy the object. But you must NEVER use the raw pointer after body's construction and you must not manually try to delete it.

    UI {    // whatever class MainTemplate::ui points to
    private:
        std::shared_ptr<<QWidget> body;
    ...
    }
    
    MainTemplate {
    public:
        std::shared_ptr<QWidget> MainTemplate::getBody()
        {
            return ui->body;
        }
    }
    

    Again, I don't think this is needed if your classes have been defined in a practical way. Also, you could make ui a shared_ptr if it does not belong to a class, and return that instead of its body. Then, let the caller access body through a returned shared_ptr<UI>.