Skip to content

fix: update shutdown logic and copyright year#193

Merged
mhduiy merged 1 commit intolinuxdeepin:masterfrom
mhduiy:force
Mar 5, 2026
Merged

fix: update shutdown logic and copyright year#193
mhduiy merged 1 commit intolinuxdeepin:masterfrom
mhduiy:force

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Mar 4, 2026

Updated copyright year from 2023 to 2026. Modified the prepareShutdown method to always call preparePlayShutdownSound regardless of force parameter. Added conditional execution for force shutdown using dde- session-ctl command with --shutdown flag. This ensures shutdown sound plays in all cases and provides a proper forced shutdown path when needed.

fix: 更新关机逻辑和版权年份

将版权年份从2023更新到2026。修改了prepareShutdown方法,使其无论force参数 如何都会调用preparePlayShutdownSound。添加了条件执行强制关机的逻辑,使用
dde-session-ctl命令和--shutdown标志。这确保了在所有情况下都会播放关机音
效,并在需要时提供正确的强制关机路径。

PMS: BUG-281747

Summary by Sourcery

Update session shutdown behavior and refresh copyright metadata.

Bug Fixes:

  • Ensure shutdown sound plays for both normal and forced shutdown flows.
  • Add a proper forced shutdown path that directly invokes dde-session-ctl with the --shutdown flag when requested.

Enhancements:

  • Simplify shutdown preparation by always triggering the shutdown sound before stopping audio services.

Chores:

  • Update copyright year notice from 2023 to 2026 in the session manager source file.

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 4, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Updates session shutdown behavior so the shutdown sound is always played and adds a force-shutdown path via dde-session-ctl, along with refreshing the copyright year.

Sequence diagram for updated SessionManager shutdown flow

sequenceDiagram
    participant SessionManager
    participant SogouIme
    participant BAMFDaemon
    participant Audio as PulseAudioService
    participant Sound as ShutdownSound
    participant DdeSessionCtl

    rect rgb(235, 245, 255)
        note over SessionManager: prepareShutdown(force == false)
        SessionManager->>SogouIme: stopSogouIme()
        SessionManager->>BAMFDaemon: stopBAMFDaemon()
        SessionManager->>Sound: preparePlayShutdownSound()
        SessionManager->>Audio: stopPulseAudioService()
    end

    rect rgb(245, 235, 255)
        note over SessionManager: prepareShutdown(force == true)
        SessionManager->>SogouIme: stopSogouIme()
        SessionManager->>BAMFDaemon: stopBAMFDaemon()
        SessionManager->>Sound: preparePlayShutdownSound()
        SessionManager->>Audio: stopPulseAudioService()
        SessionManager->>DdeSessionCtl: EXEC_COMMAND(/usr/libexec/dde-session-ctl, [--shutdown])
    end
Loading

Class diagram for updated SessionManager prepareShutdown

classDiagram
    class SessionManager {
        +void prepareShutdown(bool force)
        +void stopSogouIme()
        +void stopBAMFDaemon()
        +void preparePlayShutdownSound()
        +void stopPulseAudioService()
    }

    class DdeSessionCtl {
        +void EXEC_COMMAND(QString command, QStringList args)
    }

    SessionManager ..> DdeSessionCtl : uses_forced_shutdown
Loading

File-Level Changes

Change Details Files
Always play the shutdown sound during prepareShutdown and add a forced shutdown path when requested.
  • Removed the conditional around shutdown sound so it is always triggered in prepareShutdown, regardless of the force flag.
  • Kept the shutdown sequence order but added a conditional branch: when force is true, execute dde-session-ctl with --shutdown to immediately stop the session without waiting for apps.
  • Ensured the sound is requested before stopping the audio service to avoid losing playback on shutdown.
src/dde-session/impl/sessionmanager.cpp
Update copyright year in the session manager source file header.
  • Bumped the SPDX-FileCopyrightText year range from 2021-2023 to 2021-2026.
src/dde-session/impl/sessionmanager.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

Copy link

