QMainWindow and std::shared_ptr (segfault at exit)

[Pippin]

Hello,

I thought I was managing my pointers correctly but it seems that I'm deleting things twice. I have 7 public static member QMainWindows with no parents. I decided to create each of them in a std::shared_ptr so that they are deleted properly when the program exits (so not by Qt). The 7 windows are constructed with std::make_shared in the main function, after the construction of the QApplication instance (which is declared and constructed inside the main function, isn't a pointer and belongs to the stack).

However I still get a segfault when the program ends, so how does it work? Why would my windows get destructed by Qt if they have no parents?

Thanks in advance.

In case I was too vague, here is some code:

main.cpp

int main(int argc, char *argv[])
{
	QApplication app(argc, argv);
	app.setStyleSheet("QRadioButton { font: bold }");
	app.setStyleSheet("QToolButton { background-color: transparent; border: none; }");
	
	WS::Window1 = std::make_shared<QMainWindowSubClass1>();
	WS::Window2 = std::make_shared<QMainWindowSubClass2>();
	WS::Window3 = std::make_shared<QMainWindowSubClass3>();
	WS::Window4 = std::make_shared<QMainWindowSubClass4>();
	WS::Window5 = std::make_shared<QMainWindowSubClass5>();
	WS::Window6 = std::make_shared<QMainWindowSubClass6>();
	WS::Window7 = std::make_shared<QMainWindowSubClass7>();
	
	WS::Window1->show();
	return app.exec();
}

WS.hpp

class WS
{
	public:
	
		static std::shared_ptr<QMainWindowSubClass1>			Window1;
		static std::shared_ptr<QMainWindowSubClass2>			Window2;
		static std::shared_ptr<QMainWindowSubClass3>			Window3;
		static std::shared_ptr<QMainWindowSubClass4>			Window4;
		static std::shared_ptr<QMainWindowSubClass5>			Window5;
		static std::shared_ptr<QMainWindowSubClass6>			Window6;
		static std::shared_ptr<QMainWindowSubClass7>			Window7;
};

[bsomervi]

It looks like these objects are QWidget derivatives, do they get their parent set, directly or indirectly, anywhere. If so then they are probably having their lifetime managed by Qt as well.

Quoted from the QWidget constructor documentation:

"If parent is 0, the new widget becomes a window. If parent is another widget, this widget becomes a child window inside parent. The new widget is deleted when its parent is deleted."

[Pippin]

I'm afraid that each window has nullptr as its parent widget. I just checked that before posting.

[bsomervi]

@Pippin Ah, OK, you didn't say that.

Are you sure the issues is due to double destruction? Is it possible that the real issue is due to your QApplication derivative being deleted before the QWidgets? Having the QWidgets at global scope is not the way QWidgets are supposed to be used, why do you need them at global scope? Why do you need std::shared_ptr, are they somehow shared between threads?

Have you tried changing things so that the QWidget derivatives are created on the stack in the function that instantiates QApplication thus guaranteeing that their lifetime is within the lifetime of the QApplicationobject instance that runs them?

[Pippin]

@bsomervi

First thanks for your replies!

Well I need them to be "global variables", because they are used in several .cpp files. If I use ordinary pointers then I'll have to handle (delete) them myself, which is never a good idea. So I figured using shared pointers was the best/only option. If there are other ways I'm all ears!

[bsomervi]

@Pippin OK, so if you want a pointer to an object created on the stack then simply take its address or possibly pass a reference if, like in this case, the lifetime is guaranteed longer than the client. For example say you have a class client that needs to access one of your MainWindow object instances:

class MyMainWindow
   : public QMainWindow {
// ...
};

class Client {
public:
   explicit Client (MyMainWindow * main_window)  // could be MyMainWindow& main_window
      : main_window_ {main_window}
   {}

   // ...

private:
   MyMainWindow * main_window_;  // could be MyMainWindow& main_window
};
  
