Skip to content

fix: make upgrade delivery property reads async - #338

Merged
caixr23 merged 1 commit into
linuxdeepin:masterfrom
caixr23:master
Sep 3, 2026
Merged

fix: make upgrade delivery property reads async#338
caixr23 merged 1 commit into
linuxdeepin:masterfrom
caixr23:master

Conversation

@caixr23

@caixr23 caixr23 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor
  1. Add asyncGetUploadLimitSpeed/asyncGetDownloadLimitSpeed to UpdateAssistant, issuing async org.freedesktop.DBus.Properties.Get calls instead of blocking synchronous property reads with 25s default timeout
  2. Stop re-reading properties synchronously in onPropertyChanged, emit the new values carried by PropertiesChanged directly
  3. Add refreshUpgradeDeliveryDownloadLimitSpeed/ refreshUpgradeDeliveryUploadLimitSpeed in UpdateWorker: fetch speed limit asynchronously via QDBusPendingCallWatcher, update model only when reply is valid and non-empty, keep current config and log a warning otherwise
  4. Switch refreshUpgradeDeliveryInfo (activate path), the setUpgradeDeliveryEnabled success callback and the QML-invoked getUpgradeDeliveryXxxLimitSpeed to the async refresh helpers; public signatures unchanged so QML needs no adaptation

Log: Make upgradedelivery limit-speed property reads asynchronous to avoid occasional 30s UI stalls when
org.deepin.upgradedelivery responds slowly or hangs

Influence:

  1. Repeatedly enter update page, verify no long stall and limit speed values still display correctly via model NOTIFY bindings
  2. Stop org.deepin.upgradedelivery service and enter page: UI must not block, warning is logged and current config is kept

fix: 将升级传递属性读取改为异步避免界面卡顿

  1. 在 UpdateAssistant 中新增 asyncGetUploadLimitSpeed/ asyncGetDownloadLimitSpeed,通过异步的 org.freedesktop.DBus.Properties.Get 调用替代默认超时 25 秒 的同步属性读取
  2. onPropertyChanged 不再同步回读属性,直接使用 PropertiesChanged 信号携带的新值发出通知
  3. 在 UpdateWorker 中新增 refreshUpgradeDeliveryDownloadLimitSpeed/ refreshUpgradeDeliveryUploadLimitSpeed:通过 QDBusPendingCallWatcher 异步获取限速配置,仅当返回有效且 非空时更新 model,否则保持当前配置并输出 warning 日志
  4. 将 refreshUpgradeDeliveryInfo(activate 启动路径)、 setUpgradeDeliveryEnabled 成功回调以及 QML 调用的 getUpgradeDeliveryXxxLimitSpeed 切换到上述异步刷新方法; 对外接口签名不变,QML 无需适配

Log: 将升级传递限速属性的读取改为异步,避免
org.deepin.upgradedelivery 响应缓慢或无响应时界面偶发卡顿
30 秒的问题

Influence:

  1. 反复进入更新页,验证不再出现长时间卡顿,且限速值通过 model NOTIFY 绑定正常显示
  2. 停止 org.deepin.upgradedelivery 服务后进入页面:界面不得 阻塞、有 warning 日志、保持当前配置

PMS: BUG-372823

Summary by Sourcery

Make upgrade delivery speed-limit reads asynchronous to prevent UI stalls and safely retain current settings when the service cannot provide valid values.

Bug Fixes:

  • Prevent upgrade delivery speed-limit property reads from blocking the UI when the service is slow or unavailable.
  • Preserve existing speed-limit configurations and log warnings when asynchronous property retrieval fails or returns empty values.

Enhancements:

  • Use asynchronous DBus property retrieval throughout upgrade delivery refresh and configuration flows, while emitting values directly from property-change notifications.

1. Add asyncGetUploadLimitSpeed/asyncGetDownloadLimitSpeed to
   UpdateAssistant, issuing async org.freedesktop.DBus.Properties.Get
   calls instead of blocking synchronous property reads with 25s
   default timeout
