Skip to content

refactor(dde-session): remove dconf dependency and fix thread safety#210

Merged
mhduiy merged 2 commits into
linuxdeepin:masterfrom
mhduiy:warning
Jun 3, 2026
Merged

refactor(dde-session): remove dconf dependency and fix thread safety#210
mhduiy merged 2 commits into
linuxdeepin:masterfrom
mhduiy:warning

Conversation

@mhduiy

@mhduiy mhduiy commented Jun 3, 2026

Copy link
Copy Markdown
Contributor
  1. Remove dconf utility (dconf.cpp/h) and all related calls
  2. Remove initSwapSched() and quick-black-screen logic from sessionmanager
  3. Disable wayland dde_wldpms command with TODO comment
  4. Remove dead iowait watcher code from main.cpp
  5. Use QMetaObject::invokeMethod to dispatch session operations to main thread
  6. Fix fifo cleanup: delete directly instead of deleteLater in worker thread

Log: remove dconf dependency, clean up dead code, fix QObject thread affinity warning

refactor(dde-session): 移除 dconf 依赖并修复线程安全问题

  1. 删除 dconf 工具类(dconf.cpp/h)及所有相关调用
  2. 移除 sessionmanager 中的 initSwapSched() 和 quick-black-screen 逻辑
  3. 禁用 wayland 下的 dde_wldpms 命令并添加 TODO 注释
  4. 移除 main.cpp 中废弃的 iowait watcher 代码
  5. 使用 QMetaObject::invokeMethod 将 session 操作派发回主线程
  6. 修复 fifo 清理:工作线程中直接 delete 替代 deleteLater

Log: 移除 dconf 依赖,清理废弃代码,修复 QObject 线程亲和性警告

@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.

Sorry @mhduiy, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

