From 032a97311b694787e9a97ce471c3b379d5612d07 Mon Sep 17 00:00:00 2001 From: Krzysztof Kowalczyk Date: Wed, 20 May 2026 17:30:04 +0200 Subject: [PATCH 1/2] fix crash when user closes window during slow initial document load The EXCEPTION_ACCESS_VIOLATION in SetSidebarVisibility+0x48 (ReplaceDocumentInCurrentTab) was a use-after-free on MainWindow*. For command-line (or startup) loads we use the sync path: CreateAndShowMainWindow (window visible) -> CreateControllerForEngineOrFile (can take tens of seconds for large/complex EPUBs on slow CPUs) -> LoadDocumentFinish / Replace... / SetSidebarVisibility If the user closes the (frozen) window, WM_CLOSE is queued. During tab insertion, RedrawAll, Show/UpdateWindow etc. in the finish path the close can be processed synchronously via DestroyWindow, which calls DeleteMainWindow and tears down the win/tabs/DisplayModel while the load code is still on the stack. Added IsMainWindowValid(win) && !win->isBeingClosed guards at the entry to ReplaceDocumentInCurrentTab, after tab creation, before/after the Replace call, and immediately before SetSidebarVisibility. This follows the existing pattern in LoadModelIntoTab and the async load path (LoadDocumentAsyncFinish). The original report was crash 86.227.233.173 (EPUB, 327 pages, ~38s load, Win10 on Pentium N4200). No ailog.txt to commit. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/SumatraPDF.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/SumatraPDF.cpp b/src/SumatraPDF.cpp index 7eb7c35e043e..ebd5b9bf40f0 100644 --- a/src/SumatraPDF.cpp +++ b/src/SumatraPDF.cpp @@ -1378,6 +1378,9 @@ static void ReplaceDocumentInCurrentTab(LoadArgs* args, DocController* ctrl, Fil if (!win) { return; } + if (!IsMainWindowValid(win) || win->isBeingClosed) { + return; + } WindowTab* tab = win->CurrentTab(); ReportIf(!tab); @@ -1581,6 +1584,9 @@ static void ReplaceDocumentInCurrentTab(LoadArgs* args, DocController* ctrl, Fil // cf. https://code.google.com/p/sumatrapdf/issues/detail?id=2541 // ReportIf(win->IsDocLoaded() && args->showWin && win->canvasRc.IsEmpty() && !win->AsChm()); + if (!IsMainWindowValid(win) || win->isBeingClosed) { + return; + } SetSidebarVisibility(win, showToc, gGlobalPrefs->showFavorites); // restore scroll state after the canvas size has been restored if ((args->showWin || ss.page != 1) && win->AsFixed()) { @@ -2201,6 +2207,10 @@ MainWindow* LoadDocumentFinish(LoadArgs* args) { tab->SetFilePath(fullPath); win->currentTabTemp = AddTabToWindow(win, tab); + if (!IsMainWindowValid(win) || win->isBeingClosed) { + return nullptr; + } + // logf("LoadDocument: !forceReuse, created win->CurrentTab() at 0x%p\n", win->CurrentTab()); } else { win->CurrentTab()->SetFilePath(fullPath); @@ -2214,9 +2224,16 @@ MainWindow* LoadDocumentFinish(LoadArgs* args) { args->placeWindow = !SettingsUseTabs(); bool lazyLoad = args->lazyLoad; if (!lazyLoad) { + if (!IsMainWindowValid(win) || win->isBeingClosed) { + return nullptr; + } ReplaceDocumentInCurrentTab(args, args->ctrl, nullptr); } + if (!IsMainWindowValid(win) || win->isBeingClosed) { + return nullptr; + } + if (gPluginMode) { // hide the menu for embedded documents opened from the plugin SetMenu(win->hwndFrame, nullptr); From fcf280b97a26c51bc994b2e487eba6298165987e Mon Sep 17 00:00:00 2001 From: Krzysztof Kowalczyk Date: Wed, 20 May 2026 18:13:28 +0200 Subject: [PATCH 2/2] Fix crash with dangling tab->ctrl during rapid document loading Defer the SaveSettings() call that happens at the end of LoadDocumentFinish() by posting it via uitask::Post. This prevents SaveSettings() (which walks all tabs and calls tab->ctrl->GetDisplayState) from running while other documents are still being loaded or closed via DDE command storm. As a defensive measure, snapshot the list of tabs before iterating in SaveSettings(), since the call can now happen asynchronously relative to tab open/close operations. This addresses crashes such as: https://www.sumatrapdfreader.org/crash/2026-05-20/89bcb2fbc000001.txt The AV occurred in UpdateTabFileDisplayStateForTab (line 567) with a garbage tab->ctrl pointer (e.g. FFFFFFFF000000A1) under heavy load combined with 3rd-party window hooks (MarkAny Document SAFER + GridWndHookLM64.dll). Also ran clang-format. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/AppSettings.cpp | 5 ++++- src/SumatraPDF.cpp | 11 ++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/AppSettings.cpp b/src/AppSettings.cpp index a29441f64b44..6dc33273d25e 100644 --- a/src/AppSettings.cpp +++ b/src/AppSettings.cpp @@ -414,8 +414,11 @@ bool SaveSettings() { } logf("SaveSettings\n"); // update display states for all tabs + // we snapshot the list because SaveSettings() can be called re-entrantly + // (e.g. from LoadDocumentFinish while other documents are still loading/closing) for (MainWindow* win : gWindows) { - for (WindowTab* tab : win->Tabs()) { + Vec tabs = win->Tabs(); + for (WindowTab* tab : tabs) { UpdateTabFileDisplayStateForTab(tab); } } diff --git a/src/SumatraPDF.cpp b/src/SumatraPDF.cpp index ebd5b9bf40f0..ebba4c1cf40d 100644 --- a/src/SumatraPDF.cpp +++ b/src/SumatraPDF.cpp @@ -2178,6 +2178,14 @@ void ShowErrorLoadingNotification(MainWindow* win, const char* path, bool noSave extern void SetTabState(WindowTab* tab, TabState* state); +// we call this via uitask::Post so that SaveSettings() doesn't run +// synchronously in the middle of LoadDocumentFinish while other +// documents may still be loading or tabs are being closed +// (fixes crashes with dangling tab->ctrl under rapid DDE opens + hooks) +static void SaveSettingsVoid() { + SaveSettings(); +} + MainWindow* LoadDocumentFinish(LoadArgs* args) { MainWindow* win = args->win; const char* fullPath = args->FilePath(); @@ -2283,7 +2291,8 @@ MainWindow* LoadDocumentFinish(LoadArgs* args) { // TODO: this seems to save the state of file that we just opened // add a way to skip saving currTab? if (!args->noSavePrefs) { - SaveSettings(); + auto fn = MkFunc0Void(SaveSettingsVoid); + uitask::Post(fn, "SaveSettingsAfterDocLoad"); } }