3

My pet project has come to the point where I should start tracking pointer lifetime, and I'm trying to develop some system for it. Sadly, the popular advice to use smart pointers everywhere does not apply since Qt API itself uses naked pointers on every occasion. So, what I came up with is this:

  1. For everything owned by Qt,

    • use pointers naked locally;
    • pass them between functions naked too;
    • store them as a subclassed QPointer that makes isNull() check before conversion to naked.
  2. For everything cleanly owned by me, use smart pointers as advised. I'm going to go with the std:: versions here.

  3. The case that bothers me. For objects that switch ownership (like widgets added/removed from a layout)

    • use, store, pass pointers naked;
    • delete them manually when appropriate.

Suggestion, comments, advice? I don't like this scheme much myself.

sigil
  • 815
  • 8
  • 16
  • just use the `std::` smart pointers, what's the problem with them? slight maintenance? it will pay off.. – Abhinav Gauniyal Feb 05 '17 at 16:36
  • @Abhinav Gauniyal Which one of the `std::` smart pointers would you suggest for the case when I 1) create a widget, 2) pass it to a layout and it becomes owned by Qt, 3) *possibly* remove it from the layout later depending on user actions? Deleting something owned by Qt is a crash. – sigil Feb 05 '17 at 19:14
  • "Deleting something owned by Qt is a crash." well, no. Not in general, that is. Suppose you've got a widget which is a child of another widget: you can delete that child directly, even if it's "owned by Qt" (through the parent widget). I'd therefore like more clarifications about the case that bothers you and how exactly you're failing at finding a solution for it. – peppe Feb 05 '17 at 21:00
  • 1
    @peppe Can't agree with you, sorry. This was how I came to the realization that I need a policy for these things: investigating a crash, I realized that I delete a pointer to a widget I thought Qt had released ownership of but in fact it had not yet. After I moved the `delete` line to a later event, the crash was gone. Maybe there are some cases when this rule is relaxed, but developing a specialized solution every time does not seem overly scalable. – sigil Feb 06 '17 at 10:35
  • @peppe, see [this](https://stackoverflow.com/questions/59634605/using-stdunique-ptr-on-qt) regarding the using of smart pointers in qt – LRDPRDX Jan 19 '23 at 09:47
  • "I realized that I delete a pointer to a widget I thought Qt had released ownership of but in fact it had not yet. After I moved the delete line to a later event, the crash was gone." This sounds like you were stumbling on a bug in Qt. Qt doesn't "release ownership" of widgets in the broader sense of the term. – peppe Jan 19 '23 at 11:19
  • @LRDPRDX: the answer doesn't say much except that you can't mix and match. Which is also what I was asking for: "I'd therefore like more clarifications about the case that bothers you". – peppe Jan 19 '23 at 11:20

2 Answers2

5

First, hold things by value where you can. View each use of new, make_unique and make_shared with suspicion - you must justify each dynamic object creation. If a sub-object has the same lifetime as the parent, holding by value is a no-brainer. For example:

class MyWidget : public QWidget {
  Q_OBJECT
  QGridLayout m_topLayout{this};
  QLabel m_sign{"Hello World"};
public:
  MyWidget(QWidget * parent = nullptr) : QWidget{parent} {
    m_topLayout.addWidget(&m_sign, 0, 0);
  }
};

You're passing pointers around, but object ownership is clear and there's no change of ownership. Just because a QObject has a parent doesn't mean that the parent "owns" it. If the child is destructed before the parent, the ownership ceases. By using C++ semantics - namely the well-defined order of member construction and destruction - you have full control over child lifetimes and no QObject parent gets to interfere.

If you have non-movable objects that have one owner, use std::unique_ptr and move it around. That's the way to pass dynamically created QObjects around your own code. You can remove them from the pointer at the point where you make their ownership managed by a QObject parent, if there is such.

If you have objects with shared ownership, where their life should end as soon as possible (vs. when the application terminates, or some long-lived object gets destroyed), use std::shared_ptr. Ensure that the pointer outlives the users. For example:

class MyData : public QAbstractItemModel { /* ... */ };

class UserWindow : public QWidget {
  Q_OBJECT
  std::shared_ptr<MyData> m_data; // guaranteed to outlive the view
  QTreeView m_view;
public:
  void setData(std::shared_ptr<MyData> && data) {
    m_data = std::move(data);
    m_view.setModel(m_data.data());
  }
};

This example is perhaps contrived, since in Qt most users of objects watch the object's destroyed() signal and react to the objects destruction. But this makes sense if e.g. m_view was a third-party C API object handle that had no way of tracking the data object's lifetime.

If the object's ownership is shared across threads, then the use of std::shared_ptr is essential: the destroyed() signal is only usable within a single thread. By the time you get informed about object deletion in another thread, it's too late: the object has been already destroyed.

Thirdly, when you return instances of dynamically created objects from factory methods, you should return them by a naked pointer: it's clear that a factory creates an object for someone else to manage. If you need exception safety, you can return a std::unique_ptr instead.

Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313
  • I bookmarked your awesome answer for future reference as my project progresses. Also, I think I'm going to start tracking your posts. – sigil Feb 07 '17 at 16:20
  • The approach is only legal if nothing outside of this window would be connected by signal-slot system with nested elements. Otherwise signal system may be able to fire slot for destroyed object if window was destroyed during signal dispatch. – Swift - Friday Pie Aug 07 '20 at 12:39
  • If you have code paths that do this then it's a bug, and I'd like you to post at least a stack trace in reply to that. Otherwise, Qt goes to a lots of trouble to forget about destroyed objects as soon as they are destroyed. Ergo, it's not able to "fire a slot for a destroyed object", because as far as Qt's internal data structures are concerned, the dangling pointer to the slot-object is not present. – Kuba hasn't forgotten Monica Aug 11 '20 at 06:37
1

For starter, in Rome, be Roman.
QT was developed in the very early 90's and it was a great success at a time.
unfortunately, QT didn't really adopted new features as the time went by so the API itself has very old C++ style to it (and may I say, a Java style to it?)

you can't force QT to suddenly be C++14 because it's not. use the popular-QT conventions when it comes to QT. use raw pointer if that was the platform design goal. use value types when you can't.

But I don't think you make QT work that much with C++14. stick the QT idioms as they given by the platform.

David Haim
  • 25,446
  • 3
  • 44
  • 78
  • 9
    As far as I remember, the Roman Empire kept leaking memory, though, and eventually segfaulted. – Kerrek SB Feb 05 '17 at 15:54
  • @David Haim Could you please point me to a list of Qt idioms you mentioned? I tried looking for exactly that before asking my question, yet all Google has to offer is a single .pdf presentation - fragmentary, sketchy and not terribly useful. – sigil Feb 05 '17 at 16:10
  • @KerrekSB - there is nothing wrong with raw pointers, it is how you use them. – dtech Feb 05 '17 at 17:46
  • 3
    @DavidHaim it is Qt, not QT. Also, each new sentence starts with a capital letter. Lastly, it is "when in Rome, do as the Romans do". And finally, there is nothing preventing you from using std smart pointers in your part of the application. Qt couldn't possibly have adopted those features as they came too late, so addopting them now that the standard C++ finally decided to introduce them was not an option. That's hardly Qt's fault, the framework is far from perfect, but the lack of smart pointers in its core is the least of its flaws. It's a bad idea to give advice outside your competence. – dtech Feb 05 '17 at 17:49