Merge lp:~townsend/libertine/qtdbus-connection into lp:libertine/trunk

[Christopher Townsend]

Ensure we don't leak references to the DBus session bus.

[Larry Price (larryprice)]

lgtm! we can deal with style stuff later.

review: Approve

=== modified file 'liblibertine/libertined.cpp'
--- liblibertine/libertined.cpp	2017-01-24 16:03:25 +0000
+++ liblibertine/libertined.cpp	2017-02-14 17:48:20 +0000
@@ -30,6 +30,22 @@
 static const auto SERVICE_INTERFACE = "com.canonical.libertine.Service";
 static const auto PROGRESS_INTERFACE = "com.canonical.libertine.Service.Progress";
 
+/* Class to handle making sure the session bus isn't a singleton
+   which is how QtDBus does it by default. This is a bad thing for
+   a library to do, so we'll make sure we aren't causing global leaks */
+class SessionBus {
+public:
+  QDBusConnection bus;
+  SessionBus() :
+    /* NOTE: Must get the envvar each time or Qt caches that too */
+    bus{QDBusConnection::connectToBus(getenv("DBUS_SESSION_BUS_ADDRESS"), "libertined")}
+  {
+  }
+  ~SessionBus() {
+    QDBusConnection::disconnectFromBus("libertined");
+  }
+};
+
 static QVariantList
 dbusCall(QDBusConnection const& bus, QString const& iface, QString const& path,
          QString const& method, QVariantList const& args = QVariantList())
@@ -119,65 +135,65 @@
 QString
 container_info(char const* container_id, QString const& key)
 {
-  auto bus = QDBusConnection::sessionBus();
-  auto path = call(bus, "container_info", QVariantList{QVariant(container_id)});
+  SessionBus session;
+  auto path = call(session.bus, "container_info", QVariantList{QVariant(container_id)});
 
-  if (!waitForFinish(bus, path))
+  if (!waitForFinish(session.bus, path))
   {
     return QString();
   }
 
-  auto error = lastError(bus, path);
+  auto error = lastError(session.bus, path);
   if (!error.isEmpty())
   {
     qWarning() << "error:" << error;
     return QString();
   }
 
-  return QJsonDocument::fromJson(result(bus, path).toLatin1()).object().value(key).toString();
+  return QJsonDocument::fromJson(result(session.bus, path).toLatin1()).object().value(key).toString();
 }
 }
 
 QJsonArray
 libertined_list()
 {
-  auto bus = QDBusConnection::sessionBus();
-  auto path = call(bus, "list", QVariantList());
+  SessionBus session;
+  auto path = call(session.bus, "list", QVariantList());
 
-  if (!waitForFinish(bus, path))
+  if (!waitForFinish(session.bus, path))
   {
     return QJsonArray();
   }
 
-  auto error = lastError(bus, path);
+  auto error = lastError(session.bus, path);
   if (!error.isEmpty())
   {
     qWarning() << "error:" << error;
     return QJsonArray();
   }
 
-  return QJsonDocument::fromJson(result(bus, path).toLatin1()).array();
+  return QJsonDocument::fromJson(result(session.bus, path).toLatin1()).array();
 }
 
 QJsonArray
 libertined_list_app_ids(char const* container_id)
 {
-  auto bus = QDBusConnection::sessionBus();
-  auto path = call(bus, "list_app_ids", QVariantList{QVariant(container_id)});
+  SessionBus session;
+  auto path = call(session.bus, "list_app_ids", QVariantList{QVariant(container_id)});
 
-  if (!waitForFinish(bus, path))
+  if (!waitForFinish(session.bus, path))
   {
     return QJsonArray();
   }
 
-  auto error = lastError(bus, path);
+  auto error = lastError(session.bus, path);
   if (!error.isEmpty())
   {
     qWarning() << "error:" << error;
     return QJsonArray();
   }
 
-  return QJsonDocument::fromJson(result(bus, path).toLatin1()).array();
+  return QJsonDocument::fromJson(result(session.bus, path).toLatin1()).array();
 }
 
 QString