Skip to content

fix(vt): avoid repeated VT handlers and restore session state#92

Open
zccrs wants to merge 1 commit into
linuxdeepin:masterfrom
zccrs:master
Open

fix(vt): avoid repeated VT handlers and restore session state#92
zccrs wants to merge 1 commit into
linuxdeepin:masterfrom
zccrs:master

Conversation

@zccrs
Copy link
Copy Markdown
Member

@zccrs zccrs commented May 21, 2026

Ensure VT signal handling is connected only once per process.

确保VT信号处理在进程内只连接一次。

Restore Treeland session active state before switching users.

在切换到Treeland管理用户前恢复会话激活状态。

Log: 修复VT切换重复处理与会话状态恢复
Influence: 降低切换到Treeland用户后被拉回或状态不一致的风险。

Summary by Sourcery

Ensure VT signal handling is only connected once per process and restore session activation before switching to the Treeland-managed user.

Bug Fixes:

  • Prevent duplicate VT custom signal handlers from being connected, avoiding repeated VT handling.
  • Restore the Treeland session to an active state before switching users to avoid being pulled back or ending in an inconsistent state.

Ensure VT signal handling is connected only once per process.

确保VT信号处理在进程内只连接一次。

Restore Treeland session active state before switching users.

在切换到Treeland管理用户前恢复会话激活状态。

Log: 修复VT切换重复处理与会话状态恢复
Influence: 降低切换到Treeland用户后被拉回或状态不一致的风险。
@zccrs zccrs requested a review from Copilot May 21, 2026 08:50
@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 requested a review from wineee May 21, 2026 08:50
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 21, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Ensures VT custom signal handling is connected only once per process and restores the Treeland session active state before switching to the Treeland-managed user, reducing repeated VT handling and inconsistent session state after user switching.

Sequence diagram for Treeland session activation before user switch

sequenceDiagram
    participant RenderDisabled as renderDisabled
    participant VirtualTerminal as VirtualTerminal
    participant TreelandConnector as TreelandConnector

    RenderDisabled->>VirtualTerminal: handleVtSwitches(activeVtFd)
    RenderDisabled->>RenderDisabled: close(activeVtFd)
    RenderDisabled->>TreelandConnector: activateSession()
    RenderDisabled->>TreelandConnector: enableRender()
    RenderDisabled->>TreelandConnector: switchToUser(user)
Loading

Sequence diagram for VT customSignalReceived unique connection

sequenceDiagram
    participant DaemonApp as DaemonApp
    participant SignalHandler as SignalHandler
    participant VirtualTerminal as VirtualTerminal

    DaemonApp->>SignalHandler: addCustomSignal(RELEASE_DISPLAY_SIGNAL)
    DaemonApp->>SignalHandler: addCustomSignal(ACQUIRE_DISPLAY_SIGNAL)
    DaemonApp->>SignalHandler: connect(customSignalReceived, onVtSignal, UniqueConnection)

    SignalHandler->>VirtualTerminal: customSignalReceived(RELEASE_DISPLAY_SIGNAL)
    VirtualTerminal->>VirtualTerminal: onVtSignal(RELEASE_DISPLAY_SIGNAL)

    SignalHandler-->>VirtualTerminal: customSignalReceived(ACQUIRE_DISPLAY_SIGNAL)
    VirtualTerminal->>VirtualTerminal: onVtSignal(ACQUIRE_DISPLAY_SIGNAL)
Loading

File-Level Changes

Change Details Files
Prevent duplicate VT custom signal handler connections by making the QObject connection unique.
  • Update the QObject::connect call for customSignalReceived to pass the signal handler as the receiver object.
  • Add Qt::UniqueConnection to the connection to ensure the slot is connected only once even if initialization runs multiple times.
src/daemon/VirtualTerminal.cpp
Restore and ensure Treeland session activation before switching to the Treeland-managed user.
  • Call activateSession() after handling VT switches and closing the active VT file descriptor.
  • Keep render enabling and user switching logic the same but now ensure it occurs after session activation.
src/daemon/TreelandConnector.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码修改涉及两个文件:TreelandConnector.cppVirtualTerminal.cpp。以下是对这两处修改的详细审查意见:

1. 文件:src/daemon/TreelandConnector.cpp

修改内容
renderDisabled 函数中,关闭 activeVtFd 后,调用 conn->enableRender() 之前,增加了一行 conn->activateSession()

审查意见

  • 语法逻辑

    • 逻辑上看起来是合理的。在处理完 VT(虚拟终端)切换(关闭旧的 fd)之后,在启用渲染之前,先激活会话。这通常意味着需要确保会话处于活动状态才能进行渲染或切换用户。
    • 需要确认 activateSession() 内部是否会抛出异常或导致程序崩溃,如果它失败,后续的 enableRenderswitchToUser 是否应该执行。如果 activateSession 是关键步骤,建议增加错误处理逻辑。
  • 代码质量

    • 建议:增加注释说明为什么需要在这里调用 activateSession()。例如:// 在切换 VT 后,必须先激活会话才能进行渲染和用户切换
    • 建议:检查 conn 指针是否为空(虽然看起来这是一个回调函数,上下文可能保证了 conn 的有效性,但防御性编程总是好的)。
  • 代码性能

    • 无明显性能问题。
  • 代码安全

    • 潜在风险activateSession() 可能涉及权限提升或系统级调用。请确保该调用不会引入竞态条件或权限泄露。
    • 检查点:确认 activateSession() 的实现是否线程安全,如果 renderDisabled 可能在多线程环境下被调用。

2. 文件:src/daemon/VirtualTerminal.cpp

