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 e6bdd44e314907535db181d5f65342c1bfa7e3da
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Mon Dec 18 17:21:15 2023 +0100
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Tue Dec 19 08:20:18 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.
Change-Id: I423ff946f8235848cc3a870bc52fcf88a721fd2b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160925
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
(cherry picked from commit 164fb25f7b2db7d833d6d0f28e49c5cac68426b3)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160935
diff --git a/include/sfx2/viewsh.hxx b/include/sfx2/viewsh.hxx
index 14d9ca94881c..c1a2bc90a19d 100644
--- a/include/sfx2/viewsh.hxx
+++ b/include/sfx2/viewsh.hxx
@@ -227,6 +227,10 @@ public:
SfxViewShell( SfxViewFrame& rFrame,
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 40ee7bd2f962..24bea7e809cd 100644
--- a/sfx2/source/view/viewfrm.cxx
+++ b/sfx2/source/view/viewfrm.cxx
@@ -2548,6 +2548,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 bf96007db6b4..8c825d2f78da 100644
--- a/sw/inc/view.hxx
+++ b/sw/inc/view.hxx
@@ -277,6 +277,8 @@ class SW_DLLPUBLIC SwView: public SfxViewShell
bool m_bIsSpotlightParaStyles = false;
bool m_bIsSpotlightCharStyles = false;
+ bool m_bDying = false;
+
static constexpr sal_uInt16 MAX_ZOOM_PERCENT = 600;
static constexpr sal_uInt16 MIN_ZOOM_PERCENT = 20;
@@ -358,6 +360,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:
@@ -370,9 +374,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;
@@ -629,6 +630,8 @@ public:
SwView(SfxViewFrame& rFrame, 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 27231d28e8d6..ceae7d644c3f 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 51c6ab24d0f6..76d6e0bbc064 100644
--- a/sw/source/uibase/uiview/view.cxx
+++ b/sw/source/uibase/uiview/view.cxx
@@ -264,7 +264,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
@@ -1200,6 +1201,11 @@ SwView::~SwView()
m_pFormatClipboard.reset();
}
+void SwView::SetDying()
+{
+ m_bDying = true;
+}
+
void SwView::afterCallbackRegistered()
{
if (!comphelper::LibreOfficeKit::isActive())
