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 <[email protected]> im Auftrag von Thiago Macieira <[email protected]> Gesendet: Mittwoch, 3. September 2025 07:18 An: [email protected] 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 -- Development mailing list [email protected] https://lists.qt-project.org/listinfo/development