int main (int argc, char * argv[]) {
   QApplication application {argc, argv};
   MyMainWindow main_window;
   Client client {&main_window};     // or Client client {main_window};  if references are used
   application.exec ();
   // guaranteed destruction order here is client then main_window then application
 }

If you really want to create the QWidgets on the heap with no parent then use std::scoped_ptr to manage lifetime in the main() function. If the QWidget object instances might get destroyed while a client holds a pointer then use QPointer to hold the pointer as that can be used to check if the QObject derivative pointer it holds is still valid.

Any of the above are better than global scope objects.

[jsulm]

It is even a worse idea to have public static variables! If you have to use them across several cpp files why not just provide a get() method which returns a pointer (no need though to create them via new, just do "return &variable").

[SGaist]

Hi,

To add to @jsulm, why exactly to you need them in several cpp files ? That smells light a huge tight coupling.

[bsomervi]

@SGaist Indeed, I was going to ask why signals and slots were not an option for inter class communication rather than holding pointers or references.

[Pippin]

Thanks everyone for your replies, I'm still a bit confused as to what's best for my project. I wasn't aware of the existence of std::scoped_ptr, so I'll definitely use it if I can't use anything else.

These 7 windows have member functions that show or hide the other windows. For example, WS::Window1 can show WS::Window2 and WS::Window5 if that's what the user wants (they would both appear on top of WS::Window1). So WS::Window1 needs a pointer to WS::Window2 and another pointer to WS::Window5 in its .cpp file.

Same for the other windows, which all show other windows when the user requires it. The user spends his time browsing the 7 windows back and forth. This is why it's much simpler to make all 7 windows global. I'm not sure it's possible/easy to use references (and thus put these windows on the stack rather than on the heap) instead.

What would you guys do?

[SGaist]

Then you should rather have a dedicated object that handles showing the right window at the right moment.

[Pippin]

@SGaist Could you elaborate a bit please? How can I do that without causing any memory deletion issues?

[bsomervi]

@Pippin std::shared_ptr<T> serves two specific purposes, (1) it can be used to extend the lifetime of the heap object instance of T for the owner of the shared_ptr and (2) it does that in a thread safe way so that heap object instances of T can be shared between threads. Note that using std::shared_ptr<T> does not implicitly make the T thread safe, it just makes the sharing mechanism thread safe i.e. the reference count.

There are other subtleties for sharing non-heap objects that involve a custom deletion function but none of the shared_ptr attributes are appropriate for your requirement.

Sorry I have mislead a little, there is no std::scoped_ptr<T>, I used boost::scoped_ptr<T> for so long that I forget it has another name in Standard C++ which is std::unique_ptr<T>. is a much simpler and lightweight class, it simply makes the lifetime of a heap object the same as its own with the exception that you can create the unique_ptr<T> before the T instance if you really want to, also you can explicitly release or change the managed object instance. The other big advantage of std::unique_ptr<T> is that it can be put into a Standard Library container.

[SGaist]

If you take QTabbedWidget, it's a set of widget that you can switch to by clicking on tab.

What allows your user to switch from one window to another ?

[Pippin]

@bsomervi Okay thanks again.

@SGaist Basically a QPushButton that shows another window.

[bsomervi]

@Pippin said:

Same for the other windows, which all show other windows when the user requires it. The user spends his time browsing the 7 windows back and forth. This is why it's much simpler to make all 7 windows global. I'm not sure it's possible/easy to use references (and thus put these windows on the stack rather than on the heap) instead.

Look again at my example, it uses a pointer to a stack object. You are conflating pointer to objects and heap based objects which is wrong, pointers and the heap are different and basically unrelated concepts.

Anyway, listen to what @SGaist is saying as decoupling the relationships between your abstractions (classes) is most important. A good rule of design is to have maximum coupling within an abstraction and minimum coupling between abstractions. That means having classes knowing about each other is usually a bad thing.