Skip to content

fix: correct DBus service validity check in UpdateDbusProxy#297

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

fix: correct DBus service validity check in UpdateDbusProxy#297
xionglinlin merged 1 commit into
linuxdeepin:masterfrom
xionglinlin:master

Conversation

@xionglinlin
Copy link
Copy Markdown
Contributor

Changed the validity check from isValid() to serviceValid() for the DBus manager interface. The original method incorrectly checked the interface object itself rather than verifying whether the underlying DBus service is actually registered and available. This caused status retrieval issues after the lastore-daemon service registration, as the check would succeed even when the service wasn't fully operational.

Log: Fixed service status check in DBus proxy

Influence:

  1. Test service registration and status retrieval flow for lastore- daemon
  2. Verify that proxy correctly detects when DBus service is unavailable
  3. Test edge cases where service deregisters during operation

fix: 修正 UpdateDbusProxy 中 DBus 服务的有效性检查

将有效性检查从 isValid() 改为 serviceValid()。原先的方法错误地检查 了接口对象本身,而不是验证底层 DBus 服务是否已真正注册并可用。这会导致在
lastore-daemon 服务注册后获取状态出现问题,因为在服务未完全就绪时也能通
过检查。

Log: 修复 DBus 代理中的服务状态检查

Influence:

  1. 测试 lastore-daemon 的服务注册和状态获取流程
  2. 验证代理在 DBus 服务不可用时能正确检测
  3. 测试服务在运行中注销的边界情况

PMS: BUG-361789
Change-Id: Ie796f222f20cb83ef8e04f2723bdad1d1f5e766f

Changed the validity check from `isValid()` to `serviceValid()` for
the DBus manager interface. The original method incorrectly checked the
interface object itself rather than verifying whether the underlying
DBus service is actually registered and available. This caused status
retrieval issues after the lastore-daemon service registration, as the
check would succeed even when the service wasn't fully operational.

Log: Fixed service status check in DBus proxy

Influence:
1. Test service registration and status retrieval flow for lastore-
daemon
2. Verify that proxy correctly detects when DBus service is unavailable
3. Test edge cases where service deregisters during operation

fix: 修正 UpdateDbusProxy 中 DBus 服务的有效性检查

将有效性检查从 `isValid()` 改为 `serviceValid()`。原先的方法错误地检查
了接口对象本身,而不是验证底层 DBus 服务是否已真正注册并可用。这会导致在
lastore-daemon 服务注册后获取状态出现问题,因为在服务未完全就绪时也能通
过检查。

Log: 修复 DBus 代理中的服务状态检查

Influence:
1. 测试 lastore-daemon 的服务注册和状态获取流程
2. 验证代理在 DBus 服务不可用时能正确检测
3. 测试服务在运行中注销的边界情况

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

deepin pr auto review

你好!我是CodeGeeX,你的智能编程助手。我已仔细审查了你提供的 Git Diff 代码。

本次修改仅涉及一行代码的变更,将 updatedbusproxy.hmanagerInterIsValid() 函数的判断逻辑从 m_managerInter->isValid() 修改为了 m_managerInter->serviceValid()

以下是针对本次修改在语法逻辑、代码质量、代码性能和代码安全方面的详细审查意见:

1. 语法与逻辑审查

  • 逻辑变更分析:在 Qt D-Bus 的 QDBusAbstractInterface 类中,isValid()serviceValid() 的行为有重要区别:
    • isValid():不仅检查 D-Bus 服务是否存在,还检查底层 QDBusConnection 是否有效、内部指针是否为空等。如果 D-Bus 守护进程崩溃或连接断开,它会返回 false
    • serviceValid()检查远程服务是否在总线上注册(即服务名是否存在),它不检查底层 QDBusConnection 连接本身是否完好。
  • 潜在逻辑风险:如果将判断条件改为 serviceValid(),当底层 D-Bus 系统总线连接断开(例如 dbus-daemon 崩溃重启),但服务名尚未从总线上注销时,managerInterIsValid() 可能会返回 true。此时如果调用者基于此判断去调用 D-Bus 方法,将会引发错误或收到异常的 QDBusReply。
  • 改进建议:请确认你的业务场景。如果你仅仅关心远端 UpdateManager 服务是否正在运行,使用 serviceValid() 是合理的;但如果你需要确保当前 D-Bus 通道完全正常可以发起调用,isValid() 通常是更严谨、更安全的选择。如果坚持使用 serviceValid(),建议在调用 D-Bus 方法时增加对异步错误的处理机制。

2. 代码质量审查

  • 命名一致性:函数名为 managerInterIsValid(),字面意思是“接口是否有效”。现在内部实现却调用了 serviceValid(),这产生了语义上的不一致,容易给后续维护者带来困惑(“到底判断的是接口有效还是服务有效?”)。
  • 改进建议:如果确定要修改逻辑,建议同步修改函数名,使其与实际行为相符。例如,可以改为 managerServiceIsValid()managerServiceExists(),以提高代码的可读性和自解释性。

3. 代码性能审查

  • 性能影响isValid()serviceValid() 都是轻量级的内联或短小函数,主要进行指针判空和标志位检查,不涉及系统调用或跨进程通信(IPC)。因此,本次修改对性能几乎没有影响。
  • 现有逻辑优化return m_managerInter && m_managerInter->serviceValid(); 中的短路求值逻辑是正确且高效的,如果 m_managerInter 为空,将直接返回 false,不会引发空指针解引用。

4. 代码安全审查

  • 空指针安全:当前的短路求值 m_managerInter && ... 已经很好地防范了空指针解引用的风险,这部分是安全的。
  • 调用安全(核心风险):如前文逻辑分析所述,serviceValid() 无法保证 D-Bus 连接的绝对可用性。如果在网络抖动、总线重启等极端场景下,依赖 managerInterIsValid() 返回 true 来决定是否执行后续关键操作(如 ConfirmRollbackPoweroff),可能会导致系统状态不一致。
  • 改进建议:无论使用哪种校验方式,调用端代码必须处理 D-Bus 调用失败的情况。由于你返回的是 QDBusPendingCall(异步调用),请确保在 QDBusPendingCallWatcherfinished 信号回调中,严格检查 QDBusReply::isValid()QDBusError,防止静默失败。

总结与修改建议代码

如果你确认业务上只需要判断服务是否存在,建议对函数名进行同步重构,并在注释中说明原因,避免后续维护者误改回 isValid()

// 修改前:
// bool managerInterIsValid() const { return m_managerInter && m_managerInter->isValid(); }

// 修改后建议:
// 注意:此处使用 serviceValid() 仅校验远端服务是否在总线注册,不校验底层 QDBusConnection 的连通性。
// 调用方必须处理异步调用可能产生的 QDBusError。
bool managerServiceIsValid() const { return m_managerInter && m_managerInter->serviceValid(); }

如果你发现实际上是需要保证通道完全可用,建议撤销此次修改,保留原有的 isValid()

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