Revamp the instance-id and the dbus anchors for the controller

This is necessary for a nicer QtCreator integration, plus it fixes two
bugs:
 1) the DBus data was overwritten by an instance with the same id.
 2) the controller could not address 2 instances with the same id
    individually

Instead of 3 separate, unprotected files (one for each DBus interface)
in /tmp, we are now creating 2 files in $XDG_RUNTIME_DIR
/qtapplicationmanager (or the temp dir on non-Linux platforms)
 - a .lock file (via QLockFile), which let's us track an instance's
   lifetime
 - a .json file which currently contains the DBus addresses for the
   singleton interfaces

Pick-to: 6.7
Change-Id: Idd4fff96897ec9bb609cbf5521317b74df6d0fd4
Reviewed-by: Dominik Holland <dominik.holland@qt.io>

5 files changed, 200 insertions, 74 deletions

diff --git a/doc/configuration.qdoc b/doc/configuration.qdoc
index afebd318..6e05a2d0 100644
--- a/doc/configuration.qdoc
+++ b/doc/configuration.qdoc
@@ -143,9 +143,10 @@ ui:
             \br [\c instanceId]
         \li string
         \target instance-id
-        \li Assign an unique name to this application manager instance. Only useful if you are
-            running multiple instances at the same time and you need to address them via the
-            \l{Controller}{appman-controller tool}.
+        \li Assign a name to this application manager instance. Only useful if you are running
+            multiple instances at the same time and you need to address them via the
+            \l{Controller}{appman-controller tool}. A unique number is appended to this id to make
+            it possible to disambiguate between instances with the same id. (default: appman)
     \row
         \li \b --database
             \br [\c applications/database]
diff --git a/doc/controller.qdoc b/doc/controller.qdoc
index 9a6d1413..a530150b 100644
--- a/doc/controller.qdoc
+++ b/doc/controller.qdoc
@@ -16,9 +16,26 @@ communicating directly with its D-Bus interface.
 \note In order to use this tool, the application manager has to be connected to either a session- or
 system-bus; don't run it with \c{--dbus none}.
 
-If you are running multiple application manager instances in the same system, you need to first
-\l{instance-id}{assign unique instance-ids} to each of them and then you can address them
-individually from the \c appman-controller tool by using the \c --instance-id command line option.
+If you are running multiple application manager instances in the same system, you need to tell the
+controller which instance you are addressing.
+The default id for any system-ui is \c appman, but you can \l{instance-id}{assign custom instance-ids}.
+In addition, a unique number is appended to this configured id to make it possible to disambiguate
+between instances with the same id.
+
+A list of all currently running instances can be obtained with the \c list-instances command.
+
+The \c --instance-id option lets you choose which of the running appman instances you want to
+address. This gives you 3 possibilities:
+\list
+    \li You do not specify \c --instance-id at all: if only one appman instance is running, then it
+        will be addressed (ignoring its instance-id). If there are more instances, the tool will
+        stop with an error.
+    \li You only specify the base id without the disambiguating number (e.g. \c appman):  if only
+        one appman instance using the given base id is running, then it will be addressed. If there
+        are more instances, the tool will stop with an error.
+    \li You specify the full id with the disambiguating number (e.g. appman-1): Only the given
+        appman instance will be addressed.
+\endlist
 
 The following commands are available:
 
@@ -105,7 +122,8 @@ The following commands are available:
 \row
     \li \span {style="white-space: nowrap"} {\c list-instances}
     \li (none)
-    \li Lists all currently running \l{instance-id}{named} application manager instances.
+    \li Lists the unique \l{instance-id}{ instance ids} of all currently running application manager
+        instances.
 \row
     \li \span {style="white-space: nowrap"} {\c inject-intent-request}
     \li \c{<intent-id>}
diff --git a/src/main-lib/main.cpp b/src/main-lib/main.cpp
index 6c55d379..17b3dbaa 100644
--- a/src/main-lib/main.cpp
+++ b/src/main-lib/main.cpp
@@ -43,6 +43,8 @@
 #include <QInputDevice>
 #include <QLocalServer>
 #include <QLibraryInfo>
+#include <QStandardPaths>
+#include <QLockFile>
 
 #include "global.h"
 #include "logging.h"
@@ -108,15 +110,14 @@ QT_BEGIN_NAMESPACE_AM
 
 #if defined(QT_DBUS_LIB) && QT_CONFIG(am_external_dbus_interfaces)
 
