Skip to content

fix(reader): fix render thread quit race and adapt page-change unit test #373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lzwind merged 1 commit into from
Sep 9, 2026
Merged

fix(reader): fix render thread quit race and adapt page-change unit test #373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion reader/browser/PageRenderThread.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,9 @@ void PageRenderThread::appendTask(DocCloseTask task)

void PageRenderThread::run()
{
m_quit = false;
// 注意: 不能在这里重置 m_quit。若 destroyForever() 在线程进入 run() 前
// 已置 m_quit=true, 此处重置会吞掉退出请求,导致 wait() 永久阻塞。
// 实例创建时 m_quit 默认即为 false, 无需重复初始化。
qCDebug(appLog) << "====开始执行任务====";

while (!m_quit) {
Expand Down
4 changes: 3 additions & 1 deletion reader/browser/PageRenderThread.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
#include "Global.h"
#include "Model.h"

#include <QThread>

Check warning on line 12 in reader/browser/PageRenderThread.h

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QThread> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <QMutex>

Check warning on line 13 in reader/browser/PageRenderThread.h

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QMutex> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <atomic>

Check warning on line 14 in reader/browser/PageRenderThread.h

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <atomic> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <QStack>

Check warning on line 15 in reader/browser/PageRenderThread.h

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QStack> not found. Please note: Cppcheck does not need standard library headers to get proper results.
#include <QImage>

Check warning on line 16 in reader/browser/PageRenderThread.h

View workflow job for this annotation

GitHub Actions / cppcheck 

Include file: <QImage> not found. Please note: Cppcheck does not need standard library headers to get proper results.

class DocSheet;
class BrowserPage;
Expand Down Expand Up @@ -283,7 +284,8 @@
QMutex m_closeMutex;
QList<DocCloseTask> m_closeTasks;

bool m_quit = false;
// 原子量: 主线程 destroyForever 写、渲染线程读, 避免数据竞争
std::atomic<bool> m_quit {false};

static bool s_quitForever;

Expand Down
10 changes: 9 additions & 1 deletion tests/uiframe/ut_docsheet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Copyright (C) 2019 ~ 2020 Uniontech Software Technology Co.,Ltd.
// SPDX-FileCopyrightText: 2023 UnionTech Software Technology Co., Ltd.
// SPDX-FileCopyrightText: 2023 - 2026 UnionTech Software Technology Co., Ltd.
//
// SPDX-License-Identifier: GPL-3.0-or-later

Expand Down Expand Up @@ -1283,6 +1283,14 @@ TEST_F(TestDocSheet, UT_DocSheet_docBasicInfo_001)

TEST_F(TestDocSheet, UT_DocSheet_onBrowserPageChanged_001)
{
// onBrowserPageChanged 在浏览器不可见时会忽略页码回写（防进度污染守卫）,
// 单测环境不 show 窗口, isVisible 恒为 false, 需打桩绕过;
// 同时桩掉 sidebar 的 setCurrentPage, 避免触发缩略图渲染任务导致
// PageRenderThread 在进程退出阶段 start/wait 竞争挂死
Stub s;
s.set(ADDR(QWidget, isVisible), isVisible_stub_true);
s.set(ADDR(SheetSidebar, setCurrentPage), setCurrentPage_stub);

m_tester->m_operation.currentPage = 2;
m_tester->onBrowserPageChanged(1);
EXPECT_TRUE(m_tester->m_operation.currentPage == 1);
Expand Down
Loading