refactor: switch treeland IPC to Qt Remote Objects by zccrs · Pull Request #96 · linuxdeepin/ddm

zccrs · 2026-06-02T11:21:36Z

Summary

replace the treeland Wayland client IPC in ddm with a Qt Remote Objects replica
remove the generated treeland-ddm Wayland client protocol from ddm
keep dde-seatd control socket handling unchanged

Testing

cmake --build build

sourcery-ai

Sorry @zccrs, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

deepin-ci-robot · 2026-06-02T11:21:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zccrs

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot

Pull request overview

Refactors ddm’s Treeland IPC from a generated Wayland client protocol to a Qt Remote Objects (QtRO) replica, aligning the daemon-side connector with Qt’s IPC stack and removing the build-time Wayland-client codegen dependency.

对 ddm 与 Treeland 的 IPC 进行重构：从原先生成的 Wayland client 协议切换为 Qt Remote Objects（QtRO）replica，使 daemon 侧连接器改用 Qt 的 IPC 机制，并移除构建期的 Wayland-client 协议代码生成依赖。

Changes:

Introduces a QtRO replica definition (treelandddmremote.rep) and switches TreelandConnector to call into the generated TreelandDDMRemoteReplica.
Removes Wayland client protocol code generation/linkage from CMake and adds Qt6 RemoteObjects dependency.
Updates distro/CI dependencies to include Qt Remote Objects (Debian Build-Depends, Arch workflow).

变更点：

新增 QtRO replica 定义（treelandddmremote.rep），并让 TreelandConnector 通过生成的 TreelandDDMRemoteReplica 发起调用。
从 CMake 中移除 Wayland client 协议生成/链接，并新增 Qt6 RemoteObjects 依赖。
更新发行版/CI 依赖（Debian Build-Depends、Arch workflow）以包含 Qt Remote Objects。

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`src/daemon/treelandddmremote.rep`	Adds Qt Remote Objects replica interface for Treeland control methods.
`src/daemon/TreelandConnector.h`	Updates connector interface/members for QtRO + local socket usage.
`src/daemon/TreelandConnector.cpp`	Replaces Wayland IPC calls with QtRO replica acquisition and invocation logic.
`src/daemon/CMakeLists.txt`	Hooks `qt_add_repc_replicas()` and links `Qt6::RemoteObjects`; removes Wayland-generated source.
`CMakeLists.txt`	Drops Wayland client/TreelandProtocols discovery; adds Qt6 RemoteObjects requirement.
`debian/control`	Adds `qt6-remoteobjects-dev` to Build-Depends.
`.github/workflows/ddm-archlinux-build.yml`	Installs Qt Remote Objects dependency for Arch CI build.

Comments suppressed due to low confidence (1)

src/daemon/TreelandConnector.h:43

Include guards need to be closed at the end of the header (#endif).

建议：在文件末尾补上 #endif，与文件开头的 #ifndef/#define 成对。

};
}

zccrs · 2026-06-03T04:08:02Z

已按这轮 review 收口：

TreelandConnector 里重复实现的 dde-seatd control socket / grouped-VT 逻辑已全部删除，VT 事件统一回到现有 DdeSeatdControl + SeatManager::handleVtChanged()。
treelandddmremote.rep 已删掉未再使用的 activateSession/deactivateSession/enableRender/disableRender，两端接口只保留当前实际使用的 switchToGreeter/switchToUser。
TreelandConnector.h 已改为 #pragma once，空的 setSignalHandler() 和未使用的 grouped-VT 包装接口一并移除。
debian/control、Arch/Deepin CI 中残留的 libwayland-dev / treeland-protocols 构建依赖已删除；本分支当前已不再使用这些构建输入。

之前 Copilot 提到的同步 control-socket I/O、协议版本、getpwnam()、TOCTOU 等问题，都是基于那套已经删掉的重复实现，当前 head 已不再适用。#endif 那条也是误报：这里现在用的是 #pragma once。

zccrs · 2026-06-04T02:16:34Z