-static void registerDBusObject(QDBusAbstractAdaptor *adaptor, QString dbusName, const char *serviceName,
-                               const char *interfaceName, const char *path,
-                               const QString &instanceId) noexcept(false)
+static QString registerDBusObject(QDBusAbstractAdaptor *adaptor, QString dbusName,
+                                  const char *serviceName, const char *path) noexcept(false)
 {
     QString dbusAddress;
     QDBusConnection conn((QString()));
 
     if (dbusName.isEmpty()) {
-        return;
+        return { };
     } else if (dbusName == u"system") {
         dbusAddress = QString::fromLocal8Bit(qgetenv("DBUS_SYSTEM_BUS_ADDRESS"));
 #  if defined(Q_OS_LINUX)
@@ -134,7 +135,7 @@ static void registerDBusObject(QDBusAbstractAdaptor *adaptor, QString dbusName,
         // this case, Qt has cached the bus name and we would get the old one back.
         conn = QDBusConnection::connectToBus(dbusAddress, u"qtam_session"_s);
         if (!conn.isConnected())
-            return;
+            return { };
         dbusName = u"session"_s;
     } else {
         dbusAddress = dbusName;
@@ -164,24 +165,7 @@ static void registerDBusObject(QDBusAbstractAdaptor *adaptor, QString dbusName,
 
     qCDebug(LogSystem).nospace().noquote() << " * " << serviceName << path << " [on bus: " << dbusName << "]";
 
-    if (QByteArray::fromRawData(interfaceName, int(qstrlen(interfaceName))).startsWith("io.qt.")) {
-        // Write the bus address of the interface to a file in /tmp. This is needed for the
-        // controller tool, which does not even have a session bus, when started via ssh.
-
-        QString fileName = QString::fromLatin1(interfaceName) % u".dbus"_s;
-        if (!instanceId.isEmpty())
-            fileName = instanceId % u'-' % fileName;
-
-        QFile f(QDir::temp().absoluteFilePath(fileName));
-        QByteArray dbusUtf8 = dbusAddress.isEmpty() ? dbusName.toUtf8() : dbusAddress.toUtf8();
-        if (!f.open(QFile::WriteOnly | QFile::Truncate) || (f.write(dbusUtf8) != dbusUtf8.size()))
-            throw Exception(f, "Could not write D-Bus address of interface %1").arg(QString::fromLatin1(interfaceName));
-
-        static QStringList filesToDelete;
-        if (filesToDelete.isEmpty())
-            atexit([]() { for (const QString &ftd : std::as_const(filesToDelete)) QFile::remove(ftd); });
-        filesToDelete << f.fileName();
-    }
+    return dbusAddress.isEmpty() ? dbusName : dbusAddress;
 }
 
 #endif // defined(QT_DBUS_LIB) && QT_CONFIG(am_external_dbus_interfaces)
@@ -312,8 +296,8 @@ void Main::setup(const Configuration *cfg) noexcept(false)
                        cfg->noUiWatchdog(), cfg->allowUnknownUiClients());
 
     setupDBus(std::bind(&Configuration::dbusRegistration, cfg, std::placeholders::_1),
-              std::bind(&Configuration::dbusPolicy, cfg, std::placeholders::_1),
-              cfg->instanceId());
+              std::bind(&Configuration::dbusPolicy, cfg, std::placeholders::_1));
+    createInstanceInfoFile(cfg->instanceId());
 }
 
 bool Main::isSingleProcessMode() const
@@ -802,6 +786,48 @@ void Main::setupWindowManager(const QString &waylandSocketName, const QVariantLi
                      m_windowManager, &WindowManager::raiseApplicationWindow);
 }
 
