Skip to content

fix: change search worker connection to direct#297

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
Johnson-zs:master
May 22, 2026
Merged

fix: change search worker connection to direct#297
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
Johnson-zs:master

Conversation

@Johnson-zs
Copy link
Copy Markdown
Contributor

  1. Modified the connection type between GenericSearchEngine and
    SearchWorker for cancel request from default to Qt::DirectConnection
  2. This ensures immediate execution of cancel search operations instead
    of being queued in event loop

Log: Improved search cancellation responsiveness

Influence:

  1. Test search cancellation in various scenarios
  2. Verify immediate termination of search operations
  3. Check for any potential threading issues with direct connection

fix: 将搜索工作线程连接改为直接连接

  1. 将GenericSearchEngine与SearchWorker之间的取消请求连接类型从默认改为
    Qt::DirectConnection
  2. 这确保了立即执行取消搜索操作而不是排队到事件循环中

Log: 改进了搜索取消的响应速度

Influence:

  1. 在各种场景下测试搜索取消功能
  2. 验证搜索操作是否能立即终止
  3. 检查直接连接可能带来的线程问题

1. Modified the connection type between GenericSearchEngine and
SearchWorker for cancel request from default to Qt::DirectConnection
2. This ensures immediate execution of cancel search operations instead
of being queued in event loop

Log: Improved search cancellation responsiveness

Influence:
1. Test search cancellation in various scenarios
2. Verify immediate termination of search operations
3. Check for any potential threading issues with direct connection

fix: 将搜索工作线程连接改为直接连接

1. 将GenericSearchEngine与SearchWorker之间的取消请求连接类型从默认改为
Qt::DirectConnection
2. 这确保了立即执行取消搜索操作而不是排队到事件循环中

Log: 改进了搜索取消的响应速度

Influence:
1. 在各种场景下测试搜索取消功能
2. 验证搜索操作是否能立即终止
3. 检查直接连接可能带来的线程问题
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff代码。这段代码修改了信号槽的连接方式,将取消搜索的信号连接改为了 Qt::DirectConnection

下面我将从语法逻辑、代码质量、代码性能和代码安全四个维度为你进行详细的审查和改进建议:

1. 语法逻辑

  • 跨线程的 DirectConnection 逻辑风险
    从上下文注释 // 连接结果信号(工作线程 -> 主线程) 可以推断,m_worker 是在一个独立的工作线程中运行的,而 GenericSearchEngine 对象在主线程。
    在Qt中,跨线程的默认连接方式是 Qt::Que列连接,这保证了槽函数会在接收者所在的线程中被调用,从而保证线程安全。
    如果你强制使用 Qt::DirectConnection,这意味着当主线程发出 requestCancel 信号时,m_worker->cancelSearch() 函数将直接在主线程中同步执行,而不是在工作线程中执行。
  • 逻辑矛盾:通常,取消一个耗时搜索操作的目的,是中断工作线程中的死循环或阻塞逻辑。如果 cancelSearch 在主线程执行(仅仅是设置了一个取消标志),这在逻辑上是可行的;但如果 cancelSearch 包含了耗时操作或需要立即停止工作线程中的当前任务,直接在主线程调用会导致主线程被阻塞,甚至引发递归死锁。

2. 代码质量

  • 可读性与意图表达:直接添加 Qt::DirectConnection 掩盖了开发者的真实意图。阅读代码的人无法立刻理解为什么这里需要打破Qt的跨线程默认安全机制。
  • 建议:如果确实需要 DirectConnection(通常是为了极低延迟地设置原子变量标志位以中止工作线程的循环),应当添加详细的行内注释说明原因,否则后续维护者很容易将其当作Bug删掉。

3. 代码性能

  • 减少延迟:使用 DirectConnection 的性能优势在于零延迟。信号发出后,槽函数立即执行,不用等待目标线程的事件循环被唤醒。
  • 适用场景:如果 cancelSearch 的实现仅仅是类似 m_cancelFlag.store(true);m_timer.invalidate(); 这样的轻量级原子操作,使用 DirectConnection 可以让工作线程在下一次循环检查时立刻发现取消请求,这在极高频率的搜索中是有性能意义的。
  • 反面影响:如果 cancelSearch 内部有锁操作(如 QMutexLocker),跨线程 DirectConnection 会导致主线程去争抢工作线程的锁,如果工作线程正持有锁在执行搜索,主线程会被阻塞,导致UI卡顿,反而降低了整体性能体验。

4. 代码安全

  • 线程安全与数据竞争:这是最大的风险点。使用 DirectConnection 绕过了Qt的信号槽线程安全机制,使得主线程直接访问工作线程对象的成员变量。必须确保 cancelSearch 内部访问的所有数据都是线程安全的。
    • 如果使用了 std::atomic<bool>QAtomicInt 作为取消标志,这是安全的。
    • 如果使用了普通的 bool m_cancelled; 且没有加锁,这将产生严重的数据竞争,导致未定义行为。
    • 如果 cancelSearch 内部会触发工作线程对象的析构或内存释放,将导致灾难性的崩溃。
  • 死锁风险:假设主线程发出 requestCancel,通过 DirectConnection 同步调用了 cancelSearch,而 cancelSearch 内部试图等待工作线程结束,但工作线程此时正在等待主线程的事件循环,就会产生死锁。

改进建议与代码示例

方案一:如果 cancelSearch 仅仅是设置原子变量(推荐保持当前逻辑但增加注释)

如果经过确认,cancelSearch 确实只是轻量级的原子操作,为了追求极致的响应速度,可以保留 DirectConnection,但必须加上注释说明其合法性和前提条件,并在 cancelSearch 中确保使用了原子操作。

    // 使用 DirectConnection 使得取消信号同步执行,降低取消延迟。
    // 前提:SearchWorker::cancelSearch 必须是轻量级且线程安全的(如仅设置 std::atomic<bool> 标志),
    // 绝不能包含耗时操作或阻塞等待,否则会导致主线程卡顿或死锁。
    connect(this, &GenericSearchEngine::requestCancel,
            m_worker, &SearchWorker::cancelSearch, Qt::DirectConnection);

同时,确保 SearchWorker::cancelSearch 的实现如下:

// 在 SearchWorker.h 中
std::atomic<bool> m_cancelFlag{false};

// 在 SearchWorker.cpp 中
void SearchWorker::cancelSearch() {
    m_cancelFlag.store(true, std::memory_order_release); // 线程安全,无需加锁
}

方案二:回归Qt默认的 Qt::AutoConnection(最安全的做法)

如果 cancelSearch 内部逻辑较复杂,或者你不确定其线程安全性,强烈建议去掉 Qt::DirectConnection,让Qt自动处理跨线程调用。虽然会有微秒级的延迟,但保证了绝对的线程安全和UI流畅度。

    // 恢复默认连接,保证 cancelSearch 在工作线程中执行,避免跨线程数据竞争和死锁
    connect(this, &GenericSearchEngine::requestCancel,
            m_worker, &SearchWorker::cancelSearch);

总结:这段代码的修改是一个典型的**“用安全性换性能”**的尝试。在进行此类修改时,必须严格审查 SearchWorker::cancelSearch 的实现,确保其无锁、无阻塞、无资源释放,否则建议退回到默认的队列连接。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@Johnson-zs
Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot
Copy link
Copy Markdown

deepin-bot Bot commented May 22, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit c987458 into linuxdeepin:master May 22, 2026
21 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