+
+            connect(socket, &QLocalSocket::disconnected, socket, &QLocalSocket::deleteLater);
+            connect(socket, &QLocalSocket::disconnected, this, [this] {
+                Q_EMIT disconnected();


这样就不用写成一个lambda了，直接链接就行

已处理：去掉了这里的 lambda，QLocalSocket::disconnected 现在直接连接到 deleteLater()；同时不再额外通过 SocketServer::disconnected 做日志胶水。

zccrs · 2026-06-04T02:17:59Z

+    void SocketServer::login(QString user, QString password, int sessionType, QString sessionFile) {
+        qDebug() << "Message received from greeter: Login";
+        Session session(static_cast<Session::Type>(sessionType), sessionFile);
+        Q_EMIT loginRequested(user, password, session);


不要搞这种设计呀，不能直接调用实现方的 login 方法吗？不要用这种发出信号，再在外面做一个胶水层的方式，多此一举。

已处理：SocketServer 现在持有对应的 Display，远端 login/logout/lock/unlock/connectGreeter 请求会直接调用实现方方法，不再先发 loginRequested 等信号再由外层胶水转发。

zccrs · 2026-06-04T02:23:50Z

+// SPDX-License-Identifier: GPL-2.0-or-later
+
+class GreeterDDMRemote {
+    PROP(QString hostName)


这些属性是不是应该声明为只读的？

已处理：这些属性已改成 SOURCEONLYSETTER。这样 ddm source 侧仍可更新属性值，但 replica/greeter 侧只能读取和订阅变更，语义上是远端只读。

deepin-bot · 2026-06-04T08:43:17Z

TAG Bot

New tag: 0.3.5
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #100

zccrs · 2026-06-04T07:22:33Z

        for (Auth *auth : std::as_const(auths)) {
            if (auth->sessionOpened)
-                writer << quint32(DaemonMessages::UserLoggedIn) << auth->user << auth->xdgSessionId;
+                m_socketServer->userLoggedIn(auth->user, auth->xdgSessionId);


这个做法挺奇怪的，应该是treeland启动后，主动通过ddm提供的SLOT来获取一个已登录的用户列表信息，而不是ddm自己发送信号通知这些已经登录的。信号应该只用来真的做通知，通知上一次登录请求的结果。

zccrs · 2026-06-04T09:10:20Z

+                                                                     authPtr);
+            session->setObjectName(QStringLiteral("logindSessionWatcher"));
+            connect(session, &OrgFreedesktopLogin1SessionInterface::Lock, this, [this, authPtr] {
+                if (authPtr)


如果不是当前的sessioin不要关心这个请求，因为treeland是global的，这时候如果锁屏，会出现a用户请求锁屏，结果把b用户锁定了的情况。如果用户当前不是激活用户，当前切换到b用户时，无论如何都会锁屏，所以这里的lock操作可以认为已经完成。

zccrs · 2026-06-04T09:11:40Z

+                if (authPtr)
+                    m_socketServer->setSessionLocked(authPtr->xdgSessionId, true);
+            });
+            connect(session, &OrgFreedesktopLogin1SessionInterface::Unlock, this, [this, authPtr] {


同上，我们不应该提供unlock操作，因为treeland锁屏和greeter是一个东西，进入了锁屏就没有当前用户的含义了，不应该能被用户进程请求解锁。

zccrs · 2026-06-04T09:33:16Z


+        setHostName(daemonApp->hostName());
+        const auto capabilities = daemonApp->powerManager()->capabilities();
+        setCanPowerOff(capabilities & Capability::PowerOff);


不应该搞成set的，应该重写属性的get方法，在里面返回这个数据。但是 daemonApp->powerManager() 的设计有些问题，这里是分开的几个属性，但是daemonApp->powerManager()里是把原本分开的东西聚合到了一起，如果这里分开的属性被分别调用，那daemonApp->powerManager()那边会有多次的重复调用，这一点也要优化

zccrs · 2026-06-04T09:43:37Z

+
+    void SocketServer::addUserSession(const QString &user, int sessionId) {
+        if (sessionId > 0)
+            userSessionAdded(user, sessionId);


缺少emit，而且没看出来这个信号有啥用

zccrs · 2026-06-04T09:43:55Z

+            userSessionAdded(user, sessionId);
+    }
+
+    void SocketServer::removeUserSession(const QString &user, int sessionId) {


zccrs · 2026-06-04T09:43:59Z

+            userSessionRemoved(user, sessionId);
+    }
+
+    void SocketServer::setSessionLocked(int sessionId, bool locked) {


Replace the greeter IPC bridge with Qt Remote Objects endpoints. Use DDMRemote and TreelandRemote as cross-process call contracts. 将 greeter IPC 桥接改为 Qt Remote Objects 端点。使用 DDMRemote 和 TreelandRemote 作为跨进程调用契约。 Log: 切换 Treeland 与 DDM 的 IPC 到 Qt Remote Objects Influence: 规范远程对象和 socket 命名，影响 DDM 与 Treeland 的登录/切换通信。

deepin-ci-robot · 2026-06-04T11:17:56Z

deepin pr auto review

这是一次非常优秀的重构！此次代码审查的核心变更是将 ddm (Deepin Display Manager) 与 treeland 之间的底层 Wayland 协议通信（基于 C 接口的 wayland-client 和自定义 XML 协议）替换为了基于 Qt Remote Objects (QtRO) 的高层 IPC 通信。这极大地简化了代码逻辑，去除了繁琐的 SocketWriter/SocketServer 手动消息解析，并统一了锁屏状态同步机制。

以下是对本次重构代码的详细审查意见，分为语法逻辑、代码质量、代码性能和代码安全四个方面：

一、语法与逻辑

QtRO SLOT 返回值语义问题 (高优先级)
- 问题：在 ddmremote.rep 中，所有的 SLOT 都声明了 bool 返回值（如 SLOT(bool login(...))），并且在 SocketServer.cpp 中所有实现均直接 return true;。在 Qt Remote Objects 中，SLOT 的返回值会通过 IPC 同步返回给调用方（Replica端）。
- 风险：如果 login 验证失败，调用方（如 greeter）依然会收到 true，这会导致状态不一致。另外，如果 IPC 通信中断，QtRO 默认会返回默认构造的 bool（即 false）。
- 建议：
  - 如果不需要向调用方同步返回执行结果，建议将 .rep 文件和 C++ 实现中的返回值改为 void，改用 loginFailed 等 Signal 来异步通知结果（这也是 UI 程序的标准做法）。
  - 如果确实需要同步返回，请在逻辑中返回真实的执行状态，例如 login 失败时应返回 false。
Display::onTreelandLockStateChanged 中的死循环风险 (高优先级)
- 问题：当 locked == false 且 m_treelandLocked == true 时，代码会调用 manager.UnlockSession。但 logind 收到 Unlock 后，会再次发射 Unlock 信号，这会被 watchUserSession 捕获，再次进入 Unlock 逻辑，形成 Ping-Pong 死循环。
- 评价：代码中通过 windowMs = 2000 和 maxLockBacks = 3 进行了限流，这是一个很好的防御性编程，避免了无限死循环。
- 建议：限流机制虽然有效，但逻辑略显复杂。更根本的解决方案是：在 Display::onTreelandLockStateChanged(false) 时，直接更新 m_treelandLocked = false，并主动断开或屏蔽对应 logind session 的 Unlock 信号监听，因为此时解锁源头是 treeland，不需要再反馈给 logind。
TreelandConnector::ensureRemote 阻塞事件循环 (中优先级)
- 问题：m_remoteSocket->waitForConnected(3000) 和 m_remoteReplica->waitForSource(3000) 是阻塞式调用，会冻结 UI 或 D-Bus 事件循环最多 6 秒。
- 建议：Daemon 进程虽然对阻塞的容忍度比 UI 进程高，但在系统启动等关键路径上，阻塞 6 秒可能导致 D-Bus 超时。建议重构为基于状态机的异步连接方式，监听 QLocalSocket::connected 和 QRemoteObjectReplica::initialized 信号。
Session::Type 强转风险 (低优先级)
- 问题：SocketServer::login 中：Session session(static_cast<Session::Type>(sessionType), sessionFile);。如果 sessionType 是无效的整数值，强转会导致未定义行为。
- 建议：增加边界检查，如 if (sessionType < 0 || sessionType >= SessionTypeMax) return false;。

二、代码质量

架构解耦大幅提升 (优点)
- 移除了 wayland-client 依赖，用 QtRO 替代，使得 ddm 不再需要链接 libwayland-client 和 treeland-protocols-git，大大降低了构建依赖复杂度。
- 移除了 SocketWriter 和手动 QDataStream 解析，消除了大量的样板代码，代码可读性直线上升。
CI/CD 构建脚本清理 (优点)
- 移除了 Arch Linux 和 Deepin 构建流中手动 clone 和 build treeland-protocols 的步骤，使 CI 脚本更加清爽，构建时间缩短。
Display::watchUserSession 中的内存与生命周期管理
- 问题：auto *session = new OrgFreedesktopLogin1SessionInterface(..., authPtr); 将 authPtr 设置为 parent。但在 unwatchUserSession 中，又通过 findChildren 找到并 deleteLater()。
- 建议：当 auth 对象销毁时，作为其 children 的 session 也会自动销毁。因此 unwatchUserSession 中的手动删除其实是多余的，除非在 auth 析构前有迫切清理的需求。建议统一依赖 Qt 的对象树机制，移除 unwatchUserSession 中的手动 delete 逻辑，避免 Double Free 或悬垂指针风险。
Magic Number 提取为常量
- constexpr int windowMs = 2000; 和 constexpr int maxLockBacks = 3; 提取为常量是好的实践，建议将其提升为类的成员变量或静态常量，以便未来调整，并加上注释说明为何是 2000ms 和 3次。

三、代码性能

频繁构造 D-Bus Interface 对象 (中优先级)
- 问题：OrgFreedesktopLogin1ManagerInterface manager(...) 在 onTreelandLockStateChanged、watchUserSession (Unlock分支) 中被频繁创建。D-Bus Interface 的构造涉及字符串解析和内部注册，开销不小。
- 建议：在 Display 类中将 manager 作为成员变量缓存起来，或者使用静态/线程局部变量复用。
TreelandDisplayServer::activateUser 的逻辑优化
- 问题：原代码是向所有 greeter sockets 广播，现在改为直接调用 treelandConnector。这是正确的性能优化，因为切换用户本身就是一个全局动作，不需要遍历列表。
QtRO 底层通信效率
- QtRO 内部基于 QLocalSocket，并使用 QDataStream 进行序列化。相比于原先直接写二进制流，QtRO 增加了少量的元数据开销。但对于登录管理器这种低频 IPC 场景，这点性能损耗完全可以忽略不计，换来的是极高的开发效率。

四、代码安全

本地 Socket 权限隔离 (高优先级)
- 问题：m_server->setSocketOptions(QLocalServer::UserAccessOption); 保留了原有的权限控制，这很好，确保只有同用户能连接。但 treelandRemoteSocketName 和 ddmRemoteSocketName 是硬编码的。
- 风险：如果 ddm 以 root 运行，而 treeland 以普通用户运行，UserAccessOption 可能会导致跨用户连接失败。如果使用 WorldAccessOption，则任何本地用户都能伪造 greeter 发送 login 或 powerOff 指令。
- 建议：
  - 确认 ddm 与 treeland 的运行用户。如果同属 root 或同用户，当前逻辑安全。
  - 如果涉及跨用户，建议使用 Linux 特定的权限机制（如 SELinux/AppArmor）保护 Socket 文件，或者在 QtRO 连接建立时增加基于 PID/UID 的校验（通过 SO_PEERCRED 获取对端凭据）。
密码在内存中的明文传递
- 问题：SLOT(bool login(QString user, QString password, ...)) 将密码作为 QString 传递。
- 风险：QString 内部会在堆上分配内存，且释放后不会立即清零。通过本地 Socket 传输时，密码可能在内核的 Socket Buffer 中留下痕迹。
- 建议：对于本地登录管理器，这是可接受的实现复杂度与安全性的折中。如果追求极致安全，应使用 QByteArray 并在使用后调用 memset_s 或类似安全擦除方法覆写内存，且 .rep 文件中应传递 QByteArray。
移除了 unlock 的密码校验 (逻辑安全隐患)
- 问题：旧代码中，unlock 操作需要传入 user 和 password，并调用 auth.authenticate 进行验证。新代码中完全移除了 unlock 槽函数，解锁完全依赖 treeland 的 lockStateChanged(false) 信号。
- 风险：这意味着解锁动作的信任完全委托给了 treeland。如果 treeland 被攻破或发出误信号，ddm 会无条件解锁会话。
- 建议：确认 treeland 自身是否具备严格的解锁认证机制。如果 treeland 已经安全完成了密码校验，那么 ddm 不再重复校验是合理的，可以避免密码在多个进程间流转。

总结

本次重构质量很高，将底层且容易出错的 Wayland C 绑定替换为了现代、类型安全的 Qt Remote Objects 框架，是一次非常成功的重构。

建议修改的优先级：

必须修改：明确 QtRO Slot 的返回值语义（改为 void 或返回真实状态），避免调用方产生错误预期；检查 Unlock 信号的 Ping-Pong 循环问题，建议在收到 Treeland 解锁信号时屏蔽 Logind 的 Unlock 信号回传。
建议修改：避免在事件循环中使用 waitForConnected / waitForSource 阻塞；将频繁创建的 D-Bus Manager 提取为成员变量。
安全确认：确认 ddm 与 treeland 的 Socket 连接权限配置是否符合运行环境的安全隔离要求。

sourcery-ai Bot reviewed Jun 2, 2026

View reviewed changes

zccrs mentioned this pull request Jun 2, 2026

refactor: switch ddm IPC to Qt Remote Objects linuxdeepin/treeland#926

Open

zccrs force-pushed the qremoteobject-treeland-ipc branch 3 times, most recently from 9c5c73a to 38f5a23 Compare June 3, 2026 02:47

zccrs requested a review from Copilot June 3, 2026 02:48

Copilot started reviewing on behalf of zccrs June 3, 2026 02:48 View session

Copilot AI reviewed Jun 3, 2026

View reviewed changes

zccrs commented Jun 3, 2026

View reviewed changes

Comment thread debian/control Outdated

Comment thread src/daemon/TreelandConnector.cpp Outdated

Comment thread src/daemon/treelandddmremote.rep Outdated

Comment thread src/daemon/treelandddmremote.rep Outdated

zccrs force-pushed the qremoteobject-treeland-ipc branch from 38f5a23 to d13b4bb Compare June 3, 2026 04:04

zccrs force-pushed the qremoteobject-treeland-ipc branch from d13b4bb to 40ceee2 Compare June 3, 2026 10:36

zccrs commented Jun 4, 2026

View reviewed changes

Comment thread src/daemon/TreelandConnector.cpp Outdated

Comment thread src/daemon/TreelandConnector.cpp Outdated

zccrs commented Jun 4, 2026

View reviewed changes

zccrs force-pushed the qremoteobject-treeland-ipc branch 7 times, most recently from f308bc0 to 1ca093d Compare June 4, 2026 08:37

zccrs commented Jun 4, 2026

View reviewed changes

zccrs force-pushed the qremoteobject-treeland-ipc branch 4 times, most recently from ca02a71 to b55c2cb Compare June 4, 2026 11:12

zccrs force-pushed the qremoteobject-treeland-ipc branch from b55c2cb to 63e12e6 Compare June 4, 2026 11:16

Conversation

zccrs commented Jun 2, 2026

Summary

Testing

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

deepin-ci-robot commented Jun 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zccrs commented Jun 3, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

deepin-bot Bot commented Jun 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deepin-ci-robot commented Jun 4, 2026

deepin pr auto review

一、 语法与逻辑

二、 代码质量

三、 代码性能

四、 代码安全

总结

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

一、语法与逻辑

二、代码质量

三、代码性能

四、代码安全