Skip to content

fix: add mutex protection for unique ID generation#450

Merged
18202781743 merged 1 commit intolinuxdeepin:masterfrom
18202781743:master
Apr 22, 2026
Merged

fix: add mutex protection for unique ID generation#450
18202781743 merged 1 commit intolinuxdeepin:masterfrom
18202781743:master

Conversation

@18202781743
Copy link
Copy Markdown
Contributor

@18202781743 18202781743 commented Apr 22, 2026

  1. Added QMutexLocker to protect the generateUniqueId method from
    concurrent access
  2. Added QMutexLocker to protect the removeUniqueId method from
    concurrent access
  3. Added m_idMutex member variable to util.h for thread synchronization
  4. This fixes potential race conditions when multiple threads call these
    methods simultaneously, ensuring unique IDs remain unique and preventing
    data corruption of m_currentIds

Influence:

  1. Test generating unique IDs from multiple threads simultaneously
  2. Verify that no duplicate IDs are produced under concurrent access
  3. Test removing IDs while other threads are generating new ones
  4. Verify no crashes or data corruption occurs with heavy multi-threaded
    usage
  5. Test with stress conditions (100+ concurrent threads)
  6. Verify that the application tray plugin works correctly with multi-
    threaded operations

fix: 添加互斥锁保护唯一ID生成

  1. 为 generateUniqueId 方法添加 QMutexLocker 保护并发访问
  2. 为 removeUniqueId 方法添加 QMutexLocker 保护并发访问
  3. 在 util.h 中添加 m_idMutex 成员变量用于线程同步
  4. 修复了多线程同时调用这些方法时的竞态条件问题,确保唯一ID保持唯一性,
    防止 m_currentIds 数据损坏

Influence:

  1. 测试多线程同时生成唯一ID
  2. 验证并发访问下不会产生重复ID
  3. 测试在生成新ID的同时移除其他ID的场景
  4. 验证高并发使用下不会出现崩溃或数据损坏
  5. 测试压力条件(100+并发线程)
  6. 验证应用程序托盘插件在多线程操作下正常工作

Summary by Sourcery

Add thread-safe protection around application tray unique ID management to prevent race conditions under concurrent access.

Bug Fixes:

  • Prevent duplicate or corrupted unique ID entries when generate/remove operations are invoked concurrently from multiple threads.

Enhancements:

  • Introduce a mutex to synchronize access to the current ID set in the application tray utility and update SPDX copyright years to cover 2024–2026.

1. Added QMutexLocker to protect the generateUniqueId method from
concurrent access
2. Added QMutexLocker to protect the removeUniqueId method from
concurrent access
3. Added m_idMutex member variable to util.h for thread synchronization
4. This fixes potential race conditions when multiple threads call these
methods simultaneously, ensuring unique IDs remain unique and preventing
data corruption of m_currentIds

Influence:
1. Test generating unique IDs from multiple threads simultaneously
2. Verify that no duplicate IDs are produced under concurrent access
3. Test removing IDs while other threads are generating new ones
4. Verify no crashes or data corruption occurs with heavy multi-threaded
usage
5. Test with stress conditions (100+ concurrent threads)
6. Verify that the application tray plugin works correctly with multi-
threaded operations

fix: 添加互斥锁保护唯一ID生成

1. 为 generateUniqueId 方法添加 QMutexLocker 保护并发访问
2. 为 removeUniqueId 方法添加 QMutexLocker 保护并发访问
3. 在 util.h 中添加 m_idMutex 成员变量用于线程同步
4. 修复了多线程同时调用这些方法时的竞态条件问题,确保唯一ID保持唯一性,
防止 m_currentIds 数据损坏

Influence:
1. 测试多线程同时生成唯一ID
2. 验证并发访问下不会产生重复ID
3. 测试在生成新ID的同时移除其他ID的场景
4. 验证高并发使用下不会出现崩溃或数据损坏
5. 测试压力条件(100+并发线程)
6. 验证应用程序托盘插件在多线程操作下正常工作
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 22, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds mutex-based synchronization around unique ID generation/removal in the application tray util to make ID management thread-safe, and updates the SPDX copyright years.

Sequence diagram for concurrent unique ID generation with mutex

sequenceDiagram
    actor ThreadA
    actor ThreadB
    participant UtilInstance as Util
    participant Mutex as m_idMutex

    ThreadA->>UtilInstance: generateUniqueId(id)
    UtilInstance->>Mutex: lock via QMutexLocker
    activate UtilInstance
    UtilInstance->>UtilInstance: iterate, check m_currentIds, insert newId

    ThreadB->>UtilInstance: generateUniqueId(id)
    UtilInstance->>Mutex: wait for lock

    UtilInstance-->>Mutex: unlock when QMutexLocker destroyed
    deactivate UtilInstance
    UtilInstance-->>ThreadA: return uniqueIdA

    UtilInstance->>Mutex: lock via QMutexLocker
    activate UtilInstance
    UtilInstance->>UtilInstance: iterate, check m_currentIds, insert newId
    UtilInstance-->>Mutex: unlock when QMutexLocker destroyed
    deactivate UtilInstance
    UtilInstance-->>ThreadB: return uniqueIdB (different from uniqueIdA)
