QObject::disconnect: warn if a wildcard call disconnects from destroyed

Qt relies internally on connections to the destroyed() signal of
application-objects, for example to clean up accessibility interface
implementations or other data structures that would otherwise grow or
hold danging pointers.

The QObject::disconnect function is already documented to be potentially
dangerous, as a wildcard call (with signal, receiver, and method
defaulted to nullptr) will disconnect everything. However, what happpens
to work in one Qt release might break things in another if we add code
that relies on connections to destroyed().

To make users aware of the danger of a wildcard disconnect call, and to
help with fixing the issues that might arise, emit a warning when a
wildcard disconnect() breaks an existing connection to the sender's
destroyed() signal.

Do this while we hold the connection mutex, and only if all parameters
to the disconnect() call are wildcarded. This happens very rarely, and
should never happen in performance critical code.

Fix the only test I found emitting this warning due to a wildcard
disconnect (the drag'n'drop subsystem relies on the destroyed signal).
Store the only relevant connection instead so that we can disconnect
when done.

Fixes: QTBUG-134610
Pick-to: 6.8
Change-Id: I9e6dcade0d5572345b71e3e8f06902bbea1ef89a
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit a79d5b8d0856a8f6c4de56a8d982c84ea20905a6)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

src/corelib/kernel/qobject.cpp

@@ -3756,6 +3756,14 @@ bool QMetaObjectPrivate::disconnect(const QObject *sender,
         QObjectPrivate::ConnectionDataPointer connections(scd);
 
         if (signal_index < 0) {
+            // wildcard disconnect - warn if this disconnects destroyed()
+            if (!receiver && method_index < 0 && sender->d_func()->isSignalConnected(0)) {
+                qWarning("QObject::disconnect: wildcard call disconnects from destroyed signal of"
+                         " %s::%s", sender->metaObject()->className(),
+                                    sender->objectName().isEmpty()
+                                        ? "unnamed"
+                                        : sender->objectName().toLocal8Bit().data());
+            }
             // remove from all connection lists
             for (int sig_index = -1; sig_index < scd->signalVectorCount(); ++sig_index) {
                 if (disconnectHelper(connections.data(), sig_index, receiver, method_index, slot, senderMutex, disconnectType))

tests/auto/corelib/kernel/qobject/tst_qobject.cpp

@@ -386,6 +386,15 @@ void tst_QObject::disconnect()
     QVERIFY(!r2.called(2));
     QVERIFY(!r1.called(2));
     QVERIFY(!r2.called(2));
+
+    // breaking a connection to QObject::destroyed can be dangerous if Qt
+    // or other parts of the application depend on that signal to clean up
+    // data structures. We want to warn if this happens as a side effect of
+    // a wildcard disconnect call.
+    QObject::connect(&s, &QObject::destroyed, &r1, &ReceiverObject::slot1);
+    QTest::ignoreMessage(QtWarningMsg, QRegularExpression("wildcard call disconnects from destroyed"
+                                        " signal of SenderObject::"));
+    QVERIFY(s.disconnect());
 }
 
 class AutoConnectSender : public QObject

tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp

@@ -828,7 +828,8 @@ void tst_QWidget_window::tst_dnd_events()
 
     // Some dnd implementation rely on running internal event loops, so we have to use
     // the following queued signal hack to simulate mouse clicks in the widget.
-    QObject::connect(&dndWidget, &DnDEventRecorder::dragMoveReceived, this, [=]() {
+    auto connection = QObject::connect(&dndWidget, &DnDEventRecorder::dragMoveReceived, this,
+                                        [=]() {
         QTest::mouseRelease(window, Qt::LeftButton);
     }, Qt::QueuedConnection);
 
@@ -839,7 +840,7 @@ void tst_QWidget_window::tst_dnd_events()
     QCOMPARE(dndWidget._dndEvents, expectedDndEvents);
 
     dndWidget._dndEvents.clear();
-    dndWidget.disconnect();
+    dndWidget.disconnect(connection);
     int step = 0;
     QObject::connect(&dndWidget, &DnDEventRecorder::dragMoveReceived, this, [window, &step]() {
         switch (step++) {