2. Stop re-reading properties synchronously in onPropertyChanged,
   emit the new values carried by PropertiesChanged directly
3. Add refreshUpgradeDeliveryDownloadLimitSpeed/
   refreshUpgradeDeliveryUploadLimitSpeed in UpdateWorker: fetch
   speed limit asynchronously via QDBusPendingCallWatcher, update
   model only when reply is valid and non-empty, keep current
   config and log a warning otherwise
4. Switch refreshUpgradeDeliveryInfo (activate path), the
   setUpgradeDeliveryEnabled success callback and the QML-invoked
   getUpgradeDeliveryXxxLimitSpeed to the async refresh helpers;
   public signatures unchanged so QML needs no adaptation

Log: Make upgradedelivery limit-speed property reads asynchronous
to avoid occasional 30s UI stalls when
org.deepin.upgradedelivery responds slowly or hangs

Influence:
1. Repeatedly enter update page, verify no long stall and limit
   speed values still display correctly via model NOTIFY bindings
2. Stop org.deepin.upgradedelivery service and enter page: UI must
   not block, warning is logged and current config is kept

fix: 将升级传递属性读取改为异步避免界面卡顿

1. 在 UpdateAssistant 中新增 asyncGetUploadLimitSpeed/
   asyncGetDownloadLimitSpeed,通过异步的
   org.freedesktop.DBus.Properties.Get 调用替代默认超时 25 秒
   的同步属性读取
2. onPropertyChanged 不再同步回读属性,直接使用
   PropertiesChanged 信号携带的新值发出通知
3. 在 UpdateWorker 中新增
   refreshUpgradeDeliveryDownloadLimitSpeed/
   refreshUpgradeDeliveryUploadLimitSpeed:通过
   QDBusPendingCallWatcher 异步获取限速配置,仅当返回有效且
   非空时更新 model,否则保持当前配置并输出 warning 日志
4. 将 refreshUpgradeDeliveryInfo(activate 启动路径)、
   setUpgradeDeliveryEnabled 成功回调以及 QML 调用的
   getUpgradeDeliveryXxxLimitSpeed 切换到上述异步刷新方法;
   对外接口签名不变,QML 无需适配

Log: 将升级传递限速属性的读取改为异步,避免
org.deepin.upgradedelivery 响应缓慢或无响应时界面偶发卡顿
30 秒的问题

Influence:
1. 反复进入更新页,验证不再出现长时间卡顿,且限速值通过
   model NOTIFY 绑定正常显示
2. 停止 org.deepin.upgradedelivery 服务后进入页面:界面不得
   阻塞、有 warning 日志、保持当前配置

PMS: BUG-372823
@caixr23
caixr23 requested a review from xionglinlin August 28, 2026 01:18
@sourcery-ai

sourcery-ai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Reviewer's Guide

Upgrade-delivery upload and download limit-speed reads now use asynchronous DBus calls throughout, eliminating potential 25-second UI stalls while preserving existing model values on failed or empty responses.

Sequence diagram for asynchronous upgrade-delivery speed refresh

sequenceDiagram
    participant UI as QML UI
    participant Worker as UpdateWorker
    participant Assistant as UpdateAssistant
    participant DBus as upgradedelivery DBus service
    participant Model as Update model

    UI->>Worker: getUpgradeDeliveryDownloadLimitSpeed()
    Worker->>Assistant: asyncGetDownloadLimitSpeed()
    Assistant->>DBus: Properties.Get(DownloadLimitSpeed)
    DBus-->>Assistant: QDBusPendingReply
    Assistant-->>Worker: QDBusPendingCallWatcher.finished
    alt valid non-empty speed
        Worker->>Model: setUpgradeDownloadSpeedLimitConfig()
        Model-->>UI: NOTIFY updated value
    else error or empty speed
        Worker-->>Worker: qCWarning()
        Worker->>Model: keep current configuration
    end
