Skip to content

fix: add wait mechanism for app icons#712

Merged
BLumia merged 1 commit into
linuxdeepin:masterfrom
qxp930712:master
Feb 27, 2026
Merged

fix: add wait mechanism for app icons#712
BLumia merged 1 commit into
linuxdeepin:masterfrom
qxp930712:master

Conversation

@qxp930712
Copy link
Copy Markdown

1. Implemented a pending check mechanism for app icons with absolute paths
2. When an app is added, check if the icon file exists immediately
3. If the file is missing, place the app in a pending queue and start a 1-second timer
4. Periodically check pending items to see if the icon file has become available
5. Added a 60-second timeout to force processing if files never appear
6. This resolves race conditions where app registration happens before icon data is fully synchronized to disk or mounted

Log: Added delayed loading mechanism for application icons to handle
temporary file unavailability

Influence:
1. Install an application where the icon file exists immediately; verify the icon displays instantly
2. Install an application where the icon file is missing at first but appears within 60 seconds; verify the icon displays once the file is detected
3. Install an application with a non-existent icon path; verify the application is processed after the 60-second timeout
4. Test adding multiple applications with delayed icons simultaneously
5. Verify system resource usage and memory cleanup when the timer stops

PMS: BUG-347859

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.

Sorry @qxp930712, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Comment thread src/ddeintegration/appmgr.cpp Outdated
, m_checkTimer(new QTimer(this))
, m_checkCount(0)
{
m_checkTimer->setInterval(1000); // 1 second interval
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
m_checkTimer->setInterval(1000); // 1 second interval
m_checkTimer->setInterval(3000); // 3 second interval

至少三秒吧,不然对于真的无效的 desktop 图标相当于要检查 60 次才会放弃

BLumia
BLumia previously approved these changes Feb 27, 2026
    1. Implemented a pending check mechanism for app icons with absolute
    paths
    2. When an app is added, check if the icon file exists immediately
    3. If the file is missing, place the app in a pending queue and start a
    1-second timer
    4. Periodically check pending items to see if the icon file has become
    available
    5. Added a 60-second timeout to force processing if files never appear
    6. This resolves race conditions where app registration happens before
    icon data is fully synchronized to disk or mounted

    Log: Added delayed loading mechanism for application icons to handle
    temporary file unavailability

    Influence:
    1. Install an application where the icon file exists immediately; verify
    the icon displays instantly
    2. Install an application where the icon file is missing at first but
    appears within 60 seconds; verify the icon displays once the file is
    detected
    3. Install an application with a non-existent icon path; verify the
    application is processed after the 60-second timeout
    4. Test adding multiple applications with delayed icons simultaneously
    5. Verify system resource usage and memory cleanup when the timer stops

    PMS: BUG-347859
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码实现了一个对应用图标文件进行延迟检查的机制,主要目的是处理那些图标路径是绝对路径但文件尚未创建的情况(可能是应用安装过程中的竞态条件)。

以下是对这段 diff 的详细审查意见,包括语法逻辑、代码质量、性能和安全性方面:

1. 语法逻辑

  • 逻辑正确性

    • 主要逻辑:代码通过 m_pendingAppItems 容器暂存图标文件不存在的应用,并使用 QTimer 每 3 秒检查一次文件是否生成,最多等待 60 秒(20次)。这个逻辑是合理的,能够解决文件系统异步操作导致的图标加载失败问题。
    • 超时处理m_checkCount >= 20 的判断逻辑正确,确保不会无限期等待。
    • 重置计数器:在 initObjectManager 中添加新应用时重置 m_checkCount = 0 是合理的,这表示有新的应用加入等待队列,重置超时逻辑有助于避免因旧应用的等待导致新应用被过早强制处理。
  • 潜在逻辑隐患

    • 重复添加风险:在 checkPendingAppItems 中,如果 m_checkCount >= 20,代码会强制将所有剩余的 m_pendingAppItems 移动到 m_appItems。然而,在 watchingAppItemAdded 中,如果 fileInfo.exists() 为真,会直接添加到 m_appItems。如果在这 60 秒内,同一个 key 的应用通过 DBus 信号(InterfacesAdded)再次触发 watchingAppItemAdded,且此时文件存在,可能会导致内存泄漏或重复添加(尽管代码中有 App already exists 的检查,但那是针对 m_appItems 的,m_pendingAppItems 中的旧指针可能未被清理)。建议在 watchingAppItemAdded 中也检查 m_pendingAppItems

2. 代码质量

  • 代码可读性与维护性

    • 魔法数字m_checkTimer->setInterval(3000)m_checkCount >= 20 使用了硬编码的魔法数字。建议定义常量,例如 static const int kCheckIntervalMs = 3000;static const int kMaxCheckCount = 20;,以提高代码可读性和可维护性。
    • 资源管理m_checkTimer 使用了 new QTimer(this),利用了 Qt 的对象树机制,父对象析构时会自动析构子对象,这部分内存管理是安全的。
    • 注释:关键逻辑处有注释,如 // File doesn't exist, add to pending container,这很好。
  • 代码风格

    • isAbsolutePathIcon 函数实现简单且正确,使用了 QString::startsWith
    • 使用了 qMakePair,在 C++11 及以后可以使用 {key, appItem}qMakePair 均可,风格统一即可。

3. 代码性能

  • I/O 操作

    • QFileInfo fileInfo(appItem->iconName); fileInfo.exists() 涉及磁盘 I/O 操作。在 checkPendingAppItems 中,每 3 秒遍历一次所有待处理应用进行文件检查。如果 m_pendingAppItems 数量非常大(例如几百个),可能会造成瞬时的 I/O 峰值。
    • 改进建议:目前的实现对于桌面环境应用管理器来说,待处理应用数量通常很少,性能影响可以忽略。但如果考虑到极端情况,可以考虑限制单次检查的数量,或者保持现状,因为 3 秒的间隔已经足够缓冲。
  • 容器操作

    • 使用了 QMap,查找和删除的时间复杂度为 O(log n)。在 checkPendingAppItems 中先收集 itemsToProcess 再统一处理,避免了在遍历 m_pendingAppItems 时修改 m_appItems 导致潜在的信号发射副作用,这是一个好的做法。

4. 代码安全

  • 指针安全

    • 代码中大量使用了原始指针 (AppItem *)。Qt 中使用原始指针配合 QObject 父子关系通常是安全的,但在 m_pendingAppItemsm_appItems 之间转移指针所有权时,需要确保没有其他地方持有这些指针并在对象被删除后使用它们。
    • 悬空指针风险:如果 AppItemAppMgr 以外的逻辑删除,这里的容器就会持有悬空指针。不过看代码结构,AppMgr 似乎是这些 AppItem 的管理者,这种风险较低。
  • 线程安全

    • 代码涉及 QTimer 和 DBus 回调,这些都运行在主线程(假设 AppMgr 在主线程创建)。QMap 不是线程安全的,但因为没有跨线程访问,所以是安全的。

改进建议总结

  1. 消除魔法数字:将 300020 定义为类内静态常量。
  2. 防止重复添加:在 watchingAppItemAdded 函数中,除了检查 m_appItems,还应检查并移除 m_pendingAppItems 中可能存在的相同 key,以防逻辑冲突。
  3. 逻辑优化:在 checkPendingAppItems 中,m_checkCount 的递增是在检查之前进行的。这意味着如果第一次检查时文件就存在,m_checkCount 也会变为 1。虽然逻辑上不影响最终结果(因为队列空了会停止),但语义上稍微有点偏差。不过这属于极细微的瑕疵,不改也无妨。

修改后的代码片段建议

针对第 1 和 第 2 点的建议:

// appmgr.h
class AppMgr : public QObject
{
    // ...
private:
    static const int kCheckIntervalMs = 3000;
    static const int kMaxCheckCount = 20;
    // ...
};

// appmgr.cpp
void AppMgr::watchingAppItemAdded(const QString &key, AppItem *appItem)
{
    // 如果已经在待处理列表中,先移除,避免重复或逻辑混乱
    if (m_pendingAppItems.contains(key)) {
        // 注意:这里可能需要根据业务逻辑决定是否要 delete 旧的 appItem
        // 如果 AppMgr 拥有所有权,且 appItem 是新创建的,旧的可能需要 delete
        // 但根据上下文,DBus 通知通常意味着是同一个对象,或者需要替换
        // 简单起见,这里假设是替换引用
        m_pendingAppItems.remove(key);
    }

    if (isAbsolutePathIcon(appItem->iconName)) {
        QFileInfo fileInfo(appItem->iconName);
        if (!fileInfo.exists()) {
            m_pendingAppItems[key] = appItem;
            if (!m_checkTimer->isActive()) {
                m_checkCount = 0; // 开始新的等待周期时重置计数
                m_checkTimer->start();
            }
            return;
        }
    }
    
    m_appItems[key] = appItem;
    watchingAppItemPropertyChanged(key, appItem);
    Q_EMIT changed();
}

void AppMgr::checkPendingAppItems()
{
    // 如果列表为空,停止计时器
    if (m_pendingAppItems.isEmpty()) {
        m_checkTimer->stop();
        return;
    }

    m_checkCount++;
    
    // ... 其余逻辑不变 ...

    // 使用常量替代魔法数字
    if (m_checkCount >= kMaxCheckCount && !m_pendingAppItems.isEmpty()) {
        // ... 强制处理逻辑 ...
    }
}

总体而言,这段代码的修改是稳健的,能够解决实际问题,且没有引入明显的安全漏洞。主要的改进空间在于消除硬编码和增强边界条件的处理。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, qxp930712

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

@BLumia BLumia merged commit f26b5ff into linuxdeepin:master Feb 27, 2026
9 of 10 checks passed
}

// Check if timeout reached (60 seconds)
if (m_checkCount >= 20 && !m_pendingAppItems.isEmpty()) {
Copy link
Copy Markdown
Contributor

@18202781743 18202781743 Feb 28, 2026

Choose a reason for hiding this comment

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

安装驱动后,会弹出一个继续的对话框,确认后才会创建图标链接,如果60s后才点击对话框的确认,那这个修改方式仍然存在问题呀,
对于填写的不是绝对路径的图标,这个也不能起作用了呀,
如果在60s内又卸载了这个应用,那60s之后它还会显示在启动器里么?

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.

5 participants