From d3860918ea5237e63a49c059896cebf237d012b0 Mon Sep 17 00:00:00 2001
From: Ievgenii Meshcheriakov <ievgenii.meshcheriakov@qt.io>
Date: Mon, 27 Nov 2023 15:02:57 +0100
Subject: [PATCH] QDBusConnectionPrivate: Close connection after failed
 q_dbus_bus_register()

The connection should be closed before executing
q_dbus_connection_unref(). Failing to do so results in an assertion
inside libdbus:

    dbus[1573958]: The last reference on a connection was dropped
    without closing the connection. This is a bug in an application.
    See dbus_connection_unref() documentation for details.
    Most likely, the application was supposed to call
    dbus_connection_close(), since this is a private connection.

The q_dbus_bus_register() may fail if maximum number of active
connections for a bus was reached. This can be tested by creating
a custom bus with "max_completed_connections" parameter set to 0.
Add such a test to tst_qdbusconnection.

Fixes: QTBUG-34613
Change-Id: I6ae7df19bf8b6546c2a504931ba852dc15d35f78
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/dbus/qdbusconnectionmanager.cpp           |  1 +
 .../auto/dbus/qdbusconnection/CMakeLists.txt  |  2 ++
 .../qdbusconnection/tst_qdbusconnection.conf  | 23 +++++++++++++++
 .../qdbusconnection/tst_qdbusconnection.cpp   | 28 +++++++++++++++++++
 .../qdbusconnection/tst_qdbusconnection.h     |  1 +
 5 files changed, 55 insertions(+)
 create mode 100644 tests/auto/dbus/qdbusconnection/tst_qdbusconnection.conf

diff --git a/src/dbus/qdbusconnectionmanager.cpp b/src/dbus/qdbusconnectionmanager.cpp
index dc4116cf042..ce52c9fa633 100644
--- a/src/dbus/qdbusconnectionmanager.cpp
+++ b/src/dbus/qdbusconnectionmanager.cpp
@@ -250,6 +250,7 @@ QDBusConnectionPrivate *QDBusConnectionManager::doConnectToBus(const QString &ad
     if (c) {
         // register on the bus
         if (!q_dbus_bus_register(c, error)) {
+            q_dbus_connection_close(c);
             q_dbus_connection_unref(c);
             c = nullptr;
         }
diff --git a/tests/auto/dbus/qdbusconnection/CMakeLists.txt b/tests/auto/dbus/qdbusconnection/CMakeLists.txt
index f7b526a0cf9..56ae21f2911 100644
--- a/tests/auto/dbus/qdbusconnection/CMakeLists.txt
+++ b/tests/auto/dbus/qdbusconnection/CMakeLists.txt
@@ -16,4 +16,6 @@ qt_internal_add_test(tst_qdbusconnection
         tst_qdbusconnection.cpp tst_qdbusconnection.h
     LIBRARIES
         Qt::DBus
+    TESTDATA
+        tst_qdbusconnection.conf
 )
diff --git a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.conf b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.conf
new file mode 100644
index 00000000000..7dec73d0d78
--- /dev/null
+++ b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.conf
@@ -0,0 +1,23 @@
+<!-- This configuration file for tst_qdbusconnection.
+-->
+
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN"
+ "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
+<busconfig>
+  <type>tst_qdbusconnection</type>
+
+  <listen>unix:tmpdir=/tmp</listen>
+
+  <auth>EXTERNAL</auth>
+
+  <policy context="default">
+    <!-- Allow everything to be sent -->
+    <allow send_destination="*" eavesdrop="true"/>
+    <!-- Allow everything to be received -->
+    <allow eavesdrop="true"/>
+    <!-- Allow anyone to own anything -->
+    <allow own="*"/>
+  </policy>
+
+  <limit name="max_completed_connections">0</limit>
+</busconfig>
diff --git a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp
index dc39cadc95e..fb7c61569a7 100644
--- a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp
+++ b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.cpp
@@ -1412,6 +1412,34 @@ void tst_QDBusConnection::pendingCallWhenDisconnected()
 #endif
 }
 
+void tst_QDBusConnection::connectionLimit()
+{
+#if !QT_CONFIG(process)
+    QSKIP("Test requires QProcess");
+#else
+    if (!QCoreApplication::instance())
+        QSKIP("Test requires a QCoreApplication");
+
+    QProcess daemon;
+    daemon.start("dbus-daemon",
+                 QStringList() << "--config-file" << QFINDTESTDATA("tst_qdbusconnection.conf")
+                               << "--nofork"
+                               << "--print-address");
+    QVERIFY2(daemon.waitForReadyRead(2000),
+             "Daemon didn't print its address in time; error: \"" + daemon.errorString().toLocal8Bit() +
+             "\"; stderr:\n" + daemon.readAllStandardError());
+
+    QString address = QString::fromLocal8Bit(daemon.readAll().trimmed());
+    QDBusConnection con = QDBusConnection::connectToBus(address, "connectionLimit");
+    QVERIFY2(!con.isConnected(), "Unexpected successful connection");
+    QCOMPARE(con.lastError().type(), QDBusError::LimitsExceeded);
+
+    // kill the bus
+    daemon.terminate();
+    daemon.waitForFinished();
+#endif
+}
+
 void tst_QDBusConnection::emptyServerAddress()
 {
     QDBusServer server({}, nullptr);
diff --git a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.h b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.h
index 72198b75f38..895201ee9d6 100644
--- a/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.h
+++ b/tests/auto/dbus/qdbusconnection/tst_qdbusconnection.h
@@ -120,6 +120,7 @@ private slots:
     void callVirtualObject();
     void callVirtualObjectLocal();
     void pendingCallWhenDisconnected();
+    void connectionLimit();
 
     void emptyServerAddress();