From 21bd6f193c55ddc3165ccf9bbf8e514b2d281f1a Mon Sep 17 00:00:00 2001 From: xiepengfei Date: Sat, 19 Sep 2026 00:43:06 +0800 Subject: [PATCH] fix(reader): correct null-page guard operator in render callback slots 1. Five async render callback slots used `nullptr != task.page && !existPage(task.page)` as the guard, which short-circuits to false when task.page is null, falling through to dereference the null pointer 2. Change the operator from `&&` to `||` so the guard returns early when task.page is null OR the page has been destroyed, preventing null-pointer dereference 3. Affected functions: onDocPageNormalImageTaskFinished, onDocPageSliceImageTaskFinished, onDocPageBigImageTaskFinished, onDocPageWordTaskFinished, onDocPageAnnotationTaskFinished 4. Adapt _002 series unit tests: add existPage stub returning true and set non-null placeholder page pointer so stubbed handlers are still invoked Log: Fix null-page guard operator in PageRenderThread render callback slots Influence: Prevents crash when async render callback delivers to a null page --- reader/browser/PageRenderThread.cpp | 20 ++++++++++---------- tests/browser/ut_pagerenderthread.cpp | 23 ++++++++++++++++++----- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/reader/browser/PageRenderThread.cpp b/reader/browser/PageRenderThread.cpp index ba38b29c4..906f5da8d 100644 --- a/reader/browser/PageRenderThread.cpp +++ b/reader/browser/PageRenderThread.cpp @@ -916,8 +916,8 @@ void PageRenderThread::onDocPageNormalImageTaskFinished(DocPageNormalImageTask t { // qCDebug(appLog) << "PageRenderThread::onDocPageNormalImageTaskFinished() - Starting on doc page normal image task finished"; if (DocSheet::existSheet(task.sheet)) { - if (nullptr != task.page && !BrowserPage::existPage(task.page)) - return; // 页面已析构,丢弃残留回包(task.page 非空时必须存活才可解引用) + if (nullptr == task.page || !BrowserPage::existPage(task.page)) + return; // 页面为空或已析构,丢弃残留回包 task.page->setImageObjectRects(task.imageRects, task.rect.width(), task.rect.height()); task.page->handleRenderFinished(task.pixmapId, pixmap); } @@ -928,8 +928,8 @@ void PageRenderThread::onDocPageSliceImageTaskFinished(DocPageSliceImageTask tas { // qCDebug(appLog) << "PageRenderThread::onDocPageSliceImageTaskFinished() - Starting on doc page slice image task finished"; if (DocSheet::existSheet(task.sheet)) { - if (nullptr != task.page && !BrowserPage::existPage(task.page)) - return; // 页面已析构,丢弃残留回包 + if (nullptr == task.page || !BrowserPage::existPage(task.page)) + return; // 页面为空或已析构,丢弃残留回包 task.page->handleRenderFinished(task.pixmapId, pixmap, task.slice); } // qCDebug(appLog) << "PageRenderThread::onDocPageSliceImageTaskFinished() - On doc page slice image task finished completed"; @@ -939,8 +939,8 @@ void PageRenderThread::onDocPageBigImageTaskFinished(DocPageBigImageTask task, Q { // qCDebug(appLog) << "PageRenderThread::onDocPageBigImageTaskFinished() - Starting on doc page big image task finished"; if (DocSheet::existSheet(task.sheet)) { - if (nullptr != task.page && !BrowserPage::existPage(task.page)) - return; // 页面已析构,丢弃残留回包 + if (nullptr == task.page || !BrowserPage::existPage(task.page)) + return; // 页面为空或已析构,丢弃残留回包 task.page->setImageObjectRects(task.imageRects, task.rect.width(), task.rect.height()); task.page->handleRenderFinished(task.pixmapId, pixmap); } @@ -951,8 +951,8 @@ void PageRenderThread::onDocPageWordTaskFinished(DocPageWordTask task, QListhandleWordLoaded(words); } // qCDebug(appLog) << "PageRenderThread::onDocPageWordTaskFinished() - On doc page word task finished completed"; @@ -962,8 +962,8 @@ void PageRenderThread::onDocPageAnnotationTaskFinished(DocPageAnnotationTask tas { // qCDebug(appLog) << "PageRenderThread::onDocPageAnnotationTaskFinished() - Starting on doc page annotation task finished"; if (DocSheet::existSheet(task.sheet)) { - if (nullptr != task.page && !BrowserPage::existPage(task.page)) - return; // 页面已析构,丢弃残留回包 + if (nullptr == task.page || !BrowserPage::existPage(task.page)) + return; // 页面为空或已析构,丢弃残留回包 task.page->handleAnnotationLoaded(annots); } // qCDebug(appLog) << "PageRenderThread::onDocPageAnnotationTaskFinished() - On doc page annotation task finished completed"; diff --git a/tests/browser/ut_pagerenderthread.cpp b/tests/browser/ut_pagerenderthread.cpp index c7904c299..9dd73b7cc 100644 --- a/tests/browser/ut_pagerenderthread.cpp +++ b/tests/browser/ut_pagerenderthread.cpp @@ -114,6 +114,14 @@ static bool existSheet_true_stub(DocSheet *) return true; } +// Makes BrowserPage::existPage() return true so the onDoc*Finished guard +// (nullptr == task.page || !existPage(task.page)) passes with a non-null +// placeholder page whose handler methods are already stubbed. +static bool existPage_true_stub(const BrowserPage *) +{ + return true; +} + /*********测试用例**********/ //TEST_F(TestPageRenderThread, UT_PageRenderThread_clearImageTasks_001) //{ @@ -234,12 +242,13 @@ TEST_F(TestPageRenderThread, UT_PageRenderThread_onDocPageNormalImageTaskFinishe { Stub s; s.set(ADDR(DocSheet, existSheet), existSheet_true_stub); + s.set(ADDR(BrowserPage, existPage), existPage_true_stub); s.set(ADDR(BrowserPage, handleRenderFinished), handleRenderFinished_stub); s.set(ADDR(BrowserPage, setImageObjectRects), setImageObjectRects_stub); DocPageNormalImageTask task; task.sheet = nullptr; // existSheet is stubbed to return true anyway - task.page = nullptr; // stub will be invoked, nullptr this is ignored + task.page = reinterpret_cast(0x1); // non-null placeholder; handler methods are stubbed task.pixmapId = 1; QPixmap pix; m_tester->onDocPageNormalImageTaskFinished(task, pix); @@ -252,11 +261,12 @@ TEST_F(TestPageRenderThread, UT_PageRenderThread_onDocPageSliceImageTaskFinished { Stub s; s.set(ADDR(DocSheet, existSheet), existSheet_true_stub); + s.set(ADDR(BrowserPage, existPage), existPage_true_stub); s.set(ADDR(BrowserPage, handleRenderFinished), handleRenderFinished_stub); DocPageSliceImageTask task; task.sheet = nullptr; - task.page = nullptr; + task.page = reinterpret_cast(0x1); task.pixmapId = 2; task.slice = QRect(0, 0, 10, 10); QPixmap pix; @@ -270,12 +280,13 @@ TEST_F(TestPageRenderThread, UT_PageRenderThread_onDocPageBigImageTaskFinished_0 { Stub s; s.set(ADDR(DocSheet, existSheet), existSheet_true_stub); + s.set(ADDR(BrowserPage, existPage), existPage_true_stub); s.set(ADDR(BrowserPage, handleRenderFinished), handleRenderFinished_stub); s.set(ADDR(BrowserPage, setImageObjectRects), setImageObjectRects_stub); DocPageBigImageTask task; task.sheet = nullptr; - task.page = nullptr; + task.page = reinterpret_cast(0x1); task.pixmapId = 3; QPixmap pix; m_tester->onDocPageBigImageTaskFinished(task, pix); @@ -288,11 +299,12 @@ TEST_F(TestPageRenderThread, UT_PageRenderThread_onDocPageWordTaskFinished_002) { Stub s; s.set(ADDR(DocSheet, existSheet), existSheet_true_stub); + s.set(ADDR(BrowserPage, existPage), existPage_true_stub); s.set(ADDR(BrowserPage, handleWordLoaded), handleWordLoaded_stub); DocPageWordTask task; task.sheet = nullptr; - task.page = nullptr; + task.page = reinterpret_cast(0x1); QList words; m_tester->onDocPageWordTaskFinished(task, words); EXPECT_TRUE(g_funcName == "handleWordLoaded_stub"); @@ -304,11 +316,12 @@ TEST_F(TestPageRenderThread, UT_PageRenderThread_onDocPageAnnotationTaskFinished { Stub s; s.set(ADDR(DocSheet, existSheet), existSheet_true_stub); + s.set(ADDR(BrowserPage, existPage), existPage_true_stub); s.set(ADDR(BrowserPage, handleAnnotationLoaded), handleAnnotationLoaded_stub); DocPageAnnotationTask task; task.sheet = nullptr; - task.page = nullptr; + task.page = reinterpret_cast(0x1); QList annots; m_tester->onDocPageAnnotationTaskFinished(task, annots); EXPECT_TRUE(g_funcName == "handleAnnotationLoaded_stub");