+void Main::createInstanceInfoFile(const QString &instanceId) noexcept(false)
+{
+    // This is needed for the appman-controller tool to talk to running appman instances.
+    // (the tool does not even have a session bus, when started via ssh)
+
+    static const QString defaultInstanceId = u"appman"_s;
+
+    QString rtPath = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation);
+    if (rtPath.isEmpty())
+        rtPath = QDir::tempPath();
+    QDir rtDir(rtPath + u"/qtapplicationmanager"_s);
+    if (!rtDir.mkpath(u"."_s))
+        throw Exception("Could not create runtime state directory (%1) for the instance info").arg(rtDir.absolutePath());
+
+    QString fileName;
+    QString filePattern = (instanceId.isEmpty() ? defaultInstanceId : instanceId) + u"-%1";
+
+    static std::unique_ptr<QLockFile> lockf;
+    static std::unique_ptr<QFile, void (*)(QFile *)> infof(nullptr, [](QFile *f) { f->remove(); });
+
+    for (int i = 0; i < 32; ++i) { // Wayland sockets are limited to 32 instances as well
+        QString tryPattern = filePattern.arg(i);
+        lockf.reset(new QLockFile(rtDir.absoluteFilePath(tryPattern + u".lock"_s)));
+        lockf->setStaleLockTime(0);
+        if (lockf->tryLock()) {
+            fileName = tryPattern;
+            break; // found a free instance id
+        }
+    }
+    if (fileName.isEmpty())
+        throw Exception("Could not create a lock file for the instance info at %1").arg(rtDir.absolutePath());;
+
+    infof.reset(new QFile(rtDir.absoluteFilePath(fileName + u".json"_s)));
+
+    const QByteArray json = QJsonDocument::fromVariant(m_infoFileContents).toJson(QJsonDocument::Indented);
+    if (!infof->open(QIODevice::WriteOnly))
+        throw Exception(*infof.get(), "failed to create instance info file");
+    if (infof->write(json) !=json.size())
+        throw Exception(*infof.get(), "failed to write instance info file");
+    infof->close();
+}
+
 void Main::loadQml(bool loadDummyData) noexcept(false)
 {
     for (auto iface : std::as_const(m_startupPlugins))
@@ -915,8 +941,7 @@ void Main::showWindow(bool showFullscreen)
 }
 
 void Main::setupDBus(const std::function<QString(const char *)> &busForInterface,
-                     const std::function<QVariantMap(const char *)> &policyForInterface,
-                     const QString &instanceId)
+                     const std::function<QVariantMap(const char *)> &policyForInterface)
 {
 #if defined(QT_DBUS_LIB) && QT_CONFIG(am_external_dbus_interfaces)
     registerDBusTypes();
@@ -989,17 +1014,22 @@ void Main::setupDBus(const std::function<QString(const char *)> &busForInterface
             if (dbusName.isEmpty())
                 continue;
 
-            registerDBusObject(generatedAdaptor, dbusName,
-                               std::get<2>(iface),interfaceName, std::get<3>(iface),
-                               instanceId);
+            auto dbusAddress = registerDBusObject(generatedAdaptor, dbusName,
+                                                  std::get<2>(iface), std::get<3>(iface));
             if (!DBusPolicy::instance()->add(generatedAdaptor, policyForInterface(interfaceName)))
                 throw Exception(Error::DBus, "could not set DBus policy for %1").arg(QString::fromLatin1(interfaceName));
+
+            // Write the bus address to our info file for the appman-controller tool
+            if (QByteArrayView { interfaceName }.startsWith("io.qt.")) {
+                auto map = m_infoFileContents[u"dbus"_s].toMap();
+                map.insert(QString::fromLatin1(interfaceName), dbusAddress);
+                m_infoFileContents[u"dbus"_s] = map;
+            }
         }
     }
 #else
     Q_UNUSED(busForInterface)
     Q_UNUSED(policyForInterface)
-    Q_UNUSED(instanceId)
 #endif // defined(QT_DBUS_LIB) && QT_CONFIG(am_external_dbus_interfaces)
 }
 
