Smart pointers and parented QObjects

[jason.gibbs@x-navtech.com]

We would like to combine the benefits of a smart pointer, we don't care which one(s), and parented QObjects with lifespans shorter than their parents. Specifically, if a parent QObject has a member smart pointer to a child, and that child may get destroyed and recreated several times through the parent's existence. It would be great to use a smart pointer for the child, as a developer wouldn't leak memory accidentally - at least for the duration of the life of the parent - if they failed to check the member's existence before creating a new instance.

We run into an issue with smart pointers (specifically std::shared_ptr) during the Parent's destruction. This can lead to a segfault. We suspect that the child is destroyed by its parent because it is owned by the parent and then when the shared_ptr goes out of scope destruction tries to happen again, although the order of this is somewhat suspect to us. We would think that the shared_ptr would go out of scope first as the Parent's destructor is called, which would in turn call delete on the owned object and then the base class QObject's destructor would be called, which would try and delete the Child (who it isn't aware was deleted by the shared_ptr, for some reason). Some searching of various forums indicates that smart pointers shouldn't be mixed with parented objects for this reason.

Here's a crash stack trace for a parented QTimer that was wrapped in a smart pointer.


crashlog.20160223-141028.3295:*** SIGSEGV (@0x7f4d4c4afbb8) received by PID 3295 (TID 0x7f4d823d8a80) from PID 1279982520; stack trace: ***
crashlog.20160223-141028.3295- @ 0x7f4d793b6d40 (unknown)
crashlog.20160223-141028.3295- @ 0x7f4d7a9d395f QObject::killTimer()
crashlog.20160223-141028.3295- @ 0x7f4d7a9d8e01 QTimer::stop()
crashlog.20160223-141028.3295- @ 0x7f4d7a9d8e91 QTimer::~QTimer()
crashlog.20160223-141028.3295- @ 0x7f4d80f787c9 std::shared_ptr<>::~shared_ptr()

Regardless of our understanding of the crash, we would prefer to know the correct usage (if any) for mixing smart pointers and parented QObjects. We realize that in many cases parenting a QObject is unnecessary if a smart pointer is used, such as the above QTimer, but there are examples when the Qt parenting scheme is quite useful such as UI elements.

[d_stranz]

we would prefer to know the correct usage (if any) for mixing smart pointers and parented QObjects

The only way you can do this is using Qt's built-in QPointer. This is designed for storing QObject pointers and has the semantics that the wrapped pointer is automatically set to null when the pointer goes out of scope. QPointers are weak, as opposed to strong boost:: or std:: shared_ptrs. I am not certain of the internal implementation, but QPointer likely uses QObject's destroyed() signal to keep track of the pointer's lifetime.

Of course, this begs the question of why you would want to dynamically create and destroy child objects in the first place. If these are QWidget instances, the usual convention is to either create them on the stack (as is common with QDialog), or to show() / hide() them on demand.

[ars]

@op: Just out of curiosity: can you give us a use case for this? I'm afraid using STL or boost shared/unique pointers will cause more trouble than benefit. Using QObject parent/child relationship and one of the strong pointers on the same object instance you have two mechanisms controlling object life time that do not know of each other. This will sooner or later lead to crashes that are hard to track. As d_stranz already suggested you might better go with Qt smart pointers. However, QPointer might not be what you want. You may also consider QSharedPointer, which, by its documentation, seems to be a counterpart of std::shared_ptr which is aware of the special needs of Qt. Here's a small example showing the difference between QPointer and QSharedPointer. MyClass is a simple QObject derived class logging its creation and deletion.

#include "MyClass.h"

#include <iostream>

MyClass::MyClass(QObject *parent) :
QObject(parent)
{
std::cout << "constructing MyClass object " << this << std::endl;
}

MyClass::~MyClass()
{
std::cout << "deleting MyClass object " << this << std::endl;
}

The MainWindow CTOR looks like

#include "MainWindow.h"
#include "ui_MainWindow.h"

#include "MyClass.h"

#include <QPointer>
#include <QSharedPointer>

#include <iostream>

MainWindow::MainWindow(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::MainWindow)
{
ui->setupUi(this);
std::cout << "MainWindow::CTOR begin" << std::endl;
{
std::cout << "creating QPointer" << std::endl;
QPointer<MyClass> p = new MyClass(this);
std::cout << "creating QSharedPointer" << std::endl;
QSharedPointer<MyClass> sp(new MyClass(this));
}
std::cout << "MainWindow::CTOR end" << std::endl;
}
When running the program I get
MainWindow::CTOR begin
creating QPointer
constructing MyClass object 0x13defe0
creating QSharedPointer
constructing MyClass object 0x110f780
deleting MyClass object 0x110f780
MainWindow::CTOR end
The first MyClass object pointer 0x13defe0 is assigned to the QPointer instance, the second one (0x110f780) to QSharedPointer instance. Immediately after construction both pointers go out of scope. Only the MyClass object controlled by the QSharedPointer gets deleted.

