Skip to content

fix: resolve lastore-daemon service registration status check issue#298

Merged
xionglinlin merged 1 commit into
linuxdeepin:masterfrom
xionglinlin:master
May 19, 2026
Merged

fix: resolve lastore-daemon service registration status check issue#298
xionglinlin merged 1 commit into
linuxdeepin:masterfrom
xionglinlin:master

Conversation

@xionglinlin
Copy link
Copy Markdown
Contributor

The update service registration was failing to properly check system status after the lastore-daemon became valid. Previously, the wait service timer was not being stopped when the check system was called from the timer callback, causing potential race conditions. Additionally, the doCheckSystem method contained the D-Bus service validity check, which was moved to a new checkSystem method that properly handles the waiting logic before calling doCheckSystem.

Log: Fixed system check after lastore-daemon service registration

Influence:

  1. Test system check functionality when lastore-daemon service is already valid
  2. Test system check when lastore-daemon service is not yet available and becomes valid later
  3. Verify that wait service timer is properly stopped when service becomes valid
  4. Verify that system check is not triggered twice due to timer race conditions
  5. Test upgrade workflow end-to-end to ensure proper system check execution

fix: 修复lastore-daemon服务注册后获取状态问题

更新服务注册时,在lastore-daemon变为有效后未能正确检查系统状态。之前从
定时器回调调用checkSystem时没有停止等待服务定时器,可能导致竞态条件。
此外,doCheckSystem方法包含了D-Bus服务有效性检查,现在将其移动到新的
checkSystem方法中,该方法在调用doCheckSystem之前正确处理等待逻辑。

Log: 修复lastore-daemon服务注册后的系统检查

Influence:

  1. 测试lastore-daemon服务已生效时的系统检查功能
  2. 测试lastore-daemon服务尚未就绪后变为可用时的系统检查
  3. 验证服务可用时等待服务定时器是否正常停止
  4. 验证不会因定时器竞态条件导致系统检查被触发两次
  5. 测试完整升级流程,确保系统检查正常执行

PMS: BUG-361789
Change-Id: I5de82d873a864673eb06482f09109c32f10cdc8f

The update service registration was failing to properly check
system status after the lastore-daemon became valid. Previously, the
wait service timer was not being stopped when the check system was
called from the timer callback, causing potential race conditions.
Additionally, the doCheckSystem method contained the D-Bus service
validity check, which was moved to a new checkSystem method that
properly handles the waiting logic before calling doCheckSystem.

Log: Fixed system check after lastore-daemon service registration

Influence:
1. Test system check functionality when lastore-daemon service is
already valid
2. Test system check when lastore-daemon service is not yet available
and becomes valid later
3. Verify that wait service timer is properly stopped when service
becomes valid
4. Verify that system check is not triggered twice due to timer race
conditions
5. Test upgrade workflow end-to-end to ensure proper system check
execution

fix: 修复lastore-daemon服务注册后获取状态问题

更新服务注册时,在lastore-daemon变为有效后未能正确检查系统状态。之前从
定时器回调调用checkSystem时没有停止等待服务定时器,可能导致竞态条件。
此外,doCheckSystem方法包含了D-Bus服务有效性检查,现在将其移动到新的
checkSystem方法中,该方法在调用doCheckSystem之前正确处理等待逻辑。

Log: 修复lastore-daemon服务注册后的系统检查

Influence:
1. 测试lastore-daemon服务已生效时的系统检查功能
2. 测试lastore-daemon服务尚未就绪后变为可用时的系统检查
3. 验证服务可用时等待服务定时器是否正常停止
4. 验证不会因定时器竞态条件导致系统检查被触发两次
5. 测试完整升级流程,确保系统检查正常执行

PMS: BUG-361789
Change-Id: I5de82d873a864673eb06482f09109c32f10cdc8f
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX,很高兴为你审查这段代码。

