[Development] Opinions on QTBUG-71545

[Konstantin Shegunov]

Hello,
Since we couldn't agree, I'd love to see some more opinions about this
one.[1]

Specifically:
1) Is parenting to the application object a thing?
1.a) ... and should it be allowed (i.e. accepting the proposed change)?
1.b) .. if not allowed, should we put a warning in the documentation that
it is wrong and shouldn't be done at all, or at least that it's discouraged.

[1] https://bugreports.qt.io/browse/QTBUG-71545

[Elvis Stansvik]

Post by Konstantin Shegunov
Hello,
Since we couldn't agree, I'd love to see some more opinions about this one.[1]

I may be missing some detail, but I think what Thiago says makes
sense. When children are destroyed, you know you're in the QObject
destructor (from QObject::~QObject docs: "Destroys the object,
deleting all its child objects."), so you know the object is now a
QObject, no longer a QCoreApplication. If you require your
QCoreApplication to be alive by the time your child object is
destroyed, I think you have to ensure this on your own.

Like I said, I may be missing something, but that's what it looks like
to me. I can't see why there would be an exception to the object model
here.

Elvis

Post by Konstantin Shegunov
1) Is parenting to the application object a thing?
1.a) ... and should it be allowed (i.e. accepting the proposed change)?
1.b) .. if not allowed, should we put a warning in the documentation that it is wrong and shouldn't be done at all, or at least that it's discouraged.
[1] https://bugreports.qt.io/browse/QTBUG-71545
_______________________________________________
Development mailing list
http://lists.qt-project.org/mailman/listinfo/development

[André Somers]

Hi,

Post by Elvis Stansvik

Post by Konstantin Shegunov
Hello,
Since we couldn't agree, I'd love to see some more opinions about this one.[1]

I may be missing some detail, but I think what Thiago says makes
sense. When children are destroyed, you know you're in the QObject
destructor (from QObject::~QObject docs: "Destroys the object,
deleting all its child objects."), so you know the object is now a
QObject, no longer a QCoreApplication. If you require your
QCoreApplication to be alive by the time your child object is
destroyed, I think you have to ensure this on your own.

Problem is, I think, that this requirement is not always obvious. For
your own objects, you know you cannot rely on your parent still being a
MyClass iso of just a QObject on destruction (unless you take specific
measures to make that so), but in this case the reliance on there still
being a Q*Application around (not necessarily the parent) is usually not
as obvious as a myParent->doSomethingNotFromQObject call in your
destructor code...

Post by Elvis Stansvik
Like I said, I may be missing something, but that's what it looks like
to me. I can't see why there would be an exception to the object model
here.
Elvis

Post by Konstantin Shegunov
1) Is parenting to the application object a thing?

Yes. But you know that the same goes as for any QObject parent/child
relationship: the parent is a QObject at the time of destruction (ok,
with QWidget, you're in luck).

Post by Elvis Stansvik

Post by Konstantin Shegunov
1.a) ... and should it be allowed (i.e. accepting the proposed change)?

Yes, it should be allowed, and as you argued I think it is useful. But I
am not sure that implies the proposed change. OTOH, as so much
functionality in Qt requires the Q*Application to be alive (and that is
not always obvious) , perhaps an exception *is* in order.

Post by Elvis Stansvik

Post by Konstantin Shegunov
1.b) .. if not allowed, should we put a warning in the documentation that it is wrong and shouldn't be done at all, or at least that it's discouraged.
[1] https://bugreports.qt.io/browse/QTBUG-71545

I do think it makes sense to be able to do this, but if that is to be
discouraged, then best be explicit about that.

André

[Elvis Stansvik]

Post by André Somers
Hi,

Post by Elvis Stansvik

Post by Konstantin Shegunov
Hello,
Since we couldn't agree, I'd love to see some more opinions about this one.[1]

I may be missing some detail, but I think what Thiago says makes
sense. When children are destroyed, you know you're in the QObject
destructor (from QObject::~QObject docs: "Destroys the object,
deleting all its child objects."), so you know the object is now a
QObject, no longer a QCoreApplication. If you require your
QCoreApplication to be alive by the time your child object is
destroyed, I think you have to ensure this on your own.

Problem is, I think, that this requirement is not always obvious. For
your own objects, you know you cannot rely on your parent still being a
MyClass iso of just a QObject on destruction (unless you take specific
measures to make that so), but in this case the reliance on there still
being a Q*Application around (not necessarily the parent) is usually not
as obvious as a myParent->doSomethingNotFromQObject call in your
destructor code...

Yea, very good point. I'll leave it to the grow-ups to decide what to
do with this :) It was just my first gut reaction.

