Correct way to close a modeless QDialog?

JonB

Qt 5.95, Linux. I am experiencing SIGSEGV under some circumstances when closing a modeless QDialog.

I have played a lot, and it's really tricky. It may be a timing thing. It is (probably) connected to my use of Python/PyQt and the way that works. If I have to describe all this it will be long/complex. For now I want to ask one simple question...

...What is the correct way to close (programmatically) a modeless QDialog from code?

The QDialog is created with no parent and shown via QDialog.show(), so as to be modeless. There are buttons on it which will cause close, in addition to the user being able to click the X button to close.

QDialog can be closed from code either via its own QDialog::done() or via its inherited QWidget::close(). There is some, non-definitive, evidence that it behaves differently (crash or not) for me depending on which method approach I take. Bearing in mind that the dialog is modeless, can someone say which of done() vs close() is the "correct" one to take?

mrjj

Hi
The major difference between QDialog::done() and QWidget::close() is
that done, set return code for dialog and signals are emitted.
Both will delete it of that flag is sat.
So im not really sure what is most correct in terms of just getting rid of it
but since its being a Dialog , im leaning towards done() even if you are not utilizing its signals.

JonB

@mrjj
OK, let me try you, please, on the next question.

I need a signal whenever a (modeless) QDialog gets "closed", whether by user interaction or programmatic close()/done()/whatever.

To my surprise, I cannot find one. The problem with the finished signal is that it seems to come too early for what I need. At some level (because of the SIGSEGV) Qt stuff is still "happening" to the QDialog during finished, and I need to know when it is actually closed.

Do you know of any such? Because I find I am in agreement with the answer in https://stackoverflow.com/questions/43756898/what-is-the-signal-emitted-when-we-close-a-window-in-qt where it states:

There is no signal emitted when widgets (including QMainWindows) are closed.

I had already changed over to having to use destroyed because the modeless happens to have Qt::WA_DeleteOnClose flag set, but I don't much like that dependency. The alternative of overriding the modeless dialog's closeEvent requires changes in the modeless dialog's code, whereas I wanted only to have to make changes in the opener's code (by catching existing signals).

Do you know any better?

Kent-Dorfman

@JonB said in Correct way to close a modeless QDialog?:

I need a signal whenever a (modeless) QDialog gets "closed", whether by user interaction or programmatic close()/done()/whatever.

void QWidget::closeEvent(QCloseEvent *event)

Then subclass the dialog, and override the closeEvent() method. emit your custom signal AFTER doing the default closeEvent() code.

JonB

@Kent-Dorfman
But I explicitly wrote above:

The alternative of overriding the modeless dialog's closeEvent requires changes in the modeless dialog's code, whereas I wanted only to have to make changes in the opener's code (by catching existing signals).

