From 01488c62bdc5a01f2d24abf9a95a866299f54a11 Mon Sep 17 00:00:00 2001 From: Jan Holesovsky Date: Tue, 9 Jun 2026 12:42:16 +0200 Subject: [PATCH] coda-m sw: Fix crash on closing a document with the fill-bitmap link tracker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Change-Id: I6fba231dde0b6b41f3af9c83208c7a2d5c7f0767 Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/4131 Tested-by: Jenkins CPCI Reviewed-by: Caolán McNamara (cherry picked from commit 96558fba2bc16d2245af1776a44c238bd60f4480) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208582 Reviewed-by: Xisco Fauli Tested-by: Xisco Fauli --- sw/qa/core/doc/doc.cxx | 13 +++++++++++++ .../doc/DocumentLinksAdministrationManager.cxx | 2 +- sw/source/core/doc/FillBitmapLinkTracker.cxx | 18 +++++++----------- sw/source/core/inc/FillBitmapLinkTracker.hxx | 6 +++--- 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/sw/qa/core/doc/doc.cxx b/sw/qa/core/doc/doc.cxx index e36a2b590b57..1f54466a6fa9 100644 --- a/sw/qa/core/doc/doc.cxx +++ b/sw/qa/core/doc/doc.cxx @@ -1359,6 +1359,19 @@ CPPUNIT_TEST_FIXTURE(SwCoreDocTest, testTableAutoFormats) getProperty(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: */ diff --git a/sw/source/core/doc/DocumentLinksAdministrationManager.cxx b/sw/source/core/doc/DocumentLinksAdministrationManager.cxx index 4fc46a5437aa..bdc42b3a0505 100644 --- a/sw/source/core/doc/DocumentLinksAdministrationManager.cxx +++ b/sw/source/core/doc/DocumentLinksAdministrationManager.cxx @@ -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 ) { } diff --git a/sw/source/core/doc/FillBitmapLinkTracker.cxx b/sw/source/core/doc/FillBitmapLinkTracker.cxx index a68913e9f948..92d239bde985 100644 --- a/sw/source/core/doc/FillBitmapLinkTracker.cxx +++ b/sw/source/core/doc/FillBitmapLinkTracker.cxx @@ -9,8 +9,6 @@ #include -#include -#include #include #include @@ -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 void FillBitmapLinkTracker::onURLChangedImpl(std::map>& 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 xLink(new SwHostFillBitmapLink(*this, rHost, rLinkMgr)); - rLinkMgr.InsertFileLink(*xLink, sfx2::SvBaseLinkObjectType::ClientGraphic, rNewURL); + tools::SvRef xLink(new SwHostFillBitmapLink(*this, rHost, m_rLinkMgr)); + m_rLinkMgr.InsertFileLink(*xLink, sfx2::SvBaseLinkObjectType::ClientGraphic, rNewURL); rMap.emplace(&rHost, std::move(xLink)); } diff --git a/sw/source/core/inc/FillBitmapLinkTracker.hxx b/sw/source/core/inc/FillBitmapLinkTracker.hxx index af56aa0f6bf3..7ad94e45f485 100644 --- a/sw/source/core/inc/FillBitmapLinkTracker.hxx +++ b/sw/source/core/inc/FillBitmapLinkTracker.hxx @@ -15,11 +15,11 @@ #include 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>& rMap, Host& rHost, std::u16string_view rNewURL); - SwDoc& m_rDoc; + sfx2::LinkManager& m_rLinkMgr; std::map> m_aLinks; std::map> m_aNodeLinks; };