本次代码变更主要进行了以下几项重构和修复:

  1. 将DBus接口有效性判断从 serviceValid() 改为 isValid()
  2. doCheckSystem 的外部调用抽象为新的 checkSystem 方法,将“接口状态检查与等待逻辑”和“实际的检查系统逻辑”分离;
  3. 在定时器触发的回调中增加了对 m_waitServiceTimer 的主动停止逻辑;
  4. 更新了版权声明年份。

整体来看,这次重构提升了代码的内聚性和逻辑清晰度,修复了潜在的计时器泄漏问题。下面我将从语法逻辑、代码质量、代码性能和代码安全四个维度为你提供详细的审查意见和改进建议:

一、 语法与逻辑

  1. 定时器停止逻辑的完善(潜在Bug修复)

    • 当前代码:在 init() 方法的 m_waitingToCheckSystem 分支中,增加了 if (m_waitServiceTimer->isActive()) { m_waitServiceTimer->stop(); }
    • 分析:这是一个很好的改进,防止了在定时器还在倒计时时就重复触发 doCheckSystem。但是,在新增的 checkSystem 方法中,当发现接口无效时,直接调用了 m_waitServiceTimer->start()
    • 建议:如果 checkSystem 可能被多次调用(例如上层由于某种原因连续调用),在 start() 之前也应该判断并停止已有的定时器,或者直接使用 start() 覆盖(Qt的 QTimer::start() 会重置定时器间隔,这在某些业务逻辑下可能是不期望的)。如果业务上期望“只等待第一次超时”,建议统一加上 isActive 判断;如果期望“每次调用都重新计时”,则当前逻辑无误,但建议加上注释说明。
  2. isValid()serviceValid() 的语义差异

    • 当前代码:将 m_managerInter->serviceValid() 替换为 m_managerInter->isValid()
    • 分析:在 Qt DBus 中,QDBusAbstractInterface::isValid() 检查的是接口对象本身是否有效(即是否成功关联到DBus服务路径等);而某些自定义封装或旧版接口中的 serviceValid() 可能还会去总线上实时查询服务是否存在。请确认 isValid() 的语义足以覆盖原本 serviceValid() 的业务需求,避免出现接口对象缓存了旧状态而总线上服务已重启导致的判断失效问题。

二、 代码质量

  1. 日志信息的冗余与一致性

    • 当前代码
      • checkSystem 中:"Check system with update mode:" << updateMode << "check system stage:" << stage
      • doCheckSystem 中:"doCheckSystem Update mode:" << updateMode << ", check system stage:" << stage
    • 分析:当外部调用 checkSystem 且接口有效时,会立刻调用 doCheckSystem,这会导致几乎在同一时刻打印两条内容高度重合的日志,增加日志噪音,也不利于日志检索。
    • 建议:将详细参数日志只保留在 doCheckSystem 中,checkSystem 中只需打印调用入口日志即可;或者统一日志格式。例如:
      // checkSystem 中
      qCInfo(logUpdateModal) << "checkSystem called, checking interface validity...";
      
      // doCheckSystem 中保持不变,或优化为:
      qCInfo(logUpdateModal) << "doCheckSystem triggered, update mode:" << updateMode << ", stage:" << stage;
  2. 方法命名的语义区分

    • 分析checkSystemdoCheckSystem 的命名过于相似,在 C++ 中通常 doXxx 表示内部实际执行逻辑,xxx 表示外部带校验的逻辑,这种拆分是合理的。但为了避免后续维护者混淆,建议在头文件或方法声明处补充简短的 /** ... */ 注释,说明 checkSystem 包含了服务等待逻辑,而 doCheckSystem 是纯粹的 DBus 调用逻辑。