@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 found 1 issue, and left some high level feedback:

  • The forced shutdown branch executes dde-session-ctl --shutdown but does not check or log its return status; consider adding error handling or logging to make failures visible and easier to diagnose.
  • The hardcoded "/usr/libexec/dde-session-ctl" path may be brittle; if there is an existing helper, macro, or configuration for this binary elsewhere in the codebase, consider reusing it to avoid divergence if the path changes in future.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The forced shutdown branch executes `dde-session-ctl --shutdown` but does not check or log its return status; consider adding error handling or logging to make failures visible and easier to diagnose.
- The hardcoded `"/usr/libexec/dde-session-ctl"` path may be brittle; if there is an existing helper, macro, or configuration for this binary elsewhere in the codebase, consider reusing it to avoid divergence if the path changes in future.

## Individual Comments

### Comment 1
<location path="src/dde-session/impl/sessionmanager.cpp" line_range="525-527" />
<code_context>
+
+    if (force) {
+        // 直接停止session,默认不会等待应用程序准备完成,强制关机
+        EXEC_COMMAND("/usr/libexec/dde-session-ctl"
+                 , QStringList()
+                 << "--shutdown");
+    }
 }
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider the interaction of this direct shutdown command with the rest of the shutdown pipeline to avoid double or re-entrant shutdown.

Since `prepareShutdown(true)` now directly calls `dde-session-ctl --shutdown`, this function effectively becomes the shutdown trigger. Please verify whether `prepareShutdown` can be invoked multiple times or from different code paths, and if so, add guarding (e.g., an internal state flag) so the shutdown command is only executed once.
</issue_to_address>

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.

@mhduiy mhduiy force-pushed the force branch 2 times, most recently from a26238d to 289a4e4 Compare March 4, 2026 09:58
yixinshark
yixinshark previously approved these changes Mar 4, 2026
Updated copyright year from 2023 to 2026. Modified the prepareShutdown
method to always call preparePlayShutdownSound, removing the previous
conditional check based on the 'force' parameter. Added a new
conditional block for forced shutdowns that executes the dde-session-ctl
command with the --shutdown flag. This ensures the shutdown sound plays
consistently for all shutdown types (normal and forced) and provides
a proper, immediate shutdown path when force is required, bypassing
application preparation delays.

Log: Shutdown sound now plays during forced shutdown; forced shutdown
uses a direct command for immediate termination.

fix: 改进关机逻辑并更新版权年份

将版权年份从2023更新到2026。修改了prepareShutdown方法,使其总是调用
preparePlayShutdownSound,移除了之前基于'force'参数的条件判断。新增了一
个用于强制关机的条件代码块,该块会执行带有--shutdown标志的dde-session-
ctl命令。这确保了关机音效在所有关机类型(正常和强制)下都能一致播放,并
在需要强制关机时提供了一条直接、立即的关机路径,绕过了应用程序的准备工作
延迟。

Log: 强制关机时现在会播放关机音效;强制关机使用直接命令实现立即终止。

PMS: BUG-281747
@deepin-ci-robot
Copy link

deepin pr auto review

这份代码变更主要涉及 DDE (Deepin Desktop Environment) 会话管理器的功能调整,核心逻辑是将"强制"操作(ForceLogout/Reboot/Shutdown)移除,并将常规的"请求"操作(RequestLogout/Reboot/Shutdown)默认行为修改为"强制"模式。

以下是对该代码变更的详细审查意见:

1. 语法与逻辑审查

  • 逻辑一致性

    • 变更点:代码移除了 ForceLogoutForceRebootForceShutdown 三个方法,同时将 RequestRebootRequestShutdown 内部调用的参数从 false 改为 true
    • 潜在问题:这种修改改变了 D-Bus 接口的语义。通常 Request 暗示可能被拒绝或需要确认(例如检查 Inhibitors),而 Force 暗示绕过检查直接执行。现在的 RequestRebootRequestShutdown 实际上变成了强制执行。如果上游调用者(如 UI 层)依赖之前的语义(即调用 Request 时期望能被 Inhibitor 阻止),现在将导致逻辑失效,直接关机/重启,可能造成用户数据丢失。
    • 建议:如果这是预期的行为变更(即简化流程,所有操作都强制执行),则需要确保所有调用方(如dde-shutdown、前端UI)都知晓这一变化,并移除它们侧的"强制"与"非强制"逻辑分支,否则会造成 UI 按钮功能混乱。
  • 版权年份更新

    • sessionmanager.cppsessionmanager.h 中的版权年份从 2021 - 2023 更新到了 2021 - 2026
    • 建议:通常版权年份应更新到当前年份或未来不久的年份(如 2024/2025)。直接写到 2026 年略显超前,虽然不是语法错误,但不符合常规开源项目的维护习惯。

