include/sfx2/viewsh.hxx | 4 ++++ sfx2/source/view/viewfrm.cxx | 2 ++ sw/inc/view.hxx | 9 ++++++--- sw/qa/uibase/uiview/uiview.cxx | 19 +++++++++++++++++++ sw/source/uibase/uiview/view.cxx | 8 +++++++- 5 files changed, 38 insertions(+), 4 deletions(-)
New commits: commit df15f739eeaeab4d2333973fefda32dcbad73518 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Mon Dec 18 17:21:15 2023 +0100 Commit: Caolán McNamara <caolan.mcnam...@collabora.com> CommitDate: Tue Dec 19 11:41:54 2023 +0100 tdf#158686 sw floattable: fix print preview crash Regression from commit b8521d969ab5be4fc947e467d4afe969f9d3b563 (tdf#157263 sw floattable: prefer join over split after moving fwd, 2023-09-25), enabling Options -> Writer -> Formatting Aids -> Hidden Characters, then opening the bugdoc, finally Toggle Print Preview on the toolbar resulted in a crash. We have a memory corruption here: ==11968==ERROR: AddressSanitizer: heap-use-after-free on address 0x60f0000734e0 at pc 0x7f473822d2ee bp 0x7fffdadd3660 sp 0x7fffdadd3658 READ of size 8 at 0x60f0000734e0 thread T0 #0 0x7f473822d2ed in rtl::Reference<FmXFormShell>::operator->() const /include/rtl/ref.hxx:216:9 #1 0x7f473821feeb in FmFormShell::IsActiveControl() const /svx/source/form/fmshell.cxx:1227:12 #2 0x7f46dad4d52a in SwView::SelectShell() /sw/source/uibase/uiview/view.cxx:296:40 #3 0x7f46dad496a6 in SwView::AttrChangedNotify(LinkParamNone*) /sw/source/uibase/uiview/view.cxx:572:13 ... #32 0x7f4748944cda in SfxViewFrame::SwitchToViewShell_Impl(unsigned short, bool) /sfx2/source/view/viewfrm.cxx:2552:32 #33 0x7f47488f4e3b in SfxViewFrame::ExecView_Impl(SfxRequest&) /sfx2/source/view/viewfrm.cxx:2637:29 freed by thread T0 here: #0 0x5568ff2f9a7b in operator delete(void*, unsigned long) /home/abuild/rpmbuild/BUILD/llvm-15.0.7.src/build/../projects/compiler-rt/lib/asan/asan_new_delete.cpp:164:3 #1 0x7f4738214346 in FmFormShell::~FmFormShell() /svx/source/form/fmshell.cxx:181:1 #2 0x7f4746b04b9d in SfxDispatcher::FlushImpl() /sfx2/source/control/dispatch.cxx:1412:13 #3 0x7f4746aff767 in SfxDispatcher::Flush() /sfx2/source/control/dispatch.cxx:157:26 #4 0x7f47489100dc in SfxViewFrame::PopShellAndSubShells_Impl(SfxViewShell&) /sfx2/source/view/viewfrm.cxx:1100:24 #5 0x7f47489441a3 in SfxViewFrame::SwitchToViewShell_Impl(unsigned short, bool) /sfx2/source/view/viewfrm.cxx:2538:13 #6 0x7f47488f4e3b in SfxViewFrame::ExecView_Impl(SfxRequest&) /sfx2/source/view/viewfrm.cxx:2637:29 I.e. a new SwPagePreview replaces the old SwView, but the order is that SfxViewFrame::SwitchToViewShell_Impl() starts with deleting the SfxShells of the old SwView in PopShellAndSubShells_Impl(), then it creates the new shell, finally deletes the old shell. Given that the new shell hides hidden characters and the old shell did not, this triggers a size notification for the half-deleted old shell and we crash. Seeing that SwView::AttrChangedNotify() already had code to delay the selection of an SfxShell in the old SwView, fix the problem by introducing a new flag that allows not selecting an SfxShell at all if the old view is known to be deleted in the near future anyway. An alternative would be to make sure that all relevant pointers are maintained using an SfxBroadcaster / SfxListener protocol, but after fixing some 4 of them and that's still not enough, probably it's better to handle this at a higher level. It's also a bit unclear how this "worked" in the past; looks like the old view didn't get the size change notification by accident. (cherry picked from commit 164fb25f7b2db7d833d6d0f28e49c5cac68426b3) Conflicts: sw/inc/view.hxx Change-Id: I423ff946f8235848cc3a870bc52fcf88a721fd2b Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160971 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Tested-by: Caolán McNamara <caolan.mcnam...@collabora.com> Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com> diff --git a/include/sfx2/viewsh.hxx b/include/sfx2/viewsh.hxx index 2435caa52a47..212c63a419c1 100644 --- a/include/sfx2/viewsh.hxx +++ b/include/sfx2/viewsh.hxx @@ -220,6 +220,10 @@ public: SfxViewShell( SfxViewFrame *pFrame, SfxViewShellFlags nFlags ); virtual ~SfxViewShell() override; + /// Informs the view shell that it'll be deleted before the main loop processes the next user + /// input. + virtual void SetDying() {} + SfxInPlaceClient* GetIPClient() const; SfxInPlaceClient* GetUIActiveClient() const; SfxInPlaceClient* FindIPClient( const css::uno::Reference < css::embed::XEmbeddedObject >& xObj, vcl::Window *pObjParentWin ) const; diff --git a/sfx2/source/view/viewfrm.cxx b/sfx2/source/view/viewfrm.cxx index 9703851e7beb..9f6fda81abd1 100644 --- a/sfx2/source/view/viewfrm.cxx +++ b/sfx2/source/view/viewfrm.cxx @@ -2418,6 +2418,8 @@ bool SfxViewFrame::SwitchToViewShell_Impl // save the view data of the old view, so it can be restored later on (when needed) SaveCurrentViewData_Impl( nViewId ); + pOldSh->SetDying(); + // create and load new ViewShell SfxViewShell* pNewSh = LoadViewIntoFrame_Impl( *GetObjectShell(), diff --git a/sw/inc/view.hxx b/sw/inc/view.hxx index 9e31e337206b..25297987a4c7 100644 --- a/sw/inc/view.hxx +++ b/sw/inc/view.hxx @@ -270,6 +270,8 @@ class SW_DLLPUBLIC SwView: public SfxViewShell int m_nMaxOutlineLevelShown = 10; + bool m_bDying = false; + static constexpr sal_uInt16 MAX_ZOOM_PERCENT = 600; static constexpr sal_uInt16 MIN_ZOOM_PERCENT = 20; @@ -351,6 +353,8 @@ class SW_DLLPUBLIC SwView: public SfxViewShell public: // #i123922# Needs to be called from a 2nd place now as a helper method SAL_DLLPRIVATE bool InsertGraphicDlg( SfxRequest& ); sal_Int32 m_nNaviExpandedStatus = -1; + void SetFormShell( FmFormShell* pSh ) { m_pFormShell = pSh; } + virtual void SelectShell(); protected: @@ -363,9 +367,6 @@ protected: // for SwWebView void SetShell( SfxShell* pS ) { m_pShell = pS; } - void SetFormShell( FmFormShell* pSh ) { m_pFormShell = pSh; } - - virtual void SelectShell(); virtual void Activate(bool) override; virtual void Deactivate(bool) override; @@ -622,6 +623,8 @@ public: SwView(SfxViewFrame* pFrame, SfxViewShell*); virtual ~SwView() override; + void SetDying() override; + void NotifyDBChanged(); SfxObjectShellLock CreateTmpSelectionDoc(); diff --git a/sw/qa/uibase/uiview/uiview.cxx b/sw/qa/uibase/uiview/uiview.cxx index 5a0f1db584e2..20d3c13be7da 100644 --- a/sw/qa/uibase/uiview/uiview.cxx +++ b/sw/qa/uibase/uiview/uiview.cxx @@ -294,6 +294,25 @@ CPPUNIT_TEST_FIXTURE(SwUibaseUiviewTest, testSwitchBetweenImages) CPPUNIT_ASSERT_GREATEREQUAL(1, pInterceptor->m_nDisabled); } +CPPUNIT_TEST_FIXTURE(SwUibaseUiviewTest, testPrintPreview) +{ + // Given a normal Writer view, in half-destroyed state, similar to what + // SfxViewFrame::SwitchToViewShell_Impl() does in practice: + createSwDoc(); + SwDocShell* pDocShell = getSwDocShell(); + SwView* pView = pDocShell->GetView(); + FmFormShell* pFormShell = pView->GetFormShell(); + pView->SetFormShell(reinterpret_cast<FmFormShell*>(-1)); + pView->SetDying(); + + // When selecting a shell, similar to what happens the doc size changes: + // Then make sure we don't crash: + pView->SelectShell(); + + // Restore the state and shut down. + pView->SetFormShell(pFormShell); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/uibase/uiview/view.cxx b/sw/source/uibase/uiview/view.cxx index 94d97d8cb07a..00d4ba26df31 100644 --- a/sw/source/uibase/uiview/view.cxx +++ b/sw/source/uibase/uiview/view.cxx @@ -256,7 +256,8 @@ void SwView::SelectShell() { // Attention: Maintain the SelectShell for the WebView additionally - if(m_bInDtor) + // In case of m_bDying, our SfxShells are already gone, don't try to select a shell at all. + if(m_bInDtor || m_bDying) return; // Decision if the UpdateTable has to be called @@ -1178,6 +1179,11 @@ SwView::~SwView() m_pFormatClipboard.reset(); } +void SwView::SetDying() +{ + m_bDying = true; +} + void SwView::afterCallbackRegistered() { if (!comphelper::LibreOfficeKit::isActive())