Your code seems to be 100% what is the alternative given in the stackoverflow link (https://stackoverflow.com/a/43757266), or am I misunderstanding something?

Kent-Dorfman

what you are proposing wont work. subclassing is the only reliable way to have complete and exslusive control, but no...you are not modifying the dialog. You are creating a specialization that doesn't modify the existing form, and then refering to that specialization throughout your project. Yeah, you're going to have to do more coding than you would like..

JonB

@Kent-Dorfman

you are not modifying the dialog

Huh? To do this any modeless existing dialogs must be subclassed --- that is altering the existing dialog code, is it not?! What if I do not have control of that code? Another team designs the modeless dialog, I am just trying to detect, in my code which calls dialog.show(), when that dialog gets closed. I expected that to be an easy, existing signal, but it seems there is nothing for this, which I find pretty surprising, hence I was asking....

Kent-Dorfman

@JonB said in Correct way to close a modeless QDialog?:

Huh? To do this any modeless existing dialogs must be subclassed --- that is altering the existing dialog code, is it not?!

NO NO NO
When you subclass anything you don't touch the existing code. you create another class, using the original as its parent.

class my_dialog: public QFileDialog { }

This in no way MODIFIES the existing QFileDilaog class. It only changes the behaviour, where ALLOWED.

Override closeEvent() in my_dialog and then change all references of QFileDialog to my_dialog where they exist in the main code.

JonB

@Kent-Dorfman
HI Kent. I do understand subclassing. We are having a difference over what parts of code you want me to alter that I do not wish to alter to achieve my objective. Since you seem interested:

Let's just take your example first:

class my_dialog: public QFileDialog { }

But if my existing code has loads of QFileDialog f = new QFileDialog() dotted around, and I want to add detecting dialog close as a feature, I have to alter loads of existing new QFileDialog() over to new my_dialog(). I didn't want to have to do that.

Of course, all Qt classes are already subclassed in the app:

class AppFileDialog: public QFileDialog { }

so the app always goes via new AppFileDialog(). So AppFileDialog would be ideal for introducing the closeEvent override I would like. But that is in modules I do not want to change. This is what I meant by "that is altering the existing dialog code".

Now, in the current case the current derivation is QDialog -> JDialog (generic app dialog) -> individual classes for each specific dialog, in their own modules. Again, in this case I do not wish to alter the generic JDialog to add closeEvent override.

The outside world hands me an instance of an already-created dialog,

class SomeModelessDialog: public JDialog{ }

someInstance = new SomeModelessDialog();

In my code I want to detect when that instance closes. You need me to

either add to the existing SomeModelessDialog or JDialog code, which would be simplest but not available,
or you want me to define a new subclass of my own to add closeEvent and then change where someInstance is newed:

class KentSomeModelessDialog: public SomeModelessDialog{ override closeEvent() {} }

someInstance = new KentSomeModelessDialog();

I didn't want to change either the dialog's existing class or where it was created.

In summary: you are right, I could subclass the top-level (rather than the intermediate-level because I don't want to touch that code) modeless dialogs to add closeEvent, but that will require changing where the dialog gets created via new. And that is far, far away from the code which wants to know when an instance gets closed.

That's why I wanted to find a signal from QDialog I could hook onto for closure. (Bear in mind, I've said QDialog::finished is too early for me.) For now I am using QDialog::destroyed because our dialogs have Qt::WA_DeleteOnClose flag set, though I don't much like that dependency.

J.Hilk

@JonB

I‘m not familiar with the python side of Qt,

But since it‘s a wraper around the c++ part I would guess you can specify in your connection setup the connection type.

By default this is on auto, that defaults by same thread connections to a DirectConnection,

if you explicitly connect your slot with a QueuedConnection, your slot should be executed well after all the cleanup.

Maybe that‘s already enough for your case ?

JonB

@J.Hilk
Hi, I have no idea what you mean?!

I want a signal when a dialog gets closed. There doesn't seem to be one. I don't see any connection to what you have written about connections? :)

Kent-Dorfman

I've told you what you need to do...good luck with that

J.Hilk

@JonB
you said the finished signal comes slighlty to early for your purpose,

so my suggestion connect your slot to finished with the QueuedConnecten argument
https://doc.qt.io/qt-5/qt.html#ConnectionType-enum

and from prevoius topics I assumed, you’re still doing pyqt work, but I might be mistaken here 🙈

SGaist

Hi,

Does it happen if you use a dummy dialog ?
What are you doing in it ?

JonB

@J.Hilk
Ah, OK, I get your drift now.

@SGaist
I warned it gets complicated! The modeless dialog hosts a QWebEnginePage. The SIGSEGV backtrace shows it's somewhere in destructors from QWebEnginePage/View stuff

You've both asked, so: what I have here is a Python/PyQt problem. As briefly as possible: I need to create and show a modeless dialog, with no parent ("fully" modeless), via QDialog.show(). In Python/PyQt I must maintain a reference to such a dialog --- if I don't assign it to a "global variable" to keep a reference Python/PyQt will garbage collect and immediately destroy the unreferenced dialog. I keep that in a singleton class.

