fix: update shutdown logic and copyright year#193
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates 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 flowsequenceDiagram
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
Class diagram for updated SessionManager prepareShutdownclassDiagram
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
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The forced shutdown branch executes
dde-session-ctl --shutdownbut 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>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
a26238d to
289a4e4
Compare
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 pr auto review这份代码变更主要涉及 DDE (Deepin Desktop Environment) 会话管理器的功能调整,核心逻辑是将"强制"操作(ForceLogout/Reboot/Shutdown)移除,并将常规的"请求"操作(RequestLogout/Reboot/Shutdown)默认行为修改为"强制"模式。 以下是对该代码变更的详细审查意见: 1. 语法与逻辑审查
2. 代码质量审查
3. 代码性能审查
4. 代码安全审查
5. 改进建议总结
|
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
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:
Enhancements:
Chores: