Skip to content

fix(session): avoid tty control in Treeland#101

Merged
zccrs merged 1 commit into
linuxdeepin:masterfrom
mhduiy:session/crash
Jun 11, 2026
Merged

fix(session): avoid tty control in Treeland#101
zccrs merged 1 commit into
linuxdeepin:masterfrom
mhduiy:session/crash

Conversation

@mhduiy

@mhduiy mhduiy commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

• 问题现象:

Treeland single-mode 下,快速在 greeter 输入密码进入 session 时,dde-session 会在启动后一两秒退出,ddm 日志显示 session crash。慢一点输入密码时不容易复现。

原因:

原逻辑把所有非 X11 session 都当成需要接管 VT 的会话处理,会让 dde-session 打开 /dev/tty2 并通过 TIOCSCTTY 成为该 VT 的控制终端 leader。

但 Treeland single-mode 下,真正持有显示服务端能力的是已经运行的 Treeland compositor,dde-session 只是用户会话进程,不应该接管 VT。快速登录时,dde-session 过早成为 /dev/tty2 的控制终端 leader,后续 Treeland / dde-seatd 完成 VT 切换时触发 tty hangup,内
核向 dde-session 发送 SIGHUP,ddm 因此看到 session crash。

解法:

在 UserSession::childModifier() 中区分 Treeland single-mode 和普通 Wayland:

  • 普通 Wayland session 仍保持原逻辑,继续接管 VT。
  • Treeland single-mode session 不再打开 VT,也不再成为控制终端 leader,stdin 改为 /dev/null。

修复后,dde-session 仍通过 XDG_VTNR=2 归属用户 VT,但进程本身不再占有 /dev/tty2,因此不会再被 tty hangup 的 SIGHUP 杀掉。

  1. Skip opening the VT for Treeland single-mode user sessions, because the running Treeland compositor already owns the display server side.
  2. Keep the VT-as-stdin and TIOCSCTTY path only for non-X11 sessions that start their own display server, such as standalone Wayland sessions.
  3. Fix a fast-login race where dde-session became the controlling tty leader of /dev/tty2 while Treeland and dde-seatd were still completing VT transition.
  4. The visible symptom was that dde-session appeared to crash shortly after login, while the real reason was a kernel SIGHUP caused by tty hangup.

Log: Prevent Treeland single-mode dde-session from taking the VT as controlling tty.

fix(session): 避免 Treeland 接管 tty

  1. Treeland single-mode 用户会话不再打开 VT,因为显示服务端已经由正在运行的 Treeland compositor 持有。
  2. 仅保留自带显示服务端的非 X11 会话继续走 VT stdin 和 TIOCSCTTY 路径,例如独立 Wayland 会话。
  3. 修复快速登录时 dde-session 成为 /dev/tty2 控制终端 leader,而 Treeland 和 dde-seatd 仍在完成 VT 切换导致的时序问题。
  4. 表现现象是登录后一两秒 dde-session 看起来像 crash,实际原因是 tty hangup 触发了内核发送的 SIGHUP。

Log: 避免 Treeland single-mode 下 dde-session 将 VT 作为控制终端。

Summary by Sourcery

Adjust session VT handling to avoid Treeland single-mode sessions becoming the controlling TTY leader when a compositor is already active.

Bug Fixes:

  • Prevent Treeland single-mode user sessions from opening and taking control of the VT owned by the running compositor, avoiding kernel SIGHUP-induced session termination in fast-login scenarios.

Enhancements:

  • Restrict VT-as-stdin and TIOCSCTTY behavior to non-X11 sessions that start their own display server, such as standalone Wayland sessions.

@sourcery-ai

sourcery-ai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts UserSession childModifier TTY handling so Treeland single-mode sessions no longer open or take control of a VT, while preserving VT-leak behavior for other non-X11 sessions to avoid races and SIGHUP-induced apparent crashes.

File-Level Changes

Change Details Files
Change VT handling so Treeland single-mode sessions do not open or take the VT as controlling TTY while keeping VT-as-stdin behavior for other non-X11 sessions.
  • Refine childModifier comment to explain Treeland single-mode must not take the VT as its controlling terminal.
  • Introduce local ttyString and vtFd variables and initialize vtFd to -1.
  • Guard VT opening logic with an additional check to skip ::open when the display type is Treeland, preventing Treeland sessions from inheriting the VT as stdin and becoming the controlling TTY leader.