Loading

Class diagram for updated Util ID synchronization

classDiagram
    class Util {
        +QString generateUniqueId(QString id)
        +void removeUniqueId(QString id)
        -QSet<QString> m_currentIds
        -QMutex m_idMutex
    }

    class QMutex {
    }

    class QSet_QString_ {
    }

    Util "1" --> "1" QMutex : uses m_idMutex
    Util "1" --> "1" QSet_QString_ : manages m_currentIds
Loading

File-Level Changes

Change Details Files
Synchronize unique ID generation and removal with a mutex to avoid race conditions on m_currentIds.
  • Wrap generateUniqueId body with QMutexLocker guarding access to the shared ID set
  • Wrap removeUniqueId body with QMutexLocker to serialize removals from the shared ID set
  • Introduce a QMutex member used exclusively to protect m_currentIds
plugins/application-tray/util.cpp
plugins/application-tray/util.h
Update SPDX copyright headers to reflect the 2024–2026 range.
  • Adjust SPDX-FileCopyrightText year range in util.cpp
  • Adjust SPDX-FileCopyrightText year range in util.h
plugins/application-tray/util.cpp
plugins/application-tray/util.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

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


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.

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码的修改主要是为了解决多线程访问 m_currentIds 集合时的线程安全问题。以下是对这段代码的详细审查和改进建议:

1. 语法逻辑

  • 修改内容:在 generateUniqueIdremoveUniqueId 函数中添加了 QMutexLocker 来保护对 m_currentIds 的访问。
  • 逻辑正确性:使用 QMutexLocker 是正确的做法,它会在构造时自动加锁,在析构时自动解锁,确保异常安全。

2. 代码质量

  • 优点
    • 使用 RAII(资源获取即初始化)风格的 QMutexLocker 是良好的实践,避免了手动加锁/解锁可能导致的错误。
    • 在头文件中添加了必要的 #include <QMutex>
  • 改进建议
    • 锁的粒度:当前锁的粒度是整个函数,对于 generateUniqueId 来说,循环中每次检查 m_currentIds 都持有锁,这可能会影响性能。可以考虑缩小锁的范围,但需要注意原子性。
    • 循环次数generateUniqueId 中硬编码了循环次数 100,如果超过 100 次仍未找到可用 ID,函数会返回空字符串。建议:
      • 增加日志记录,帮助调试 ID 生成失败的情况。
      • 考虑动态扩展 ID 生成策略,而不仅仅是追加数字。

3. 代码性能

  • 潜在问题
    • generateUniqueId 中每次循环都会创建新的 QString 对象并检查集合,这在高频调用时可能会有性能开销。
    • 持有锁的时间较长(尤其是循环期间),可能会阻塞其他线程对 m_currentIds 的访问。
  • 改进建议
    • 如果 ID 生成逻辑允许,可以先在锁外生成候选 ID,然后在锁内检查和插入。
    • 考虑使用更高效的数据结构(如 QSet 已经是哈希表实现,性能较好,但可以评估是否需要优化)。

4. 代码安全

  • 线程安全
    • 修改后,m_currentIds 的访问是线程安全的,因为所有访问都通过 QMutexLocker 保护。
  • 潜在风险
    • 如果 generateUniqueId 返回空字符串(循环 100 次未找到可用 ID),调用方需要正确处理这种情况,否则可能导致后续逻辑错误。建议:
      • 在函数文档中明确说明返回空字符串的条件。
      • 考虑抛出异常或提供其他错误处理机制。

5. 其他建议

  • 版权年份更新:修改了版权年份范围(2024 - 2026),这是合理的,但需要确保与项目的整体版权策略一致。
  • 代码注释:建议为 m_idMutex 添加注释,说明其用途(如“保护 m_currentIds 的线程安全访问”)。

改进后的代码示例

// util.h
class Util : public QObject
{
    // ... 其他成员 ...
private:
    QSet<QString> m_currentIds;
    QMutex m_idMutex; // 保护 m_currentIds 的线程安全访问
};

// util.cpp
QString Util::generateUniqueId(const QString &id)
{
    QMutexLocker locker(&m_idMutex);
    for (int i = 0; i < 100; i++) {
        QString newId = id + "-" + QString::number(i);
        if (!m_currentIds.contains(newId)) {
            m_currentIds.insert(newId);
            return newId;
        }
    }
    qWarning() << "Failed to generate unique ID for base ID:" << id;
    return QString(); // 返回空字符串表示失败
}

void Util::removeUniqueId(const QString &id) {
    QMutexLocker locker(&m_idMutex);
    m_currentIds.remove(id);
}

总结

这段修改解决了线程安全问题,但可以在性能和错误处理方面进一步优化。建议根据实际使用场景评估是否需要调整锁的粒度或 ID 生成策略。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@18202781743 18202781743 merged commit bdcbe70 into linuxdeepin:master Apr 22, 2026
11 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