1. Remove Dconf wrapper class (dconf.h/cpp) and its CMake references
2. Remove initSwapSched() placeholder and swap-sched-enabled check
3. Remove Dconf-based quick-black-screen logic in suspend/shutdown/reboot
4. Remove dead iowait watcher code block (was already #if 0)
5. Comment out Wayland DPMS command in setDPMSMode()

Log: Remove Dconf utility class and related dead/unused code paths

refactor(dde-session): 移除 Dconf 工具类及无用代码

1. 移除 Dconf 封装类(dconf.h/cpp)及其 CMake 引用
2. 移除 initSwapSched() 占位函数及 swap-sched-enabled 配置检查
3. 移除挂起/关机/重启中基于 Dconf 的 quick-black-screen 逻辑
4. 移除已禁用的 iowait watcher 代码块(原为 #if 0)
5. 注释掉 setDPMSMode() 中的 Wayland DPMS 命令

Log: 移除 Dconf 工具类及其相关的无用/未使用代码路径
1. Move session->setSessionPid/setSessionPath to main thread via
   QMetaObject::invokeMethod with Qt::QueuedConnection
2. Replace fifo->deleteLater() with direct delete for immediate cleanup
3. Use QueuedConnection for qApp->quit() to ensure thread-safe exit

Log: Fix thread safety issues in FIFO pipe read callback by deferring
session operations and app quit to the main thread event loop

fix(dde-session): 修复 FIFO 管道处理中的线程安全问题

1. 通过 QMetaObject::invokeMethod(Qt::QueuedConnection) 将
   setSessionPid/setSessionPath 调度到主线程执行
2. 将 fifo->deleteLater() 改为直接 delete 以立即释放资源
3. 使用 QueuedConnection 调度 qApp->quit() 确保线程安全退出

Log: 修复 FIFO 管道读取回调中的线程安全问题,将 session 操作和应用退出
调度到主线程事件循环执行
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已经仔细审查了你提供的Git Diff。这次代码变更主要涉及:移除了自定义的Dconf封装类,清理了未使用/废弃的功能代码(如swap-schedquick-black-screeniowait),并修复了多线程并发访问Qt对象的潜在崩溃问题。

总体来说,这次重构和清理工作非常有价值,特别是线程安全方面的修复。但是,代码中仍存在一些逻辑漏洞、性能隐患以及代码规范上的问题。以下是详细的审查意见:

1. 语法与逻辑

  • Wayland DPMS 逻辑被注释导致功能缺失
    • 位置: src/dde-session/impl/sessionmanager.cpp 第821-822行
    • 问题: if (Utils::IS_WAYLAND_DISPLAY) 分支下的代码被整体注释掉,只留下了 // TODO。在 Wayland 环境下,这会导致显示器电源管理(DPMS)完全失效,系统将无法在 Wayland 下关闭或开启屏幕。
    • 建议: 如果 dde_wldpms 工具目前不可用或存在问题,应该保留原代码并增加条件编译(如 #ifdef ENABLE_WLDPMS),或者提供一个备用的 Wayland DPMS 处理逻辑,而不是直接注释掉导致功能降级。
  • fifo 对象的内存泄漏与生命周期管理
    • 位置: src/dde-session/main.cpp 第159行
    • 问题: 将 fifo->deleteLater() 改为了 delete fifo。由于 fifo 是在一个独立线程(由 QThreadPool 启动)中创建和使用的,直接 delete 本身没有线程安全问题(因为在该线程中没有事件循环跑 deleteLater)。但是,如果 fifo 内部有需要事件循环来安全关闭的异步操作(如网络、进程IO),直接 delete 可能会导致资源未正确释放。
    • 建议: 确认 Fifo 类的析构函数是否是线程安全且完全同步的。如果是,delete 是可以的;如果它继承自 QObject 且有父子关系或信号槽连接,建议使用 QScopedPointer 或确保在合适的上下文中销毁。

2. 代码性能

  • DConfig 实例的重复创建与销毁(历史遗留问题)
    • 位置: 被删除的 dconf.cpp
    • 问题: 虽然你删除了 Dconf 类,但回顾其实现发现了一个严重的性能问题:每次调用 GetValueSetValue 都会通过 connectDconf 创建一个新的 DConfig 对象(DConfig::create),并用 QSharedPointer 包裹使其在作用域结束时销毁。读取配置是非常高频的操作,这种做法会导致极大的不必要的IPC开销和对象构造/析构开销。
    • 建议: 既然已经移除了这个 Dconf 类,在后续使用 DConfig 时,务必避免这种模式。应该在业务类初始化时创建一次 DConfig 实例并作为成员变量缓存,后续直接读取缓存的实例。

3. 代码安全

  • 线程安全修复非常棒,但需注意对象生命周期
    • 位置: src/dde-session/main.cpp 第148-152行
    • 问题: 你将原来的 session->setSessionPid(pid) 改为了 QMetaObject::invokeMethod(session, ..., Qt::QueuedConnection),这修复了在工作线程中直接操作主线程QObject的严重线程安全问题,非常赞!但是,这里存在一个隐患:如果线程池执行到此处时,qApp 已经因为某些异常退出,session 可能已经是一个悬空指针。
    • 建议: 在调用 invokeMethod 前,最好增加有效性检查,或者确保 session 的销毁晚于所有子线程。可以通过 QPointer<SessionManager> sessionPtr(session); 并在 lambda 内检查 if (!sessionPtr) return; 来防止潜在的崩溃。
  • qApp->quit() 的线程安全修复
    • 位置: src/dde-session/main.cpp 第161行
    • 问题: 同样,将 qApp->quit() 改为 QMetaObject::invokeMethod(qApp, &QCoreApplication::quit, Qt::QueuedConnection) 是非常正确的做法,因为不能在非主线程直接调用 quit()
    • 建议: 逻辑无误,保持即可。

4. 代码质量与规范

  • 引入了命名空间但未使用相关功能
    • 位置: src/dde-session/impl/sessionmanager.cpp 第26行 using namespace Dtk::Core;
    • 问题: 你移除了所有 Dconf::GetValue 的调用,目前 sessionmanager.cpp 中似乎没有直接使用 Dtk::Core 命名空间下的类。如果引入 DConfig 头文件只是为了将来使用,那么现在留下这个 using namespace 会污染当前文件的命名空间。
    • 建议: 如果当前文件没有使用到 Dtk::Core 中的符号,请移除 using namespace Dtk::Core;。即使将来要用,也建议在局部函数内使用 Dtk::Core::DConfig 而非在全局 using
  • 版权年份格式错误
    • 位置: src/dde-session/main.cpp 第1行
    • 问题: -// SPDX-FileCopyrightText: 2021 - 2023 UnionTech Software Technology Co., Ltd. 改为了 +// SPDX-FileCopyrightText: 2021 - 2026 UnionTech Software Technology Co., Ltd.
    • 建议: 版权结束年份通常应该是代码最后修改的年份(如2024),而不是未来的年份(2026)。除非这是你们项目的特殊规范,否则建议修改为当前年份。
  • 多余的空行
    • 位置: src/dde-session/main.cpp 第143行
    • 问题: 删除 #endifqInfo() << "iowait watcher disabled"; 后,留下了连续的空行,影响了代码紧凑性。
    • 建议: 删除多余的空行。

总结与修改建议代码片段

针对 main.cpp 中的线程安全与生命周期问题,建议修改如下:

// main.cpp 中的线程池任务部分
QThreadPool::globalInstance()->start([&session]() {
    qInfo()<< "systemd service pipe thread id: " << QThread::currentThreadId();
    // ... 前置逻辑 ...

    if (ok && pid > 0) {
        // 使用 QPointer 防止 session 在子线程执行期间被主线程销毁导致悬空指针
        QPointer<SessionManager> sessionGuard(session);
        QMetaObject::invokeMethod(session, [sessionGuard, pid]() {
            if (!sessionGuard) return; // 确保对象仍然存活
            sessionGuard->setSessionPid(pid);
            sessionGuard->setSessionPath();
        }, Qt::QueuedConnection);
    }
    
    // 如果 Fifo 不是 QObject 或者没有复杂的异步关闭逻辑,delete 是可以的
    delete fifo; 
    qInfo() << "pipe read finish, app exit.";
    
    // 安全地在主线程退出应用
    QMetaObject::invokeMethod(qApp, &QCoreApplication::quit, Qt::QueuedConnection);
});

针对 sessionmanager.cpp 中的 Wayland DPMS 问题:

void SessionManager::setDPMSMode(bool on)
{
    if (Utils::IS_WAYLAND_DISPLAY) {
        // 建议:不要直接注释掉,如果功能不可用,应该加宏控制或提供空实现,避免编译出无用的分支
        qWarning() << "Wayland DPMS: dde_wldpms is currently disabled or unavailable.";
        // EXEC_COMMAND("dde_wldpms", QStringList() << "-s" << (on ? "On" : "Off"));
    } else {
        // TODO 可通过Xlib发送对应请求实现此功能
        EXEC_COMMAND("xset", QStringList() << "dpms" << "force" << (on ? "on" : "off"));
    }
}

@deepin-ci-robot

Copy link
Copy Markdown

[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 a7e70d0 into linuxdeepin:master Jun 3, 2026
16 of 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