From a79d5b8d0856a8f6c4de56a8d982c84ea20905a6 Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Sat, 22 Mar 2025 14:27:23 +0100
Subject: [PATCH] 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.9 6.8
Change-Id: I9e6dcade0d5572345b71e3e8f06902bbea1ef89a
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
---
 src/corelib/kernel/qobject.cpp                           | 8 ++++++++
 tests/auto/corelib/kernel/qobject/tst_qobject.cpp        | 9 +++++++++
 .../widgets/kernel/qwidget_window/tst_qwidget_window.cpp | 5 +++--
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index a12efad25ab..62202178288 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -3759,6 +3759,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))
diff --git a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
index 609229b1ae7..c68d8e83166 100644
--- a/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
+++ b/tests/auto/corelib/kernel/qobject/tst_qobject.cpp
@@ -389,6 +389,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
diff --git a/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp b/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp
index a04ab9f6cb4..a00a96c925e 100644
--- a/tests/auto/widgets/kernel/qwidget_window/tst_qwidget_window.cpp
+++ b/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++) {