include/sfx2/viewsh.hxx | 4 ++++
sfx2/source/view/viewfrm.cxx | 3 +++
sw/inc/view.hxx | 9 ++++++---
sw/qa/uibase/uiview/uiview.cxx | 19 +++++++++++++++++++
sw/source/uibase/uiview/view.cxx | 8 +++++++-
5 files changed, 39 insertions(+), 4 deletions(-)
New commits:
commit 1f4e1b84722c4387f65614ad7f1765dcddb8aeb9
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Mon Dec 18 17:21:15 2023 +0100
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Fri Dec 22 10:10:26 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
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/160972
diff --git a/include/sfx2/viewsh.hxx b/include/sfx2/viewsh.hxx
index b805f1cf99d4..f6cbc16f5268 100644
--- a/include/sfx2/viewsh.hxx
+++ b/include/sfx2/viewsh.hxx
@@ -226,6 +226,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 e144de203958..4bfb58a724ec 100644
--- a/sfx2/source/view/viewfrm.cxx
+++ b/sfx2/source/view/viewfrm.cxx
@@ -2495,6 +2495,9 @@ bool SfxViewFrame::SwitchToViewShell_Impl
// save the view data of the old view, so it can be restored later on
(when needed)
SaveCurrentViewData_Impl( nViewId );
+ if (pOldSh)
+ 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 301ed1a92bb8..856d9db90a3e 100644
--- a/sw/inc/view.hxx
+++ b/sw/inc/view.hxx
@@ -272,6 +272,8 @@ class SW_DLLPUBLIC SwView: public SfxViewShell
bool m_bIsHighlightCharDF = false;
+ bool m_bDying = false;
+
static constexpr sal_uInt16 MAX_ZOOM_PERCENT = 600;
static constexpr sal_uInt16 MIN_ZOOM_PERCENT = 20;
@@ -352,6 +354,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& );
+ void SetFormShell( FmFormShell* pSh ) { m_pFormShell = pSh; }
+ virtual void SelectShell();
protected:
@@ -364,9 +368,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;
@@ -623,6 +624,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 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 5e59d75ba043..8ba807dedfa8 100644
--- a/sw/source/uibase/uiview/view.cxx
+++ b/sw/source/uibase/uiview/view.cxx
@@ -254,7 +254,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())