Loading

Sequence diagram for direct PropertiesChanged speed notifications

sequenceDiagram
    participant DBus as upgradedelivery DBus service
    participant Assistant as UpdateAssistant
    participant Model as Update model

    DBus-->>Assistant: PropertiesChanged(UploadLimitSpeed, DownloadLimitSpeed)
    Assistant->>Assistant: onPropertyChanged()
    Assistant-->>Model: UploadLimitSpeedChanged(new value)
    Assistant-->>Model: DownloadLimitSpeedChanged(new value)
Loading

File-Level Changes

Change Details Files
Replaced synchronous DBus limit-speed property reads with asynchronous Properties.Get calls and direct signal-value propagation.
  • Added asynchronous upload/download property accessors using org.freedesktop.DBus.Properties.Get.
  • Updated PropertiesChanged handling to emit values from the signal payload without re-reading properties synchronously.
src/common/dbus/updateassistant.cpp
src/common/dbus/updateassistant.h
Moved upgrade-delivery limit-speed model refreshes onto asynchronous pending-call workflows with failure-safe updates.
  • Added watcher-based download and upload refresh helpers.
  • Updated activation, enable-success, and QML-invoked read paths to use the async helpers.
  • Updated the model only for successful, non-empty replies; otherwise retained existing configuration and logged warnings.
src/dcc-update-plugin/operation/updatework.cpp
src/dcc-update-plugin/operation/updatework.h

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 found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src/dcc-update-plugin/operation/updatework.cpp" line_range="1881" />
<code_context>
+            qCWarning(logDccUpdatePlugin) << "async get download limit speed is empty";
+            return;
+        }
+        m_model->setUpgradeDownloadSpeedLimitConfig(transferDeliveryConfigToLastoreDeliveryConfig(speed).toUtf8());
+    });
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** The refresh callbacks accept any non-empty DBus string as valid and pass it to `transferDeliveryConfigToLastoreDeliveryConfig`; malformed or otherwise invalid JSON is converted into a default/empty configuration and overwrites the model instead of preserving the current configuration.

**Triggers:** When org.deepin.upgradedelivery returns a non-empty malformed or invalid limit-speed configuration.

**Suggested fix:** Validate `UpgradeSpeedLimitConfig::fromJson` successfully parsed the returned configuration before updating the model.
</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.

qCWarning(logDccUpdatePlugin) << "async get download limit speed is empty";
return;
}
m_model->setUpgradeDownloadSpeedLimitConfig(transferDeliveryConfigToLastoreDeliveryConfig(speed).toUtf8());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (bug_risk): The refresh callbacks accept any non-empty DBus string as valid and pass it to transferDeliveryConfigToLastoreDeliveryConfig; malformed or otherwise invalid JSON is converted into a default/empty configuration and overwrites the model instead of preserving the current configuration.

Triggers: When org.deepin.upgradedelivery returns a non-empty malformed or invalid limit-speed configuration.

Suggested fix: Validate UpgradeSpeedLimitConfig::fromJson successfully parsed the returned configuration before updating the model.

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 96 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 本次变更将DBus属性同步读取改为异步读取,有效解决界面卡顿问题。代码逻辑正确,异步模式使用规范,错误处理完善。未发现安全漏洞,仅存在少量代码重复和防御性编程改进空间。

📝 变更概述

项目 信息
PR linuxdeepin/deepin-update-ui#338
标题 fix: make upgrade delivery property reads async
作者 caixr23
修改文件 4 个 C++ 文件(+72 行,-8 行)
关联Bug PMS: BUG-372823
分析模式 全量分析(GitHub 平台)

变更目的: 将升级传递限速属性的读取改为异步,避免 org.deepin.upgradedelivery 响应缓慢或无响应时界面偶发卡顿 30 秒的问题。


🔍 详细分析

1. 语法逻辑 ✅

评价: 语法正确,逻辑清晰 ✅ 通过(23/25 分)

