master

[Johnson-zs]

• chore: bump version to 1.3.56
• fix: change search worker connection to direct

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Johnson-zs

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[github-actions]

TAG Bot

TAG: 1.3.56
EXISTED: yes
DISTRIBUTION: unstable

[deepin-ci-robot]

deepin pr auto review

你好！我是CodeGeeX。我已仔细审查了你提供的Git Diff记录。本次修改主要涉及两个文件：Debian changelog的版本更新，以及搜索引擎核心类中信号槽连接方式的修改。

下面我将从语法逻辑、代码质量、代码性能和代码安全四个维度对代码修改进行详细的审查，并提供改进意见。

1. 语法逻辑

核心问题：跨线程使用 Qt::DirectConnection 导致的逻辑崩溃风险

在 genericsearchengine.cpp 中，你将 requestCancel 信号的连接方式强制指定为 Qt::DirectConnection：

connect(this, &GenericSearchEngine::requestCancel,
        m_worker, &SearchWorker::cancelSearch, Qt::DirectConnection);

从上下文注释 // 连接结果信号（工作线程 -> 主线程） 以及常规的搜索引擎架构来看，m_worker 是在一个独立的工作线程中运行的，而 GenericSearchEngine 对象通常生活在主线程。

• 默认行为分析：如果 GenericSearchEngine 和 m_worker 在不同线程，默认的 Qt::AutoConnection 会在发射信号时，自动将槽函数的调用以事件的形式投递到 m_worker 所在的线程事件循环中，这是线程安全的。
• 修改后行为分析：强制使用 Qt::DirectConnection 后，当主线程发射 requestCancel 信号时，cancelSearch() 槽函数将立即在主线程中被执行，而不是在工作线程中执行。
• 逻辑风险：
  1. 线程不安全：如果 cancelSearch() 内部访问了 m_worker 的成员变量（例如修改 m_isCanceled = true;），而工作线程的 doSearch() 也在读取该变量，这将构成数据竞争，导致未定义行为甚至程序崩溃。
  2. 语义错误：取消操作的本意是让工作线程停止当前耗时的搜索，如果取消动作在主线程同步执行，工作线程可能根本无法及时感知到取消请求，违背了取消逻辑的初衷。

2. 代码质量

• 缺乏必要的注释：跨线程的信号槽连接修改为 DirectConnection 是一种非常规且高风险的操作，但 Diff 中并未添加任何注释说明为何需要这样做。这会让后续维护者感到困惑。
• 潜在的架构不一致：requestSearch 依然使用默认的 AutoConnection（跨线程队列调用），而 requestCancel 却变成了 DirectConnection（跨线程直接调用）。这种不对称的连接方式增加了代码的理解难度和心智负担。

3. 代码性能

• Changelog 提到的性能优化未在代码中体现：Debian changelog 中明确写道 * perf: cache resolved indexed directories to avoid repeated resolution（性能优化：缓存已解析的索引目录以避免重复解析），但在当前 Diff 的 C++ 代码中，并未看到任何关于“缓存”、“目录解析”的逻辑修改，仅仅看到了信号槽连接的修改。这可能是提交时遗漏了文件，或者 Commit Message 与代码修改不匹配。
• DirectConnection 对性能的影响：使用 DirectConnection 确实省去了事件队列的投递和循环提取开销，在信号槽调用极其频繁的场景下能提升纳秒级的性能。但是，对于“取消搜索”这种极低频的操作（通常只在用户点击取消按钮时触发一次），这种性能优化毫无意义，反而牺牲了线程安全。

4. 代码安全

• 数据竞争与崩溃：如逻辑分析中所述，跨线程的 DirectConnection 极易引发数据竞争。如果 SearchWorker::cancelSearch() 中使用了互斥锁，而在 doSearch() 中也使用了相同的互斥锁，主线程的 DirectConnection 可能会因为尝试获取工作线程持有的锁而导致主线程阻塞（界面卡死），甚至如果锁是非递归的，还会导致死锁。

---

💡 改进建议

1. 移除 Qt::DirectConnection，恢复默认连接：
   除非你有极其特殊的理由（例如 cancelSearch 内部完全只使用了 std::atomic 原子变量且不需要在工作线程上下文执行后续清理），否则不应该打破 Qt 的跨线程信号槽安全机制。

2. 如果确实需要立即生效的取消逻辑，建议使用 QThread::requestInterruption()：
   自 Qt 5.2 起，提供了安全的线程中断机制。你可以利用它来替代自定义的取消信号。

3. 核对提交内容：
   请检查本次提交是否遗漏了实现“缓存索引目录”逻辑的代码文件。

🛠️ 推荐的代码修改方案

方案一：恢复默认（最推荐）
直接移除 Qt::DirectConnection，让 Qt 自动处理跨线程调用。

connect(this, &GenericSearchEngine::requestCancel,
        m_worker, &SearchWorker::cancelSearch); 
// 默认 Qt::AutoConnection，跨线程时自动变为 Qt::QueuedConnection，安全可靠

方案二：使用原子变量 + 安全的内存屏障（如果嫌事件队列太慢）
如果 doSearch 是一个紧密循环，且等不及事件循环去处理取消信号，应该在 SearchWorker 中使用 std::atomic<bool>。

// SearchWorker.h
std::atomic<bool> m_isCanceled{false};

// SearchWorker.cpp
void SearchWorker::cancelSearch() {
    m_isCanceled.store(true, std::memory_order_release);
}

void SearchWorker::doSearch() {
    // ... 密集循环中检查
    if (m_isCanceled.load(std::memory_order_acquire)) {
        break; // 提前退出
    }
}

注意：即使采用此方案，信号槽连接依然建议保持默认的 AutoConnection，因为槽函数只是修改了原子变量，开销极小，没必要为了这点开销去破坏 Qt 的线程模型。

方案三：使用 Qt 原生的中断机制

// 取消时
m_workerThread->requestInterruption(); 

// 工作线程的 doSearch 循环中
while(!QThread::currentThread()->isInterruptionRequested()) {
    // 搜索逻辑
}