修改内容
VirtualTerminal::init 函数中,修改了 QObject::connect 的调用方式,增加了接收者上下文对象 daemonApp->signalHandler() 和连接类型 Qt::UniqueConnection

审查意见

  • 语法逻辑

    • 修改分析
      • 原代码:connect(sender, &Signal, receiver, function_ptr)。这是 Qt 5 的标准连接方式,使用的是函数指针。
      • 新代码:connect(sender, &Signal, context, function_ptr, Qt::UniqueConnection)。这里指定了 context 对象,并使用了 Qt::UniqueConnection
    • 逻辑问题
      • 接收者对象:新代码将 daemonApp->signalHandler() 同时作为发送者和接收者上下文。这意味着当 signalHandler 对象被销毁时,连接会自动断开。这在对象生命周期管理上是合理的,防止了悬空信号。
      • Qt::UniqueConnection:这个标志确保了如果相同的信号和槽已经连接过,则不会重复连接。这对于 init 这种可能被多次调用的函数非常有用,可以防止同一个槽函数被多次触发。
  • 代码质量

    • 优点:使用 Qt::UniqueConnection 是一个很好的改进,防止了重复连接。
    • 潜在问题onVtSignal 是一个静态函数还是成员函数?
      • 如果是静态函数全局函数,这种写法是正确的。
      • 如果是成员函数,通常需要传递对象指针(如 this)作为接收者,而不是 daemonApp->signalHandler(),除非 onVtSignal 确实是 SignalHandler 的成员函数且意图在 SignalHandler 内部处理。
      • 注意:如果 onVtSignalVirtualTerminal 类的静态辅助函数,将 signalHandler 作为上下文对象是可以的,但逻辑上稍显奇怪(通常上下文对象应该是拥有槽函数逻辑的对象)。
  • 代码性能

    • 无明显性能问题。Qt::UniqueConnection 会增加一点连接时的检查开销,但可以忽略不计。
  • 代码安全

    • 内存安全:指定 context 对象(这里是 daemonApp->signalHandler())提高了安全性,确保当该对象销毁时连接自动断开,避免了访问已销毁对象的风险。
    • 重复调用Qt::UniqueConnection 防止了 init 函数被多次调用时导致的资源泄漏(重复连接)或逻辑错误(信号触发多次)。

综合改进建议

  1. 针对 TreelandConnector.cpp

    • 建议为新增的 activateSession() 调用添加注释,解释其必要性。
    • 检查 activateSession() 的错误处理机制。
  2. 针对 VirtualTerminal.cpp

    • 确认 onVtSignal 的性质。如果它是 VirtualTerminal 的私有静态函数,且 VirtualTerminal 的生命周期比 signalHandler 短,或者逻辑上属于 VirtualTerminal,建议重新考虑是否应该将 VirtualTerminal 对象(如果有的话)作为上下文,或者保持原样但确保 signalHandler 生命周期足够长。
    • 如果 onVtSignal 确实是处理 VT 信号的核心逻辑,且依赖于 signalHandler 的存在,那么当前的修改是安全的。
  3. 通用建议

    • 确保 daemonApp->signalHandler() 在整个连接期间有效,不会被提前销毁。
    • 考虑使用现代 C++ 的 lambda 表达式配合智能指针来管理回调生命周期(如果适用),这可能比原始函数指针更清晰。

总结

这两处修改在逻辑上是合理的,主要目的是增强会话管理的顺序性和信号连接的安全性。VirtualTerminal.cpp 中的修改尤其值得肯定,因为它增加了对重复连接的防护和对象生命周期的管理。只需确认 onVtSignalsignalHandler 的关系是否符合设计意图即可。

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.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

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 targets VT switching stability in ddm by preventing repeated VT signal handler connections and ensuring the Treeland session is re-activated before switching to a Treeland-managed user, reducing inconsistent session state after VT transitions.

Changes:

  • Use Qt::UniqueConnection (with an explicit context object) to avoid duplicate connections to SignalHandler::customSignalReceived in VT handling.
  • Restore Treeland session active state (activateSession()) before enableRender()/switchToUser() when switching to a VT running Treeland.

上述改动主要提升 ddm 在 VT 切换场景下的稳定性:避免 VT 信号处理重复连接,并在切换到 Treeland 管理的用户前恢复会话激活状态,从而降低 VT 切换后会话状态不一致的风险。

变更点:

  • 在 VT 处理逻辑中使用 Qt::UniqueConnection(并显式指定 context 对象)避免对 SignalHandler::customSignalReceived 的重复连接。
  • 当切换到正在运行 Treeland 的 VT 时,在 enableRender()/switchToUser() 前调用 activateSession() 恢复会话激活状态。

Reviewed changes

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

File Description
src/daemon/VirtualTerminal.cpp Prevent duplicate Qt signal connections for VT custom-signal handling via Qt::UniqueConnection. / 通过 Qt::UniqueConnection 避免 VT 自定义信号处理的重复 Qt 连接。
src/daemon/TreelandConnector.cpp Activate Treeland session before rendering/user switch during VT transition to Treeland. / 在切换到 Treeland 的 VT 流程中先激活会话再恢复渲染并切换用户。

Comment on lines 118 to +121
daemonApp->signalHandler()->addCustomSignal(RELEASE_DISPLAY_SIGNAL);
daemonApp->signalHandler()->addCustomSignal(ACQUIRE_DISPLAY_SIGNAL);
QObject::connect(daemonApp->signalHandler(), &SignalHandler::customSignalReceived, onVtSignal);
QObject::connect(daemonApp->signalHandler(), &SignalHandler::customSignalReceived,
daemonApp->signalHandler(), onVtSignal, Qt::UniqueConnection);
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