[Development] Not delivering signals to slots in class sub-objects already destroyed

[Fabian Kosmale]

Hey Thiago,

first of all, thanks for looking into this. Give that this is a pitfall when porting to new style connection,
and we had issues with this in Qt (and IIRC Creator) itself, I definitely think that we should investigate it.
I had discussed a different, less generic idea (introduce indexOfSlot and use the same connect code as
with string based connect) in private with Mårten and Marc, and the difference between lambdas and
PMFs was also a point of contention. One possible way out there might be to support passing an
explicit target meta-object + a clazy check which warns about capturing lambdas without that explicit
meta-object.

Regarding performance: I'm wary about the overhead, but we can reduce it. Lars proposed back in the
day to track all ancestor meta-objects, but that ran into opposition because he required that every
QObject had a Q_OBJECT macro. However, that's not actually necessary; after all, a QObject without
the macro doesn't have it's own meta-object. We also know that moc must be able to see all parent classes,
as you can't have an incomplete base class. So I would expect that moc can track the depth[1], and we can store
it directly inside the meta-object, making the check O(1) for code compiled against a new enough code.

Regards,
Fabian

[1] There's some complication with deriving from (potentially specialized) template instantiations, which is hard
to properly support in moc. C++26 reflection solves this, and as we need to support meta-objects coming from
older Qt versions, we'd have the loop as a fallback available in any case.

________________________________________
Von: Development <development-bounces at qt-project.org> im Auftrag von Thiago Macieira <thiago.macieira at intel.com>
Gesendet: Mittwoch, 3. September 2025 07:18
An: development at qt-project.org
Betreff: [Development] Not delivering signals to slots in class sub-objects already destroyed

Re: https://codereview.qt-project.org/c/qt/qtbase/+/672087

TL;DR: I've got a solution to make the new-style QObject::connect() match the
behaviour of the old-style string-based one, which due to the nature of the
virtual table dispatching, would not deliver signals to slots in class sub-
objects already destroyed. But it isn't perfect. So I'd like a discussion:

a) should this functionality be provided at all?
b) should it be provided by default for PMFs? [*]
c) if it's provided for PMFs, should they be allowed to opt out?
d) should it be provided by default for callbacks other than PMFs?
e) if there's a flag to opt in or out, what do we call the Qt::XxxConnection?

Longer version:
I found a neat solution to implement a check for the dynamic class type that
does not involve RTTI, and will restore the behaviour that we used to have
with the string-based connections. The solution is to count how many meta
object superClass() we have until we reach the QObject's, and store this count
in the QObjectPrivate::Connection object. I've also done that without
increasing that object's size, because there are members unused when
connecting to a QSlotObjectBase.

There are two drawbacks:
1) it adds overhead to the activation of the slots, because it must count meta
objects for every Connection prior to activation. As a singly-linked list,
this is O(n), and though N is usually very small (99-percentile less than 10),
it may cross library boundaries and may thus run into cache misses. The same
operation is required on connect() itself, but the overhead there is
relatively smaller compared to the rest of the work. I have not measured it
and quite frankly I don't know what a good benchmark would be, if the issue is
going to be cache misses. But I am wary of just adding content to
QObject::doActivate(), which should be FAST.

2) I think it's safe to apply this by default when the receiver is a PMF and
the class of the PMF has destroyed (implemented on the commit after the above,
672088), but what about when the receiver is a static or non-member function,
or lambda or functor? The lifetime of the receiver's current dynamic class is
not tightly bound with the callback. An example of this is
Q_APPLICATION_STATIC:

 QObject::connect(app, &QObject::destroyed, app, reset, Qt::DirectConnection);

The dynamic class of the receiver stopped being QCoreApplication by the time
~QObject emitted destroyed(). Should the signal be delivered to reset()?
Obviously the code above expects it to do so. In fact, that's the expectation
for every use of &QObject::destroyed, because otherwise nothing would ever
run.

But how about

    QObject::connect(notifier, &QWinEventNotifier::activated, this,
        [this, registerForNotification] {
            emit valueChanged();
            registerForNotification();
        });
     QObject::connect(menuAction, &QAction::changed, q, [this] {
        if (!tornPopup.isNull())
            tornPopup->updateWindowTitle();
    });
QObject::connect(reply, &QNetworkReply::errorOccurred, query, [this, reply] {
     m_networkErrors << reply->errorString();
 });

Those are lambdas that, by capturing [this], are effectively member functions
in disguise, but there's no way we can tell that. Invoking those callbacks
after the members used in bodies of the lambdas have been destroyed is UB.
Even if the callback is a static or non-member function, it could still find
the receiver via some global variable. And I don't know if we'd want a
different behaviour depending on whether the callback is a lambda or a function
pointer.

Even the distinction between PMFs and others could be a pitfall. Take the first
example from QWinEventNotifier and let's say the original code was

    QObject::connect(notifier, &QWinEventNotifier::activated,
                     this, &QWinRegistryKey::valueChanged);

That code would have had the benefit of the protection. But then after some
bug-fixing or additional functionality, it became the lambda above, which does
not have the protection. That could lead to a subtle new bug[*] that may
escape detection during unit-testing and get found only by users.

[*] Right now, we don't have the protection against calling a PMF in a
destroyed sub-object, but we have an *enforcement* in debug mode, so if the
delivery could have hit the PMF before the change, it should have been seen
and fixed. Since we have the enforcement, I think that if we provide the
functionality, it should be active for PMFs by default.

--
Thiago Macieira - thiago.macieira (AT) intel.com
  Principal Engineer - Intel Platform Engineering Group