diff --git a/src/main-lib/main.h b/src/main-lib/main.h
index cbb93b07..9e280ce8 100644
--- a/src/main-lib/main.h
+++ b/src/main-lib/main.h
@@ -82,7 +82,7 @@ protected:
     void loadStartupPlugins(const QStringList &startupPluginPaths) noexcept(false);
     void parseSystemProperties(const QVariantMap &rawSystemProperties);
     void setupDBus(const std::function<QString(const char *)> &busForInterface,
-                   const std::function<QVariantMap(const char *)> &policyForInterface, const QString &instanceId);
+                   const std::function<QVariantMap(const char *)> &policyForInterface);
     void setMainQmlFile(const QString &mainQml) noexcept(false);
     void setupSingleOrMultiProcess(bool forceSingleProcess, bool forceMultiProcess) noexcept(false);
     void setupRuntimesAndContainers(const QVariantMap &runtimeConfigurations, const QStringList &runtimeAdditionalLaunchers,
@@ -102,6 +102,7 @@ protected:
     void setupWindowTitle(const QString &title, const QString &iconPath);
     void setupWindowManager(const QString &waylandSocketName, const QVariantList &waylandExtraSockets,
                             bool slowAnimations, bool noUiWatchdog, bool allowUnknownUiClients);
+    void createInstanceInfoFile(const QString &instanceId) noexcept(false);
 
     enum SystemProperties {
         SP_ThirdParty = 0,
@@ -139,6 +140,7 @@ private:
     QString m_installationDir;
     QString m_documentDir;
     QString m_installationDirMountPoint;
+    QVariantMap m_infoFileContents;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS(Main::InitFlags)
diff --git a/src/tools/controller/controller.cpp b/src/tools/controller/controller.cpp
index bd35f3e3..7def4a4d 100644
--- a/src/tools/controller/controller.cpp
+++ b/src/tools/controller/controller.cpp
@@ -16,6 +16,8 @@
 #include <QDBusError>
 #include <QMetaObject>
 #include <QStringBuilder>
+#include <QRegularExpression>
+#include <QJsonDocument>
 
 #include <functional>
 
@@ -45,11 +47,9 @@ public:
         registerDBusTypes();
     }
 
-    void setInstanceId(const QString &instanceId)
+    void setInstanceInfo(const QVariantMap &instanceInfo)
     {
-        m_instanceId = instanceId;
-        if (!m_instanceId.isEmpty())
-            m_instanceId.append(u'-');
+        m_dbusAddresses = instanceInfo[u"dbus"_s].toMap();
     }
 
     void connectToManager() noexcept(false)
@@ -78,21 +78,15 @@ private:
     {
         QDBusConnection conn(iface);
 
-        QFile f(QDir::temp().absoluteFilePath(m_instanceId % QString(u"%1.dbus"_s).arg(iface)));
-        QString dbus;
-        if (f.open(QFile::ReadOnly)) {
-            dbus = QString::fromUtf8(f.readAll());
-            if (dbus == u"system") {
-                conn = QDBusConnection::systemBus();
-                dbus = u"[system-bus]"_s;
-            } else if (dbus.isEmpty()) {
-                conn = QDBusConnection::sessionBus();
-                dbus = u"[session-bus]"_s;
-            } else {
-                conn = QDBusConnection::connectToBus(dbus, u"custom"_s);
-            }
+        QString dbus = m_dbusAddresses.value(iface).toString();
+        if (dbus == u"system") {
+            conn = QDBusConnection::systemBus();
+            dbus = u"[system-bus]"_s;
+        } else if (dbus.isEmpty()) {
+            conn = QDBusConnection::sessionBus();
+            dbus = u"[session-bus]"_s;
         } else {
-            throw Exception(Error::IO, "Could not find the D-Bus interface of a running application manager instance.\n(did you start the appman with '--dbus none'?");
+            conn = QDBusConnection::connectToBus(dbus, u"custom"_s);
         }
 
         if (!conn.isConnected()) {
@@ -160,7 +154,7 @@ public:
 private:
     IoQtPackageManagerInterface *m_packager = nullptr;
     IoQtApplicationManagerInterface *m_manager = nullptr;
-    QString m_instanceId;
+    QVariantMap m_dbusAddresses;
     QStringList m_connections;
     QTimer *m_disconnectTimer = nullptr;
     bool m_disconnectedEmitted = false;
@@ -231,6 +225,9 @@ static Command command(QCommandLineParser &clp)
     return NoCommand;
 }
 
+static std::pair<QString, QMultiHash<QString, int>> runningInstanceIds();
+static QVariantMap resolveInstanceInfo(const QString &instanceId);
+
 static void startOrDebugApplication(const QString &debugWrapper, const QString &appId,
                                     const QMap<QString, int> &stdRedirections, bool restart,
                                     const QString &documentUrl) noexcept(false);
@@ -321,16 +318,19 @@ int main(int argc, char *argv[])
     clp.setOptionsAfterPositionalArgumentsMode(QCommandLineParser::ParseAsPositionalArguments);
 
     // ignore the return value here, as we also accept options we don't know about yet.
-    // If an option is really not accepted by a command, the comman specific parsing should report
+    // If an option is really not accepted by a command, the command specific parsing should report
     // this.
-    clp.parse(QCoreApplication::arguments());
     clp.setOptionsAfterPositionalArgumentsMode(QCommandLineParser::ParseAsOptions);
+    clp.parse(QCoreApplication::arguments());
 
-    dbus()->setInstanceId(clp.value(u"instance-id"_s));
 
     // REMEMBER to update the completion file util/bash/appman-prompt, if you apply changes below!
     try {
-        switch (command(clp)) {
+        auto cmd = command(clp);
+        if ((cmd != NoCommand) && (cmd != ListInstances))
+            dbus()->setInstanceInfo(resolveInstanceInfo(clp.value(u"instance-id"_s)));
+
+        switch (cmd) {
         case NoCommand:
             if (clp.isSet(u"version"_s))
                 clp.showVersion();
@@ -994,23 +994,98 @@ void showInstallationLocation(bool asJson) noexcept(false)
     qApp->quit();
 }
 
-void listInstances()
+static std::pair<QString, QMultiHash<QString, int>> runningInstanceIds()
 {
-    QString dir = QDir::temp().absolutePath() % u'/';
-    QString suffix = u"io.qt.ApplicationManager.dbus"_s;
+    QMultiHash<QString, int> result;
 
-    QDirIterator dit(dir, { u'*' % suffix });
+    QString rtPath = QStandardPaths::writableLocation(QStandardPaths::RuntimeLocation);
+    if (rtPath.isEmpty())
+        rtPath = QDir::tempPath();
+    QDir rtDir(rtPath);
+    if (!rtDir.cd(u"qtapplicationmanager"_s))
+        return { rtDir.path(), result };
+
+    const QString suffix = u".lock"_s;
+    QDirIterator dit(rtDir.path(), { u'*' + suffix });
     while (dit.hasNext()) {
-        QString name = dit.next();
+        QString path = dit.next();
+        QString name = dit.fileName();
         name.chop(suffix.length());
-        name = name.mid(dir.length());
-        if (name.isEmpty()) {
-            name = u"(no instance id)"_s;
-        } else {
-            name.chop(1); // remove the '-' separator
-            name = u'"' % name % u'"';
+        if (auto dashPos = name.lastIndexOf(u'-'); dashPos > 0) {
+            bool counterOk = false;
+            int counter = QStringView { name }.sliced(dashPos + 1).toInt(&counterOk);
+            if (counterOk)
+                result.insert(name.left(dashPos), counter);
+        }
+    }
+    return { rtDir.path(), result };
+}
+
+static QVariantMap resolveInstanceInfo(const QString &instanceId)
+{
+    static const QString defaultInstanceId = u"appman"_s;
+    static QRegularExpression re(uR"(^(.+?)(?:-(\d+))?$)"_s);
+
+    const auto [baseDir, running] = runningInstanceIds();
+    QString iid = instanceId.isEmpty() ? defaultInstanceId : instanceId;
+    QString result;
+
+    try {
+        QString id;
+        int counter = -1;
+        auto m = re.match(iid);
+        if (!m.hasMatch())
+            throw Exception("Invalid instance-id");
+        id = m.captured(1);
+        bool counterOk = true;
+        counter = m.hasCaptured(2) ? int(m.captured(2).toUInt(&counterOk)) : -1;
+        if (!counterOk)
+            throw Exception("Invalid instance-id");
+
+        if (counter >= 0) {
+            // fully qualified instance id: must match exactly
+            if (running.contains(id, counter))
+                result = instanceId;
+        } else if (running.count(id) == 1) {
+            // id only: matches if there's exactly one instance with that name
+            result = id + u'-' + QString::number(running[id]);
+        } else if (instanceId.isEmpty() && (running.count(id) == 0)
+                   && (running.count() == 1)) {
+            // no id: matches even a named instance, if that is the only instance running
+            result = running.constBegin().key() + u'-' + QString::number(running.constBegin().value());
         }
-        fprintf(stdout, "%s\n", name.toLocal8Bit().constData());
+
+        if (result.isEmpty()) {
+            throw Exception("Could not resolve the given instance-id (%1) to any running appman instance.\n  (did you start the appman with '--dbus none'?)")
+                .arg(instanceId);
+        }
+    } catch (const Exception &e) {
+        QStringList allIds;
+        for (auto it = running.cbegin(); it != running.cend(); ++it)
+            allIds.append(it.key() + u'-' + QString::number(it.value()));
+        throw Exception(u"%1\n\nAvailable instances:\n  %2"_s.arg(e.errorString())
+                        .arg(allIds.join(u"\n  ")));
+    }
+
+    QFile infof(baseDir + u'/' + result + u".json"_s);
+    if (!infof.open(QIODevice::ReadOnly))
+        throw Exception(infof, "Could not open instance info file");
+
+    QJsonParseError jsonError;
+    const auto json = QJsonDocument::fromJson(infof.readAll(), &jsonError);
+    if (json.isNull()) {
+        throw Exception("Failed to parse instance info file (%1) as JSON: %2")
+            .arg(infof.fileName()).arg(jsonError.errorString());
+    }
+    return json.toVariant().toMap();
+}
+
+void listInstances()
+{
+    const auto [_, running] = runningInstanceIds();
+    for (auto it = running.cbegin(); it != running.cend(); ++it) {
+        auto &name = it.key();
+        fprintf(stdout, "%s-%d\n", name.toLocal8Bit().constData(), it.value());
     }
     qApp->quit();
 }