Skip to content

refactor(vt): route VT control through dde-seatd#95

Merged
zccrs merged 1 commit into
linuxdeepin:masterfrom
zccrs:vt-control-socket-refactor
Jun 3, 2026
Merged

refactor(vt): route VT control through dde-seatd#95
zccrs merged 1 commit into
linuxdeepin:masterfrom
zccrs:vt-control-socket-refactor

Conversation

@zccrs
Copy link
Copy Markdown
Member

@zccrs zccrs commented Jun 2, 2026

Summary

  • replace ddm direct VT discovery and switching with dde-seatd control-socket calls
  • reduce VirtualTerminal to tty path handling only and remove the old VT ioctl helpers
  • drop unused custom signal glue that only existed for the deleted VT path

Dependency

  • depends on the matching dde-seatd control-socket extension PR

Test

  • cmake --build build

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Copy Markdown

[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:

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

@zccrs zccrs force-pushed the vt-control-socket-refactor branch from f650b58 to 7bc2336 Compare June 2, 2026 06:34
@zccrs zccrs requested a review from Copilot June 2, 2026 06:39
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors VT (virtual terminal) handling in ddm to route VT discovery and switching through the dde-seatd control socket, removing the previous direct VT ioctl/signal-based implementation.
本 PR 重构了 ddm 的 VT(虚拟终端)处理流程:通过 dde-seatd 控制 socket 来完成 VT 查询与切换,并移除此前基于 VT ioctl/信号的实现。

Changes:

  • Replace direct VT discovery/switching with TreelandConnector control-socket requests (activeVt, findAvailableVt, switchVt).
    TreelandConnector 的控制 socket 请求替代直接 VT 探测/切换(activeVt / findAvailableVt / switchVt)。
  • Reduce VirtualTerminal to only provide tty path formatting, removing legacy VT helpers.
    VirtualTerminal 收敛为仅提供 tty 路径格式化,删除旧的 VT 辅助逻辑。
  • Remove now-unused custom signal handling glue from SignalHandler.
    删除 SignalHandler 中已不再需要的自定义信号处理代码。

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/daemon/VirtualTerminal.h Removes legacy VT helper API, keeps only path(int).(删除旧 VT API,仅保留路径函数)
src/daemon/VirtualTerminal.cpp Drops ioctl-based VT implementation; keeps tty path logic only.(移除 ioctl 实现,仅保留路径处理)
src/daemon/TreelandConnector.h Adds VT control-socket API surface.(新增 VT 控制 socket 接口声明)
src/daemon/TreelandConnector.cpp Implements VT control-socket protocol helpers and VT operations.(实现控制协议与 VT 操作)
src/daemon/Display.cpp Switches greeter/session VT operations to TreelandConnector.(改用 TreelandConnector 进行 VT 切换/分配)
src/daemon/Auth.cpp Requests VT switch via TreelandConnector before opening PAM session.(打开会话前通过 TreelandConnector 请求切 VT)
src/common/SignalHandler.h Removes custom-signal API and state.(移除自定义信号相关接口与状态)
src/common/SignalHandler.cpp Removes custom-signal socketpair + notifier logic.(移除自定义信号 socketpair/notifier 逻辑)

Comment thread src/daemon/TreelandConnector.cpp Outdated
Comment thread src/daemon/TreelandConnector.cpp Outdated
@zccrs zccrs force-pushed the vt-control-socket-refactor branch 3 times, most recently from 2a00f90 to b5f1d1c Compare June 2, 2026 08:57
Comment thread src/daemon/SeatManager.cpp Outdated
@zccrs zccrs force-pushed the vt-control-socket-refactor branch 4 times, most recently from fa66be1 to ecd0a9b Compare June 2, 2026 09:42
@zccrs zccrs requested a review from Copilot June 2, 2026 11:45
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

Comment thread src/daemon/DdeSeatdControl.h
Comment thread src/daemon/TreelandConnector.h
Comment thread src/daemon/TreelandConnector.cpp
Comment thread src/daemon/TreelandConnector.cpp Outdated
Comment thread src/daemon/DdeSeatdControl.cpp
Comment thread src/daemon/DaemonApp.cpp Outdated
Comment thread src/daemon/Display.cpp
@zccrs zccrs force-pushed the vt-control-socket-refactor branch from ecd0a9b to 102df00 Compare June 2, 2026 11:58
@zccrs
Copy link
Copy Markdown
Member Author

zccrs commented Jun 2, 2026

Addressed the valuable review findings on this branch:

  • validate createGroupVt() responses and reject invalid event.vt <= 0 before the value can propagate upward
  • listen for QLocalSocket::errorOccurred on the dde-seatd event socket and tear it down cleanly on transport errors
  • replace the fixed-size string magic number with a named constant in DdeSeatdControl
  • remove the mutable cache pattern from TreelandConnector::mainPid() and make it an ordinary non-const cached lookup
  • replace the hard-coded greeter user check in SeatManager with a shared file-local constant

I also re-checked the two Copilot comments on blocking / timeout handling:

  • the timeout path in waitForReadable() already sets errno = ETIMEDOUT in the current branch
  • the old long blocking switchVt(..., wait=true) polling path is no longer present in the current branch, so that comment is not applicable anymore

Local validation: cmake --build build (from the PR worktree build dir).

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Comment thread src/daemon/TreelandConnector.cpp
Comment thread src/daemon/TreelandConnector.cpp
Comment thread src/daemon/TreelandConnector.cpp Outdated
Comment thread src/daemon/DdeSeatdControl.cpp
Comment thread src/daemon/DaemonApp.cpp Outdated
@zccrs zccrs force-pushed the vt-control-socket-refactor branch from 102df00 to db95043 Compare June 2, 2026 13:17
@zccrs
Copy link
Copy Markdown
Member Author

zccrs commented Jun 2, 2026

@copilot review

@zccrs zccrs force-pushed the vt-control-socket-refactor branch from db95043 to d7599df Compare June 2, 2026 13:19
@zccrs zccrs requested a review from Copilot June 2, 2026 13:24
@zccrs zccrs review requested due to automatic review settings June 2, 2026 13:29
@zccrs zccrs force-pushed the vt-control-socket-refactor branch from d7599df to 91c70dd Compare June 3, 2026 01:36
@zccrs zccrs requested a review from Copilot June 3, 2026 01:43
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Comment thread src/daemon/TtyUtils.h
@zccrs
Copy link
Copy Markdown
Member Author

zccrs commented Jun 3, 2026

@copilot review

@zccrs zccrs force-pushed the vt-control-socket-refactor branch from 91c70dd to 3b16901 Compare June 3, 2026 01:52
@zccrs
Copy link
Copy Markdown
Member Author

zccrs commented Jun 3, 2026

@copilot review

@zccrs zccrs force-pushed the vt-control-socket-refactor branch from 3b16901 to a6c67bf Compare June 3, 2026 01:56
@zccrs
Copy link
Copy Markdown
Member Author

zccrs commented Jun 3, 2026

@copilot review

@zccrs zccrs force-pushed the vt-control-socket-refactor branch from a6c67bf to 986c0d3 Compare June 3, 2026 01:58
@zccrs
Copy link
Copy Markdown
Member Author

zccrs commented Jun 3, 2026

@copilot review

Move VT discovery and switching out of DDM and call dde-seatd through the control socket instead.

将VT查询与切换从DDM中移出,改为通过control socket调用dde-seatd。

Keep only tty path/control-tty handling in DDM and drop the old VirtualTerminal VT ioctl helpers plus unused signal glue.

在DDM中仅保留tty路径与control tty处理,删除旧的VirtualTerminal VT ioctl辅助逻辑及无用signal glue。

Log: 收口DDM中的VT控制

Influence: DDM不再直接执行VT_ACTIVATE等VT ioctl,VT状态机统一由dde-seatd处理。
@zccrs zccrs force-pushed the vt-control-socket-refactor branch from 986c0d3 to e28afcf Compare June 3, 2026 02:22
@zccrs
Copy link
Copy Markdown
Member Author

zccrs commented Jun 3, 2026

@copilot review

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这是一次非常出色且大规模的重构。核心思路是将原本通过内核 ioctl 和 Unix 信号处理的虚拟终端(VT)管理逻辑,迁移到基于 dde-seatd 的 IPC Socket 通信架构上。这不仅解耦了代码,还解决了 Linux 控制台驱动中常见的 VT_AUTO/KD_GRAPHICS 死锁问题,并移除了不安全的异步信号处理。

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

一、 语法与逻辑

  1. DdeSeatdControlm_activeVt 缓存同步潜在逻辑漏洞

    • 位置: DdeSeatdControl.cpp -> activeVt()requestSwitchVt()
    • 问题: 在 requestSwitchVt 中,如果请求的 VT 恰好是当前的 m_activeVt,代码设置 m_pendingVt = -1。但是在 activeVt() 中,如果 m_activeVt <= 0,会重新查询,查询后如果 m_activeVt == m_pendingVt,则将 m_pendingVt 置为 -1。
    • 风险: 这套状态机逻辑略显晦涩。如果 dde-seatd 在外部(如用户通过 Ctrl+Alt+Fx)切换了 VT,而 DdeSeatdControl 尚未读取到 EventSocket 的推送事件,此时 m_activeVt 的缓存就是脏数据。依赖本地缓存的 m_pendingVt 进行状态判断可能会导致逻辑错乱。
    • 建议: 明确 m_pendingVt 的语义(是“我们请求去但还没确认到达的 VT”还是“非当前活动VT的请求目标”)。建议在 requestSwitchVt 时,只要发起请求就记录 m_pendingVt = vt,统一在 handleEventSocket 收到 ControlVtChanged 时去清理 m_pendingVt
  2. TreelandConnector 中 Wayland 事件分发死循环风险

    • 位置: TreelandConnector.cpp -> connect()
    • 代码:
      while (wl_display_dispatch_pending(m_display) > 0) {
      }
      wl_display_flush(m_display);
    • 问题: 如果 Wayland 协议栈中存在不断产生待处理事件的逻辑(例如某些错误重试机制),这个没有退出条件或最大迭代次数的 while 循环可能会导致进程卡死在这里。
    • 建议: 增加最大循环次数保护,或者直接使用 wl_display_roundtrip(m_display) 来确保所有初始请求被处理完毕,因为上面已经调用过一次 roundtrip 了。
  3. SeatManager 中 VT 切换逻辑可能丢失状态

    • 位置: SeatManager.cpp -> handleVtChanged()
    • 代码:
      if (newIsGreeterVt || newUser == QString::fromLatin1(greeterUserName)) {
          daemonApp->treelandConnector()->switchToGreeter();
      } else {
          daemonApp->treelandConnector()->switchToUser(newUser);
      }
    • 问题: 如果 newUser 为空(例如在数据结构中找不到对应的用户),switchToUser 会传递一个空字符串给 Treeland,这可能导致 Wayland 服务端无法正确激活会话。
    • 建议: 增加对 newUser.isEmpty() 的判断,如果找不到用户,应当记录警告并尝试回退到 Greeter 或忽略该请求。

二、 代码质量

  1. DdeSeatdControl 的同步阻塞调用可能拖慢 UI/主事件循环

    • 位置: DdeSeatdControl.cpp -> queryActiveVt(), findAvailableVt(), requestSwitchVt(), createGroupVt()
    • 问题: 这些方法内部都使用了 connectUnixSocket + sendControlRequest + waitForReadable + readControlResponse 的同步阻塞模式,且超时时间设置为 3000 ms。这些函数是在 Qt 的主事件循环中被调用的(如 Display::start()),如果 dde-seatd 响应缓慢,会导致整个 DDM 守护进程主线程卡死 3 秒,用户会看到登录界面冻结。
    • 建议:
      • 方案A(推荐):将这些操作移至子线程,或者使用 QLocalSocket 的异步机制重写请求-响应逻辑。
      • 方案B(快速修复):如果确定 dde-seatd 必须是本地且极快响应的,将超时时间从 3000 ms 降低到 100-500 ms,并在注释中明确说明这必须是本地极速 IPC。
  2. TtyUtils.h 中的魔法数字与边界检查

    • 位置: TtyUtils.h -> path()
    • 代码:
      const char c = tty <= 10 ? static_cast<char>('0' + (tty - 1))
                               : static_cast<char>('a' + (tty - 11));
    • 问题: 如果传入的 tty 是负数或者过大(例如 tty > 36),会导致字符溢出或越界。原代码中虽然没有严格检查,但作为新提炼的公共工具类,应当具备健壮性。
    • 建议: 增加边界检查:
      if (tty < 1 || tty > 36) {
          qWarning("Invalid tty number: %d", tty);
          return QStringLiteral("/dev/ttyv?");
      }
  3. SignalHandler 清理了不必要的自定义信号机制,非常棒

    • 移除了 sigcustomFdcustomSignalHandler,彻底消除了在 Unix 信号处理函数中调用非异步安全函数(如 write 可能触发死锁)的风险。这是极佳的改进。

三、 代码性能

  1. Socket 连接复用问题

    • 位置: DdeSeatdControl.cpp -> queryActiveVt(), findAvailableVt(), requestSwitchVt(), createGroupVt()
    • 问题: 每次进行控制面通信时,都会新建一个 Unix Socket 连接(connectUnixSocket),完成一次请求-响应后立即 close(fd)。频繁的 socket() -> connect() -> close() 会带来较大的内核开销(包括三次握手、文件描述符分配释放等)。
    • 建议: 考虑在 DdeSeatdControl 内部维护一个长连接的控制 Socket(类似于 Event Socket 的做法),复用该连接进行所有的 Request/Response 通信。如果担心连接断开,可以实现自动重连逻辑。
  2. QByteArray::remove(0, messageSize) 的开销

    • 位置: DdeSeatdControl.cpp -> handleEventSocket()
    • 问题: m_eventBuffer.remove(0, messageSize) 会将缓冲区后部的所有数据向前搬移,如果缓冲区积累了大量数据,这会引发多次内存拷贝。
    • 建议: 对于低频的 VT 事件,这不会成为性能瓶颈,可以保留。如果追求极致,可以使用 QByteArray 配合内部索引或 QQueue 来避免内存搬移。

四、 代码安全

  1. Unix Socket 路径硬编码与权限问题

    • 位置: DdeSeatdControl.cpp -> controlSocketPath
    • 代码: static constexpr auto controlSocketPath = "/run/dde-seatd-control.sock";
    • 风险: /run 目录下的 Socket 文件如果权限配置不当(例如被设置为 0777),任何本地用户都可以向其发送伪造的 ControlSwitchVtControlCreateGroupVt 请求,导致未授权的 VT 切换或会话劫持。
    • 建议:
      • 确保 dde-seatd 在创建该 Socket 时,权限掩码设置为 07700700
      • DdeSeatdControl 发送请求(特别是 createGroupVt)时,可以考虑增加 PEERCRED 验证,确保连接方确实是 DDM 守护进程本身或具有特权的进程。
  2. ControlHeader 缺乏有效载荷校验

    • 位置: DdeSeatdControl.cpp -> readControlResponse()
    • 问题: 虽然检查了 responseHeader.size != payloadSize,但如果恶意服务端返回了巨大的 size,直接传入 recvAll 可能会导致分配超大内存并读取溢出。
    • 建议: 在 readControlResponsehandleEventSocket 中,已经存在 maxControlPayloadSize = 4096 的校验,这是很好的。但建议在 readControlResponse 中也显式检查 payloadSize > maxControlPayloadSize,以防传入的预期大小超出安全范围。
  3. copyFixedString 缺少末尾空字符保证

    • 位置: DdeSeatdControl.cpp -> copyFixedString()
    • 代码: 已经使用 memset(target, 0, N) 并限制了 min(bytes.size(), N - 1),这非常安全,消除了原代码中 copyFixedString 可能未以 \0 结尾的隐患。非常棒的改进。

总结

本次重构质量很高,将原本深度的内核 VT 接口耦合剥离到 dde-seatd 这一中间层,使得 DDM 的架构更加清晰,也规避了内核 VT 状态机死锁的痛点。SignalHandler 的精简也提升了系统的稳定性。

主要修改建议优先级:

  1. 高优:排查同步阻塞调用卡死主事件循环的风险(考虑降超时或改异步)。
  2. 中优:修复 TreelandConnector 中可能死循环的 while 分发逻辑。
  3. 中优:完善 handleVtChanged 中对空用户的处理逻辑。
  4. 低优:考虑控制 Socket 的连接复用以提升性能。

@zccrs
Copy link
Copy Markdown
Member Author

zccrs commented Jun 3, 2026

依赖:linuxdeepin/dde-seatd#3

@zccrs zccrs merged commit 8363354 into linuxdeepin:master Jun 3, 2026
10 checks passed
@zccrs zccrs deleted the vt-control-socket-refactor branch June 3, 2026 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants