Skip to content

fix: prevent duplicate plugin registration with mutex lock#82

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
wyu71:master
Dec 30, 2025
Merged

fix: prevent duplicate plugin registration with mutex lock#82
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
wyu71:master

Conversation

@wyu71
Copy link
Contributor

@wyu71 wyu71 commented Dec 30, 2025

  • Added QMutex to protect registration state in ServiceBase
  • Used QMutexLocker to ensure thread-safe register/unregister operations

Log: prevent duplicate plugin registration with mutex lock
pms: BUG-321175

- Added QMutex to protect registration state in ServiceBase
- Used QMutexLocker to ensure thread-safe register/unregister operations

Log: prevent duplicate plugin registration with mutex lock
pms: BUG-321175
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wyu71

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

@deepin-ci-robot
Copy link

deepin pr auto review

我来对这段代码修改进行审查:

  1. 语法逻辑:
  • 代码语法正确,使用了QMutexLocker来确保线程安全
  • 在ServiceBase中添加了mutable QMutex m_registerMutex成员变量
  • 在isRegister()、registerService()和unregisterService()函数中使用了QMutexLocker来保护m_isRegister的访问
  1. 代码质量:
  • 优点:
    • 使用RAII风格的QMutexLocker而不是手动lock/unlock,更安全
    • 在访问共享资源m_isRegister时都进行了加锁保护
    • 将互斥锁声明为mutable,可以在const函数中使用
  1. 代码性能:
  • 加锁会带来一定的性能开销,但考虑到这是服务注册/注销的关键操作,安全性比性能更重要
  • 建议考虑锁的粒度是否合适,目前整个函数都被锁保护,可以考虑只保护关键部分
  1. 代码安全:
  • 加锁保护了m_isRegister的并发访问,避免了竞态条件
  • 使用QMutexLocker确保了异常安全性,即使发生异常也会自动释放锁

改进建议:

  1. 可以考虑在ServiceQtDBus的registerService()和unregisterService()中,将锁的范围缩小到只保护关键代码段:
bool ServiceQtDBus::registerService()
{
    qCInfo(dsm_service_qt) << "service register: " << policy->name;

    if (libFuncCall("DSMRegister", true)) {
        QMutexLocker locker(&m_registerMutex);
        ServiceBase::registerService();
        return true;
    }
    return false;
}
  1. 考虑添加注释说明加锁的目的和保护的资源

  2. 可以考虑使用QRecursiveMutex而不是QMutex,以防未来可能出现的重入情况

  3. 建议在头文件中对新增的互斥锁添加注释说明其用途

总的来说,这是一个很好的改进,提高了代码的线程安全性,但还可以进一步优化锁的粒度和添加必要的注释。

@wyu71
Copy link
Contributor Author

wyu71 commented Dec 30, 2025

/forcemerge

@deepin-bot deepin-bot bot merged commit e6ba18d into linuxdeepin:master Dec 30, 2025
17 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