Depending on your use case that might be a better choice.

Best regards
ars

[yeye_olive]

We would like to combine the benefits of a smart pointer, we don't care which one(s), and parented QObjects with lifespans shorter than their parents. Specifically, if a parent QObject has a member smart pointer to a child, and that child may get destroyed and recreated several times through the parent's existence. It would be great to use a smart pointer for the child, as a developer wouldn't leak memory accidentally - at least for the duration of the life of the parent - if they failed to check the member's existence before creating a new instance.
(emphasis is mine)
Then you shouldn't use reference-counted smart pointers such as std::shared_ptr or QSharedPointer, because they precisely allow objects to escape a scope. The parent QObject is already managing the child's lifetime; everyone else can just manipulate a plain Child * pointer without worrying when to delete it (but must still be careful not to access the child after the parent has been destroyed).

I suggest one of those two options:

store the pointer to the child in a plain Child * private member, and add a helper method void resetChild(Child *newChild) that deletes the existing child, if any (or calls deleteLater(), depending on your needs), before setting the new pointer. Explain to the developers that they must go through this method to set up a new child.
if that is not enough (i.e. some developers keep modifying the Child * member directly), wrap the Child * in a smart pointer that forces updates to go through the method. std::unique_ptr (or QScopedPointer) do the trick, except that the parent's destructor must take care of calling unique_ptr::release() (or QScopedPointer::take()) to avoid double deletion. Or just implement a minimal ad hoc smart pointer yourself, which only initializes the Child * to NULL and forces updates to delete the existing child first.

[jason.gibbs@x-navtech.com]

Thank you all for your feedback. There were a few questions about why we would like to do this. Here's an example we ran across that prompted this discussion in our group. Suppose there is a dialog with the following properties:



it is complex and easier to construct from scratch when it is used than manage update/reset rules
it is modeless
we never want multiple instances of this dialog displayed


We want this dialog to be recreated whenever a particular slot in the parent is called and either deleted-on-closed or possibly deleted by a separate slot call.

Essentially this lead to our thinking that it would be fantastic if we could get all the benefits of the parent/child UI relationship without mandatory ownership by the parent. Some "emancipated minor" option to ask "hey, Parent- can you teach me where i fit in the UI world but not be in charge of my life after that?" A decoupling of object ownership and UI parenting benefits.




store the pointer to the child in a plain Child * private member, and add a helper method void resetChild(Child *newChild) that deletes the existing child, if any (or calls deleteLater(), depending on your needs), before setting the new pointer. Explain to the developers that they must go through this method to set up a new child.
if that is not enough (i.e. some developers keep modifying the Child * member directly), wrap the Child * in a smart pointer that forces updates to go through the method. std::unique_ptr (or QScopedPointer) do the trick, except that the parent's destructor must take care of calling unique_ptr::release() (or QScopedPointer::take()) to avoid double deletion. Or just implement a minimal ad hoc smart pointer yourself, which only initializes the Child * to NULL and forces updates to delete the existing child first.


Agreed - both are ways to accomplish this task. We were trying to find a way to use a more C++11(and beyond) approach to handling this situation - especially as we bring new developers into the fold. Our goal is to eliminate "delete" in our codebase if possible.

We really appreciate the time and effort put in to the replies. Thank you so much!

[d_stranz]

it is complex and easier to construct from scratch when it is used than manage update/reset rules
it is modeless
we never want multiple instances of this dialog displayed




This is puzzling. Whatever you do upon construction could be factored into an initialization method called from the constructor or a reset() method.
That just makes it a little awkward to use a stack-based instance to control lifetime
So what happens if the dialog already is on display? Does executing the action that invokes it do nothing, thus causing the user to try to execute it again in frustration? Does the existing dialog get moved to the top with whatever contents it currently contains, thus confusing the user? Does the existing dialog vanish only to get replaced with a clean new version, thus annoying the user who was editing or observing something when the action invoking the new dialog was executed from behind the scenes somehow?


To me, this says that you are trying to wrap smart pointer functionality around something that should instead be controlled by business logic. The business logic should determine that the action(s) which can invoke the dialog must be disabled if the dialog is already in existence. If your developers are too undisciplined to abide by the business rules, then code review and testing will uncover the violation.

You could implement your dialog as a singleton with a private constructor and an instance() method. That could ensure that only one is in existence, but it wouldn't help with the issues I raised in (3) above. It would help with the update/reset issue, but only if your singleton was constructed in a way that deleted the instance upon closure of the dialog.