Elvis

Post by André Somers

Post by Elvis Stansvik
Like I said, I may be missing something, but that's what it looks like
to me. I can't see why there would be an exception to the object model
here.
Elvis

Post by Konstantin Shegunov
1) Is parenting to the application object a thing?

Yes. But you know that the same goes as for any QObject parent/child
relationship: the parent is a QObject at the time of destruction (ok,
with QWidget, you're in luck).

Post by Elvis Stansvik

Post by Konstantin Shegunov
1.a) ... and should it be allowed (i.e. accepting the proposed change)?

Yes, it should be allowed, and as you argued I think it is useful. But I
am not sure that implies the proposed change. OTOH, as so much
functionality in Qt requires the Q*Application to be alive (and that is
not always obvious) , perhaps an exception *is* in order.

Post by Elvis Stansvik

Post by Konstantin Shegunov
1.b) .. if not allowed, should we put a warning in the documentation that it is wrong and shouldn't be done at all, or at least that it's discouraged.
[1] https://bugreports.qt.io/browse/QTBUG-71545

I do think it makes sense to be able to do this, but if that is to be
discouraged, then best be explicit about that.
André
_______________________________________________
Development mailing list
http://lists.qt-project.org/mailman/listinfo/development

[Luca Beldi]

I'm in favour of just adding a line of documentation and maybe include Thiago's workaround. Should take 30seconds and hurt nobody

Luca Beldi (VRonin)

[Tomasz Siekierda]

Post by Konstantin Shegunov
Hello,
Since we couldn't agree, I'd love to see some more opinions about this one.[1]
1) Is parenting to the application object a thing?

Never done it myself. But Q*Application is clearly marked as derived
from QObject in the docs, so users can definitely expect it to behave
the same as all other QObjects and clean up it's children properly.

Post by Konstantin Shegunov
1.a) ... and should it be allowed (i.e. accepting the proposed change)?
1.b) .. if not allowed, should we put a warning in the documentation that it is wrong and shouldn't be done at all, or at least that it's discouraged.

Either is OK I think, with preference for 1.a). Note: these are not
mutually exclusive - we could have the patch integrated & a warning in
the docs that this is not encouraged.


Disclaimer: I'm not a reviewer, nor approver, barely a contributor
even, so feel free to ignore my opinion.

[Thiago Macieira]

Post by Tomasz Siekierda

Post by Konstantin Shegunov
Hello,
Since we couldn't agree, I'd love to see some more opinions about this one.[1]
1) Is parenting to the application object a thing?

Never done it myself. But Q*Application is clearly marked as derived
from QObject in the docs, so users can definitely expect it to behave
the same as all other QObjects and clean up it's children properly.

Post by Konstantin Shegunov
1.a) ... and should it be allowed (i.e. accepting the proposed change)?
1.b) .. if not allowed, should we put a warning in the documentation that
it is wrong and shouldn't be done at all, or at least that it's discouraged.

Either is OK I think, with preference for 1.a). Note: these are not
mutually exclusive - we could have the patch integrated & a warning in
the docs that this is not encouraged.

The problem is that most Qt API is only supported while QCoreApplication still
exists (and GUI stuff while QGuiApplication exists; widgets while QApplication
exists). By the time the QObject destructor destroys the children of any of
those, the QCoreApplication object no longer exists, so you're out of support.

That's why my opinion in the bug report was that you shouldn't do it.

[Elvis Stansvik]

Post by Tomasz Siekierda

Post by Konstantin Shegunov
Hello,
Since we couldn't agree, I'd love to see some more opinions about this one.[1]
1) Is parenting to the application object a thing?

Never done it myself. But Q*Application is clearly marked as derived
from QObject in the docs, so users can definitely expect it to behave
the same as all other QObjects and clean up it's children properly.

If it is to be the same as all other QObjects, then it should maintain
its current behavior I think. The destruction of children happens in
the QObject destructor. I don't even think one have to look at the
QObject destructor docs to understand that - where else would it be
done, considering the parent/child mechanism is a mechanism common to
all QObject-derived classes

Elvis

Post by Tomasz Siekierda