src/daemon/UserSession.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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In the Treeland branch auth->type != Display::Treeland leaves vtFd = -1 and ttyString empty; double‑check subsequent logic in childModifier() doesn’t assume a valid fd or nonempty path (it may be clearer to early‑return for Treeland or to guard all VT operations on a vtFd >= 0 check).
  • The nested if (auth->type != Display::Treeland) inside if (auth->type != Display::X11) reads a bit inverted; consider explicitly enumerating the display types that should open a VT (e.g., a switch on auth->type) to make the control flow and future extension to new display backends safer.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the Treeland branch `auth->type != Display::Treeland` leaves `vtFd = -1` and `ttyString` empty; double‑check subsequent logic in `childModifier()` doesn’t assume a valid fd or nonempty path (it may be clearer to early‑return for Treeland or to guard all VT operations on a `vtFd >= 0` check).
- The nested `if (auth->type != Display::Treeland)` inside `if (auth->type != Display::X11)` reads a bit inverted; consider explicitly enumerating the display types that should open a VT (e.g., a `switch` on `auth->type`) to make the control flow and future extension to new display backends safer.

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.

1. Skip opening the VT for Treeland single-mode user sessions, because the running Treeland compositor already owns the display server side.
2. Keep the VT-as-stdin and TIOCSCTTY path only for non-X11 sessions that start their own display server, such as standalone Wayland sessions.
3. Fix a fast-login race where dde-session became the controlling tty leader of /dev/tty2 while Treeland and dde-seatd were still completing VT transition.
4. The visible symptom was that dde-session appeared to crash shortly after login, while the real reason was a kernel SIGHUP caused by tty hangup.

Log: Prevent Treeland single-mode dde-session from taking the VT as controlling tty.

fix(session): 避免 Treeland 接管 tty

1. Treeland single-mode 用户会话不再打开 VT,因为显示服务端已经由正在运行的 Treeland compositor 持有。
2. 仅保留自带显示服务端的非 X11 会话继续走 VT stdin 和 TIOCSCTTY 路径,例如独立 Wayland 会话。
3. 修复快速登录时 dde-session 成为 /dev/tty2 控制终端 leader,而 Treeland 和 dde-seatd 仍在完成 VT 切换导致的时序问题。
4. 表现现象是登录后一两秒 dde-session 看起来像 crash,实际原因是 tty hangup 触发了内核发送的 SIGHUP。

Log: 避免 Treeland single-mode 下 dde-session 将 VT 作为控制终端。
PMS: TASK-390987
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

这段代码的修改主要是增加了对Treeland会话类型的特殊处理。让我对这段代码进行审查:

  1. 语法逻辑:
  • 代码逻辑清晰,条件判断结构合理
  • 使用了明确的枚举类型比较(auth->type != Display::Treeland)
  • 变量初始化规范(ttyString和vtFd都进行了初始化)
  1. 代码质量:
  • 注释清晰解释了代码目的和特殊处理的原因
  • 代码结构更加模块化,将Treeland的特殊情况单独处理
  • 变量命名规范(ttyString, vtFd等)
  1. 代码性能:
  • 性能影响可以忽略不计,只是在条件分支中增加了一个判断
  • 提前初始化变量避免了可能的重复分配
  1. 代码安全:
  • 增加了条件判断,防止对Treeland会话进行不必要的VT操作
  • 使用O_NOCTTY标志是正确的,防止子进程成为控制终端
  • vtFd初始化为-1是良好的实践,可以后续检查有效性

改进建议:

  1. 可以考虑在if语句块中添加对vtFd有效性的检查,例如:
if (auth->type != Display::Treeland) {
    ttyString = TtyUtils::path(auth->tty);
    vtFd = ::open(qPrintable(ttyString), O_RDWR | O_NOCTTY);
    if (vtFd == -1) {
        qCWarning(log) << "Failed to open VT" << ttyString;
        // 可以考虑添加错误处理逻辑
    }
}
  1. 可以考虑将ttyString和vtFd的定义放在更接近使用的位置,提高代码的局部性:
if (auth->type != Display::Treeland) {
    QString ttyString = TtyUtils::path(auth->tty);
    int vtFd = ::open(qPrintable(ttyString), O_RDWR | O_NOCTTY);
    // ... 使用ttyString和vtFd ...
}
  1. 建议添加日志记录,特别是对于这种可能会影响系统终端状态的操作:
if (auth->type != Display::Treeland) {
    qCDebug(log) << "Opening VT for non-Treeland session:" << auth->tty;
    QString ttyString = TtyUtils::path(auth->tty);
    int vtFd = ::open(qPrintable(ttyString), O_RDWR | O_NOCTTY);
    if (vtFd == -1) {
        qCWarning(log) << "Failed to open VT" << ttyString;
    }
}

总体来说,这段代码的修改是合理的,增加了对特殊会话类型的支持,提高了代码的健壮性。上述建议可以进一步提高代码的可靠性和可维护性。

@mhduiy mhduiy requested a review from zccrs June 11, 2026 01:30
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mhduiy, 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 merged commit 888815e into linuxdeepin:master Jun 11, 2026
10 checks passed
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