ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

coda-m sw: Fix crash on closing a document with the fill-bitmap link tracker

Browse Source 
Create 2 new Writer documents in coda-m (so that they are both as tabs)
and close the first one.  Collabora Office then crashes in
~FillBitmapLinkTracker:

  #0 0x000000040c0705b4 in sw::FillBitmapLinkTracker::~FillBitmapLinkTracker at engine/sw/source/core/doc/FillBitmapLinkTracker.cxx:108
  #1 0x000000040c070b14 in sw::FillBitmapLinkTracker::~FillBitmapLinkTracker at engine/sw/source/core/doc/FillBitmapLinkTracker.cxx:107
  [... unique_ptr delete ...]
  #6 0x000000040c06eb50 in sw::DocumentLinksAdministrationManager::~DocumentLinksAdministrationManager at engine/sw/source/core/doc/DocumentLinksAdministrationManager.cxx:436
  #7 0x000000040c06ebbc in sw::DocumentLinksAdministrationManager::~DocumentLinksAdministrationManager at engine/sw/source/core/doc/DocumentLinksAdministrationManager.cxx:435
  [... unique_ptr delete ...]
  #12 0x000000040bfd0b50 in SwDoc::~SwDoc at engine/sw/source/core/doc/docnew.cxx:632

The tracker reached its LinkManager through the document:

  m_rDoc.getIDocumentLinksAdministration().GetLinkManager()

SwDoc::getIDocumentLinksAdministration() returns
*m_pDocumentLinksAdministrationManager.  On close the tracker is destroyed
while that manager is being destroyed: ~SwDoc destroys its
m_pDocumentLinksAdministrationManager member, and a unique_ptr clears its
stored pointer before it runs the deleter, so for the whole duration of
~DocumentLinksAdministrationManager (and the ~FillBitmapLinkTracker it
triggers) getIDocumentLinksAdministration() dereferences a null pointer.
The LinkManager it wanted is in fact still alive, it is just unreachable
through the half-destroyed document.

To fix this, hold the LinkManager directly.

Also includes a unit test, and I've tested it crashes without the change.
If it passes on other platforms, the problem might be platform-specific,
because the macOS libc++'s unique_ptr clears its stored pointer before
running the deleter, so the document was already unreachable during the
tracker's destruction.

This was introduced in d8a09afac610 (register writer XATTR_FILLBITMAP URLs
as SvBaseLinks via SwFormat and SwContentNode).