Post by Konstantin Shegunov
1.a) ... and should it be allowed (i.e. accepting the proposed change)?
1.b) .. if not allowed, should we put a warning in the documentation that it is wrong and shouldn't be done at all, or at least that it's discouraged.

Either is OK I think, with preference for 1.a). Note: these are not
mutually exclusive - we could have the patch integrated & a warning in
the docs that this is not encouraged.
Disclaimer: I'm not a reviewer, nor approver, barely a contributor
even, so feel free to ignore my opinion.
_______________________________________________
Development mailing list
http://lists.qt-project.org/mailman/listinfo/development

[Thiago Macieira]

Post by Elvis Stansvik
If it is to be the same as all other QObjects, then it should maintain
its current behavior I think. The destruction of children happens in
the QObject destructor. I don't even think one have to look at the
QObject destructor docs to understand that - where else would it be
done, considering the parent/child mechanism is a mechanism common to
all QObject-derived classes

Ah, but there's an exception: QWidget's destructor destroys its children, so
that they get to see their parent before the QWidget ceases to be a QWidget.

[Elvis Stansvik]

Post by Thiago Macieira

Post by Elvis Stansvik
If it is to be the same as all other QObjects, then it should maintain
its current behavior I think. The destruction of children happens in
the QObject destructor. I don't even think one have to look at the
QObject destructor docs to understand that - where else would it be
done, considering the parent/child mechanism is a mechanism common to
all QObject-derived classes

Ah, but there's an exception: QWidget's destructor destroys its children, so
that they get to see their parent before the QWidget ceases to be a QWidget.

Ah yes, I was talking about QObjects in general. QWidget is an
exception. Having hardly ever relied on that behavior more than
indirectly, I almost forgot about it.

But seems to me it would be a slippery slope to accept more
exceptions. What's next, will I have to implement the destruction
myself in my own widgets? :)

Elvis

Post by Thiago Macieira
--
Thiago Macieira - thiago.macieira (AT) intel.com
Software Architect - Intel Open Source Technology Center
_______________________________________________
Development mailing list
http://lists.qt-project.org/mailman/listinfo/development

[Konstantin Shegunov]

Post by Elvis Stansvik
But seems to me it would be a slippery slope to accept more
exceptions.

You say exception, but I say expected behavior, which is actually the crux
of the disagreement.

Post by Elvis Stansvik
What's next, will I have to implement the destruction
myself in my own widgets? :)

Yes, in the general case you actually have to if your dependent objects are
attached to a QObject holding an application-global state. Which is exactly
what Q*Application is - a QObject that has/is a global state.

Post by Elvis Stansvik
The title of the bug report is about QCoreApplication, while the demo
code is a QApplication - so I'm not 100% sure if I completely understood
the discussion.

I'm at fault for not attaching the stack trace, but I'll try to explain.
Indeed the example code is for QApplication, the problem however manifests
whenever the destruction of said application object goes through. The
segfault happens because QCoreApplication sets its static member - the
global instance of the application object - to null before the children are
cleaned up by the QObject destructor. This means that objects parented to
the application are destroyed after there's Q*Application object no more.

But at least for QApplication I would consider having children being
[snippet]
Interesting point, I haven't thought about it.

[Konstantin Shegunov]

On Tue, Nov 6, 2018 at 1:07 AM Giuseppe D'Angelo via Development <

Note however that those children are deleted explicitly (via manual
calls to delete), and NOT via the parent/child relation.

Noted. When I started that thread I didn't intend it to become one of those
lengthy arguments really. More like a simple "poll" whether the thing's
allowed, and if not how to handle it. As I wrote in the bug report -
"disallowing" the application to be parent is acceptable as an answer to
me, but then I think it should be noted in the documentation otherwise it's
rather misleading from the user's point of view.

[Uwe Rathmann]

Post by Konstantin Shegunov
1) Is parenting to the application object a thing?

The title of the bug report is about QCoreApplication, while the demo
code is a QApplication - so I'm not 100% sure if I completely understood
the discussion.

But at least for QApplication I would consider having children being
common practice and actually Qt does this too:

int main ( int argc, char **argv )
{
QApplication a( argc, argv );
a.setStyle( "Windows" );

qDebug() << a.children();
...
}

=>

QPAEventDispatcherGlib(0x10a9af0)
QSessionManager(0x10ac560)
QWindowsStyle(0x10b34f0, name = "windows")

Uwe