三、 代码性能

  1. 单例与频繁的接口有效性检查
    • 分析UpdateWorker::instance() 似乎是一个单例,m_dbusProxy 也是长期持有的对象。managerInterIsValid() 会被频繁调用(如每次 checkSystem 时)。
    • 建议isValid() 本身是轻量级操作(主要检查内部标志位),性能开销极小,当前设计没有问题。但如果 DBus 服务异常断开又重连,单纯依赖 isValid() 可能无法及时感知(因为接口对象的 isValid 在服务掉线时可能仍为 true,直到下一次调用报错)。可以考虑结合 DBus 的 serviceOwnerChanged 信号来主动更新状态,减少无效的 DBus 调用带来的性能开销(虽然这不是本次 diff 引入的问题,但作为长远优化建议提出)。

四、 代码安全

  1. 版权声明的合规性

    • 当前代码SPDX-FileCopyrightText: 2022 - 2026 UnionTech Software Technology Co., Ltd.
    • 分析:版权年份通常应该记录的是代码首次发布到当前修改的年份,例如 2022 - 20232022 - present。写死到 2026 年是不常见的做法,可能在开源合规审计时被标记。
    • 建议:确认是否应改为 2022 - 2023(假设今年是2023年修改)或 2022 - present
  2. DBus Pending Call 的异常处理

    • 分析:在 doCheckSystem 中,使用了 QDBusPendingCallWatcher 来异步处理 CheckUpgrade 的返回。这是 Qt DBus 的标准安全做法,不会阻塞主线程。
    • 建议:确保 watcherfinished 信号关联的 lambda 中有对 reply.isError() 的严格判断。如果 DBus 调用超时或服务端崩溃,必须要在 lambda 中 watcher->deleteLater(),否则会造成内存泄漏。(由于 diff 中未展示 lambda 内部的完整实现,仅作安全提醒)。

综合修改建议示例

基于以上分析,我为你提供一份优化后的代码片段参考:

src/dde-update/updateworker.cpp

void UpdateWorker::checkSystem(int updateMode, UpdateModel::CheckSystemStage stage)
{
    qCInfo(logUpdateModal) << "checkSystem called, checking interface validity...";
    if (!m_dbusProxy->managerInterIsValid()) {
        qCWarning(logUpdateModal) << "org.deepin.dde.Lastore1 interface is invalid, wait for service to be valid";
        m_waitingToCheckSystem = true;
        // 建议:如果业务要求每次调用重置定时器,则直接 start;如果要求不重置,需加 isActive 判断
        m_waitServiceTimer->start(); 
        return;
    }

    doCheckSystem(updateMode, stage);
}

void UpdateWorker::doCheckSystem(int updateMode, UpdateModel::CheckSystemStage stage)
{
    // 统一日志格式,避免与 checkSystem 打印重复信息
    qCInfo(logUpdateModal) << "doCheckSystem: update mode:" << updateMode << ", check system stage:" << stage;
    
    QDBusPendingReply<QDBusObjectPath> reply = m_dbusProxy->CheckUpgrade(updateMode, static_cast<int>(stage));
    QDBusPendingCallWatcher *watcher = new QDBusPendingCallWatcher(reply, this);
    connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, watcher] {
        // 安全提醒:确保在此处有如下类似逻辑防止内存泄漏
        // if (watcher->isError()) {
        //     qCWarning(...) << watcher->error().message();
        // } else {
        //     ...
        // }
        watcher->deleteLater(); 
    });
    // ... 其他逻辑
}

src/dde-update/updateworker.h

// 建议补充注释说明接口职责区分
void checkSystem(int updateMode, UpdateModel::CheckSystemStage stage); // 包含服务有效性检查及等待逻辑
void doCheckSystem(int updateMode, UpdateModel::CheckSystemStage stage); // 直接执行DBus检查调用

版权声明建议修改为:

// SPDX-FileCopyrightText: 2022 - present UnionTech Software Technology Co., Ltd.

希望这些审查意见对你有所帮助!如果还有其他代码需要审查,随时告诉我。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@xionglinlin xionglinlin merged commit 702d4e3 into linuxdeepin:master May 19, 2026
7 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