Signed-off-by: Jan Holesovsky <kendy@collabora.com>
Change-Id: I6fba231dde0b6b41f3af9c83208c7a2d5c7f0767
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/4131
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
(cherry picked from commit 96558fba2bc16d2245af1776a44c238bd60f4480)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208582
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Xisco Fauli <xiscofauli@libreoffice.org>
This commit is contained in:
Jan Holesovsky
2026-07-25 01:48:51 +02:00
committed by Xisco Fauli
parent dbd695c8c6
commit 01488c62bd
4 changed files with 24 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sw/qa/core/doc/doc.cxx
+13
View File
@@ -1359,6 +1359,19 @@ CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testTableAutoFormats)
getProperty<awt::FontSlant>(getParagraphOfText(1, xCell->getText()), u"CharPosture"_ustr));
}
CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testCloseDocWithFillBitmapLinkTracker)
{
// Closing a freshly created document must not crash while its fill-bitmap
// link tracker is torn down.
createSwDoc();
CPPUNIT_ASSERT(mxComponent.is());
// Without the accompanying fix in place, this crashed in
// ~FillBitmapLinkTracker as the document was destroyed.
mxComponent->dispose();
mxComponent.clear();
}
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
sw/source/core/doc/DocumentLinksAdministrationManager.cxx
+1 -1
View File
@@ -168,7 +168,7 @@ DocumentLinksAdministrationManager::DocumentLinksAdministrationManager( SwDoc& i
: mbVisibleLinks(true)
, mbLinksUpdated( false ) //#i38810#
, m_pLinkMgr( new sfx2::LinkManager(nullptr) )
, m_pFillBitmapLinkTracker( new FillBitmapLinkTracker(i_rSwdoc) )
, m_pFillBitmapLinkTracker( new FillBitmapLinkTracker(*m_pLinkMgr) )
, m_rDoc( i_rSwdoc )
{
}
sw/source/core/doc/FillBitmapLinkTracker.cxx
+7 -11
View File
@@ -9,8 +9,6 @@
#include <FillBitmapLinkTracker.hxx>
#include <IDocumentLinksAdministration.hxx>
#include <doc.hxx>
#include <format.hxx>
#include <node.hxx>
@@ -99,19 +97,18 @@ public:
namespace sw
{
FillBitmapLinkTracker::FillBitmapLinkTracker(SwDoc& rDoc)
: m_rDoc(rDoc)
FillBitmapLinkTracker::FillBitmapLinkTracker(sfx2::LinkManager& rLinkMgr)
: m_rLinkMgr(rLinkMgr)
{
}
FillBitmapLinkTracker::~FillBitmapLinkTracker()
{
sfx2::LinkManager& rLinkMgr = m_rDoc.getIDocumentLinksAdministration().GetLinkManager();
for (const auto& rEntry : m_aLinks)
rLinkMgr.Remove(rEntry.second.get());
m_rLinkMgr.Remove(rEntry.second.get());
m_aLinks.clear();
for (const auto& rEntry : m_aNodeLinks)
rLinkMgr.Remove(rEntry.second.get());
m_rLinkMgr.Remove(rEntry.second.get());
m_aNodeLinks.clear();
}
@@ -119,17 +116,16 @@ template <typename Host>
void FillBitmapLinkTracker::onURLChangedImpl(std::map<Host*, tools::SvRef<sfx2::SvBaseLink>>& rMap,
Host& rHost, std::u16string_view rNewURL)
{
sfx2::LinkManager& rLinkMgr = m_rDoc.getIDocumentLinksAdministration().GetLinkManager();
auto it = rMap.find(&rHost);
if (it != rMap.end())
{
rLinkMgr.Remove(it->second.get());
m_rLinkMgr.Remove(it->second.get());
rMap.erase(it);
}
if (rNewURL.empty())
return;
tools::SvRef<sfx2::SvBaseLink> xLink(new SwHostFillBitmapLink<Host>(*this, rHost, rLinkMgr));
rLinkMgr.InsertFileLink(*xLink, sfx2::SvBaseLinkObjectType::ClientGraphic, rNewURL);
tools::SvRef<sfx2::SvBaseLink> xLink(new SwHostFillBitmapLink<Host>(*this, rHost, m_rLinkMgr));
m_rLinkMgr.InsertFileLink(*xLink, sfx2::SvBaseLinkObjectType::ClientGraphic, rNewURL);
rMap.emplace(&rHost, std::move(xLink));
}
sw/source/core/inc/FillBitmapLinkTracker.hxx
+3 -3
View File
@@ -15,11 +15,11 @@
#include <string_view>
class SwContentNode;
class SwDoc;
class SwFormat;
namespace sfx2
{
class LinkManager;
class SvBaseLink;
}
@@ -35,7 +35,7 @@ namespace sw
class FillBitmapLinkTracker final
{
public:
explicit FillBitmapLinkTracker(SwDoc& rDoc);
explicit FillBitmapLinkTracker(sfx2::LinkManager& rLinkMgr);
~FillBitmapLinkTracker();
// Called by SwFormat::SwClientNotify / SwContentNode::SwClientNotify
@@ -51,7 +51,7 @@ private:
void onURLChangedImpl(std::map<Host*, tools::SvRef<sfx2::SvBaseLink>>& rMap, Host& rHost,
std::u16string_view rNewURL);
SwDoc& m_rDoc;
sfx2::LinkManager& m_rLinkMgr;
std::map<SwFormat*, tools::SvRef<sfx2::SvBaseLink>> m_aLinks;
std::map<SwContentNode*, tools::SvRef<sfx2::SvBaseLink>> m_aNodeLinks;
};