[plasma/krdp] server: Simplify session resolution to XDG_SESSION_ID alone, address review

Git commit 3ea7d6c6b6f24a984d482b81026043c4121c6c4a by David Edmundson, on behalf of Nick Haghiri.
Committed on 22/07/2026 at 08:06.
Pushed by davidedmundson into branch 'master'.

Simplify session resolution to XDG_SESSION_ID alone, address review

GetSessionByPID never resolves for krdpserver as actually deployed: it
runs as a systemd --user service under [email protected]/app.slice, not
inside a session scope, so logind can't map its PID to a session.
Verified live: GetSessionByPID on krdpserver's real PID fails with
"PID does not belong to any known session", while the same call on
the session leader's PID succeeds. Drop the fallback entirely and
resolve from $XDG_SESSION_ID only, warning if it's unset.

Also drop the comment block (the reasoning stands on its own from the
code), and check "last client" after the erase instead of before, so
it reads as size() == 0.

M  +26   -58   server/SessionController.cpp

https://invent.kde.org/plasma/krdp/-/commit/3ea7d6c6b6f24a984d482b81026043c4121c6c4a

diff --git a/server/SessionController.cpp b/server/SessionController.cpp
index 690aade..1b68f6d 100644
--- a/server/SessionController.cpp
+++ b/server/SessionController.cpp
@@ -148,67 +148,36 @@ void SessionController::setSessionLocked(bool locked)
     if (!m_lockOnDisconnect) {
         return;
     }
-    // Ask logind to lock/unlock the graphical session (kscreenlocker honours its Lock/Unlock
-    // signals). krdpserver is a user-service process not in a login session, so resolve the
-    // session from $XDG_SESSION_ID, falling back to our PID.
-    //
-    // Everything is done asynchronously with QDBusMessage + QDBusConnection::asyncCall (NOT
-    // QDBusInterface, whose constructor blocks on introspection): a slow or hung logind must
-    // never stall the GUI thread, which also drives PipeWire/input/video for every other
-    // connected client (a blocking call could stall it up to the 25s D-Bus timeout).
+
+    const QString sessionId = qEnvironmentVariable("XDG_SESSION_ID");
+    if (sessionId.isEmpty()) {
+        qWarning() << "krdp: XDG_SESSION_ID is not set, cannot" << (locked ? "lock" : "unlock") << "the session";
+        return;
+    }
+
     auto bus = QDBusConnection::systemBus();
     const QString service = u"org.freedesktop.login1"_s;
-    const QString managerPath = u"/org/freedesktop/login1"_s;
-    const QString managerIface = u"org.freedesktop.login1.Manager"_s;
-
-    // Phase two: Lock/Unlock the resolved session object.
-    auto lockSession = [this, locked, bus, service](const QDBusObjectPath &path) {
-        QDBusMessage msg =
-            QDBusMessage::createMethodCall(service, path.path(), u"org.freedesktop.login1.Session"_s, locked ? u"Lock"_s : u"Unlock"_s);
-        auto *watcher = new QDBusPendingCallWatcher(bus.asyncCall(msg), this);
-        connect(watcher, &QDBusPendingCallWatcher::finished, this, [locked](QDBusPendingCallWatcher *self) {
+    QDBusMessage msg = QDBusMessage::createMethodCall(service, u"/org/freedesktop/login1"_s, u"org.freedesktop.login1.Manager"_s, u"GetSession"_s);
+    msg.setArguments({sessionId});
+    auto *watcher = new QDBusPendingCallWatcher(bus.asyncCall(msg), this);
+    connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, locked, bus, service](QDBusPendingCallWatcher *self) {
+        const QDBusPendingReply<QDBusObjectPath> reply = *self;
+        self->deleteLater();
+        if (reply.isError()) {
+            qWarning() << "krdp: could not resolve the logind session to" << (locked ? "lock" : "unlock") << ":" << reply.error().message();
+            return;
+        }
+        QDBusMessage lockMsg =
+            QDBusMessage::createMethodCall(service, reply.value().path(), u"org.freedesktop.login1.Session"_s, locked ? u"Lock"_s : u"Unlock"_s);
+        auto *lockWatcher = new QDBusPendingCallWatcher(bus.asyncCall(lockMsg), this);
+        connect(lockWatcher, &QDBusPendingCallWatcher::finished, this, [locked](QDBusPendingCallWatcher *self) {
             const QDBusPendingReply<> reply = *self;
             if (reply.isError()) {
                 qWarning() << "krdp: could not" << (locked ? "lock" : "unlock") << "the logind session:" << reply.error().message();
             }
             self->deleteLater();
         });
-    };
-
-    // Phase one fallback: resolve the session by our PID.
-    auto resolveByPid = [this, locked, bus, service, managerPath, managerIface, lockSession]() {
-        QDBusMessage msg = QDBusMessage::createMethodCall(service, managerPath, managerIface, u"GetSessionByPID"_s);
-        msg.setArguments({static_cast<quint32>(QCoreApplication::applicationPid())});
-        auto *watcher = new QDBusPendingCallWatcher(bus.asyncCall(msg), this);
-        connect(watcher, &QDBusPendingCallWatcher::finished, this, [locked, lockSession](QDBusPendingCallWatcher *self) {
-            const QDBusPendingReply<QDBusObjectPath> reply = *self;
-            if (reply.isError()) {
-                qWarning() << "krdp: could not resolve a logind session to" << (locked ? "lock" : "unlock") << ":" << reply.error().message();
-            } else {
-                lockSession(reply.value());
-            }
-            self->deleteLater();
-        });
-    };
-
-    // Phase one: resolve by $XDG_SESSION_ID, else fall back to PID.
-    const QString sessionId = qEnvironmentVariable("XDG_SESSION_ID");
-    if (!sessionId.isEmpty()) {
-        QDBusMessage msg = QDBusMessage::createMethodCall(service, managerPath, managerIface, u"GetSession"_s);
-        msg.setArguments({sessionId});
-        auto *watcher = new QDBusPendingCallWatcher(bus.asyncCall(msg), this);
-        connect(watcher, &QDBusPendingCallWatcher::finished, this, [resolveByPid, lockSession](QDBusPendingCallWatcher *self) {
-            const QDBusPendingReply<QDBusObjectPath> reply = *self;
-            if (reply.isError()) {
-                resolveByPid();
-            } else {
-                lockSession(reply.value());
-            }
-            self->deleteLater();
-        });
-    } else {
-        resolveByPid();
-    }
+    });
 }
 
 void SessionController::onNewConnection(KRdp::RdpConnection *newConnection)
@@ -231,17 +200,16 @@ void SessionController::onNewConnection(KRdp::RdpConnection *newConnection)
     wrapper->session->start();
 
     connect(wrapper.get(), &SessionWrapper::connectionDestroyed, this, [this](SessionWrapper *wrapper) {
-        // Lock the machine as the last client leaves.
-        if (m_wrappers.size() == 1 && m_wrappers.front().get() == wrapper) {
-            setSessionLocked(true);
-        }
-
         m_wrappers.erase(std::remove_if(m_wrappers.begin(),
                                         m_wrappers.end(),
                                         [wrapper](std::unique_ptr<SessionWrapper> &entry) {
                                             return entry.get() == wrapper;
                                         }),
                          m_wrappers.end());
+
+        if (m_wrappers.empty()) {
+            setSessionLocked(true);
+        }
     });
 
     connect(wrapper.get(), &SessionWrapper::sessionError, this, [newConnection] {