2. 代码质量审查

  • 代码删除与重构

    • 删除了 ForceLogout 等函数,减少了代码冗余,这是好的做法。
    • 未删除的 ForceLogout 逻辑ForceLogout 被删除了,但 RequestLogout 的实现并未像 RequestReboot 那样修改为强制模式(prepareLogout(true))。这意味着注销行为可能与其他两个行为不一致。
    • 建议:检查 RequestLogout 是否也需要修改为强制模式,以保持行为的一致性。如果注销不需要强制,应明确注释说明原因。
  • prepareShutdown 函数逻辑变动

    • 移除了 if (!force) 的判断,导致 preparePlayShutdownSound() 无论是否强制都会被调用。
    • 新增了 EXEC_COMMAND 调用 dde-amdde-session-ctl
    • 硬编码路径:使用了 /usr/bin/dde-am/usr/libexec/dde-session-ctl 等硬编码绝对路径。
    • 建议:确保这些路径在所有支持的发行版或文件系统布局下都是有效的。如果可能,使用构建系统或配置文件来管理这些路径。

3. 代码性能审查

  • 外部命令执行
    • prepareShutdown 中新增了 EXEC_COMMAND。这是一个外部进程调用,涉及到 fork 和 exec,在关机路径上通常是可接受的,但需要注意如果该命令卡死,可能会延迟关机过程。
    • 建议:确保 dde-session-ctl --shutdown 是非阻塞的或者有超时机制,防止关机流程挂起。

4. 代码安全审查

  • 权限提升风险
    • RequestRebootRequestShutdown 默认改为强制模式(force=true),意味着任何有权限调用该 D-Bus 方法的进程都可以直接关机/重启系统,而不再受到 Inhibitor(如未保存的文档提示)的限制。
    • 安全建议
      1. 权限审查:必须严格检查 D-Bus 策略文件(如 org.deepin.dde.SessionManager1.conf),确保只有受信任的用户(如当前登录的桌面用户)或 root 用户才能调用这些方法。防止恶意软件通过 D-Bus 直接调用导致系统强制关机。
      2. Inhibitor 机制失效:由于强制模式绕过了 Inhibitor,应用程序(如 LibreOffice、Gedit 等)注册的"阻止关机"请求将失效。这可能导致用户数据未保存就丢失。这在用户体验上是巨大的倒退,除非系统层面有其他机制保证数据安全。

5. 改进建议总结

  1. 统一行为:确认 RequestLogout 是否也应该像 RequestReboot 一样改为强制模式。如果不改,需在代码注释中解释原因。
  2. 版权年份:建议将版权年份修改为 2021 - 2024 或当前实际年份。
  3. D-Bus 策略检查:务必审查并收紧 D-Bus 访问控制策略(polkit 或 dbus-daemon 配置),防止未授权调用导致强制关机。
  4. 数据安全提示:由于绕过了 Inhibitor,建议在 UI 层(dde-shutdown)调用 RequestShutdown 之前,手动实现一遍检查未保存文件的逻辑,或者在文档中明确告知用户该行为变更带来的风险。
  5. 硬编码路径:考虑将 /usr/bin/dde-am 等路径定义为宏或常量,便于维护和跨平台适配。
  6. 代码注释prepareShutdown 中新增的 EXEC_COMMAND 部分缺少对 dde-amdde-session-ctl 功能的详细注释,建议补充说明这两个工具的作用。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mhduiy, yixinshark

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

@mhduiy mhduiy merged commit 187438e into linuxdeepin:master Mar 5, 2026
18 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