潜在问题:

  1. refreshUpgradeDeliveryDownloadLimitSpeed()refreshUpgradeDeliveryUploadLimitSpeed() 方法内部未对 m_updateAssistant 进行空指针检查,依赖调用方保证非空。虽然当前所有调用方(getUpgradeDeliveryDownloadLimitSpeedrefreshUpgradeDeliveryInfosetUpgradeDeliveryEnabled 回调)都做了空指针检查,但存在防御性编程不足的风险。(src/dcc-update-plugin/operation/updatework.cpp,第 1866/1885 行)

建议: 建议在 refreshUpgradeDeliveryDownloadLimitSpeed()refreshUpgradeDeliveryUploadLimitSpeed() 方法入口处增加 m_updateAssistant 空指针检查,增强代码健壮性。


2. 代码质量 ✅

评价: 代码结构清晰,注释完整 ✅ 通过(23/25 分)

潜在问题:

  1. refreshUpgradeDeliveryDownloadLimitSpeed()refreshUpgradeDeliveryUploadLimitSpeed() 两个方法代码结构高度相似,仅在调用的异步方法名和 model 设置方法上不同,存在代码重复。(src/dcc-update-plugin/operation/updatework.cpp,第 1866-1902 行)

建议: 可考虑将两个 refresh 方法合并为一个通用方法,通过参数区分上传/下载方向,减少代码重复。但考虑到仅 2 个方法且逻辑清晰,当前重复程度可接受。


3. 代码性能 ✅

评价: 性能良好,资源使用合理 ✅ 通过(20/20 分)

潜在问题:
✅ 未发现性能问题

建议: 本次变更的核心目标就是性能优化:将同步 DBus 属性读取(25 秒超时)替换为异步读取,有效避免 UI 线程阻塞。QDBusPendingCallWatcher 使用规范,watcher 通过 deleteLater() 及时释放,空值检查避免了不必要的 model 更新。onPropertyChanged 改为直接使用信号携带的值,避免了冗余的同步回读操作。


4. 代码安全 🔒

评价: 存在0个安全漏洞 ✅ 通过(30/30 分)

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个

建议: 本次变更未引入安全漏洞。asyncGetProperty 方法仅接受内部硬编码的属性名,不存在注入风险。DBus 消息通过 Qt API 正确构造,错误处理完善(错误日志记录 + 保持当前配置)。安全扫描器标记的 7 个 RCE 风险均为预存代码中的误报(DBus 方法调用被误识别为命令执行),均不在本次变更范围内。


💡 改进建议代码示例

// 建议在 refresh 方法中增加空指针检查:
void UpdateWorker::refreshUpgradeDeliveryDownloadLimitSpeed()
{
    if (!m_updateAssistant) {
        qCWarning(logDccUpdatePlugin) << "m_updateAssistant is null, skip refresh download limit speed";
        return;
    }
    auto watcher = new QDBusPendingCallWatcher(m_updateAssistant->asyncGetDownloadLimitSpeed(), this);
    // ... 其余逻辑不变
}

📋 审查检查清单

  • 已按四维度标准完成代码分析(语法逻辑、代码质量、代码性能、代码安全)
  • 代码安全维度第一行包含"存在0个安全漏洞"
  • 安全漏洞已按等级分类(本次无漏洞)
  • 包含漏洞对比统计行(新增/减少/持平)
  • 每个维度标记✓/✕与评价词一致
  • 审查结论考虑了 commit message 的目的(异步化避免卡顿)
  • 问题定位准确,行号和函数名正确
  • 环境变量使用正确,文件路径无误

本报告由 AI 代码审查工具自动生成 | 扫描时间: 2026-08-28 09:18:00

@deepin-bot

deepin-bot Bot commented Sep 1, 2026

Copy link
Copy Markdown

TAG Bot

New tag: 1.0.62
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #340

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: caixr23, 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

@caixr23
caixr23 merged commit 6a3694c into linuxdeepin:master Sep 3, 2026
6 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