When the dialog closes this reference must be cleared, to release the Python reference. So I need a reliable signal to tell me when it gets closed. Outline:

singleton.modelessDialog = QDialog(parent=None)
singleton.modelessDialog.finished.connect(singleton_modeless_closed)
singleton.modelessDialog.show()

def singleton_modeless_closed()
    singleton.modelessDialog = None

From the signals available to me, the best I could see is finished. But when I set the reference to None in that slot, as shown above, there is a (later?) SIGSEGV in the modeless dialog, down in QWebEnginePage destructors. I'm releasing the Python reference during the finished signal, I'm guessing that destroys stuff while it's still in use or whatever.

So I wanted a signal which comes later, when the dialog gets closed. I expected a closed signal, but it appears Qt does not supply one. The stackoverflow post, and @Kent-Dorfman, seem to be confirming this.

If you know better, or a better way I can achieve this using Python, please explain. I think the crashing is a Python behaviour issue, you probably wouldn't get it in C++. You could probably go:

singleton.modelessDialog.deleteLater();
singleton.modelessDialog = nullptr;

SGaist

What about keep that dialog as a class variable ? That should keep it alive and then you can try to enable the Qt. WA_DeleteOnClose widget attribute.

Hope it helps

JonB

@SGaist
It happens to be a singleton variable, same effect as a class variable. It's not that --- it's when I set it to None that is problematic.

We have come full-circle! I am currently able to use Qt.WA_DeleteOnClose on the dialog so can connect to QWidget.destroyed, instead of QDialog.finished, as the signal to indicate when I can clear the reference. That does not produce the SIGSEGV. But relies on Qt.WA_DeleteOnClose, which can be good or bad for dialogs.

JonB

@Kent-Dorfman said in Correct way to close a modeless QDialog?:

@JonB said in Correct way to close a modeless QDialog?:

I need a signal whenever a (modeless) QDialog gets "closed", whether by user interaction or programmatic close()/done()/whatever.
void QWidget::closeEvent(QCloseEvent *event)
Then subclass the dialog, and override the closeEvent() method. emit your custom signal AFTER doing the default closeEvent() code.

As I said, I need to know when the dialog is "closed" regardless of how. I have spent ages only to discover that closeEvent does not get emitted if code calls QDialog::accept/reject/done(), non-intuitive and non-documented... :( Further, it does not get called if the user clicks Escape to close, though it does get called if user clicks the X to close, or if code calls close() explicitly, only those two cases. Crazy.

Other than working with the destroyed signal I'm finding it unreliable/hard to detect QDialog closure...?

Kent-Dorfman

I did some checking and you are right about closeEvent(). It doesn't always get called. However, I did a short test program and the virtual void hideEvent() always seems to be called when anything derived from the QWidget base class closes (immediately after the window system closes the window).

class MyDialog: public QFileDialog {
    
    Q_OBJECT
    public:
    MyDialog();
    
    void hideEvent(QHideEvent* evt);
};

MyDialog::MyDialog(): QFileDialog() {}
    
void MyDialog::hideEvent(QHideEvent* evt) {
    std::cout << "hideEvent()" << std::endl;
    QFileDialog::hideEvent(evt);
        
}

JonB

@Kent-Dorfman
You are kind to look at this, but I'm afraid no cigar :( This time I think hideEvent is way too often for what I want!

Note: A widget receives spontaneous show and hide events when its mapping status is changed by the window system, e.g. a spontaneous hide event when the user minimizes the window

Because I am Python/PyQt, I need to know when I can release the global reference I must keep to a shown modeless dialog. If I do it too early I will "crash" because the dialog is still in use; if I do it too late/miss the event I will not free the resource.

This is proving surprisingly difficult :( If you are in C++ it corresponds to:

globalModeless = new QDialog(nullptr);
globalModeless.show();
...

Now the question is: when exactly are you going to call delete globalModeless, in response to the user/code permanently closing the dialog, by any means? Assume that you are not using Qt::WA_DeleteOnClose flag .