tdf#147731 sw: fix memory leak in SwDoc::CopyPageDesc()
Commit 963de9feb37105560fde14b44d992e47f341bb5b "sw: fix issue with
copying stashed frame format" fixed the actual bug here, but introduced
a new memory leak.
This causes an assert in CppunitTest_uiwriter3:
cppunittester: svl/source/items/itempool.cxx:779: void SfxItemPool::Remove(const SfxPoolItem&): Assertion `rItem.GetRefCount() && "RefCount == 0, Remove impossible"' failed.
The assert happens only when this is backported to the libreoffice-7-6
branch, because commit ab7c81f55621d7b0d1468c63305163016dd78837 "ITEM:
Get away from classic 'poolable' Item flag" removed the assert.
The problem is that a SwFormatFrameSize inside a footer SwFrameFormat is
leaked 4 times, because 4 SwFrameFormats are leaked; the leak is that
SwDoc::CopyPageDesc() creates a new pNewFormat, passed it to
StashFrameFormat(), which copies it but doesn't free it.
There is also a usage of std::shared_ptr here that is very questionable;
SwFrameFormat should never be shared between different SwPageDesc.
(regression from commit b802ab694a8a7357d4657f3e11b571144fa7c7bf)
Change-Id: I44133bc5e6789a51ce064f1aa5ea8b325224365b
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163854
Tested-by: Jenkins
Reviewed-by: Tomaž Vajngerl <quikee@gmail.com>
(cherry picked from commit 5c4ae1b19c51dcd62dad8e1d3e8beb87a0311352)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/163846
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
diff --git a/sw/inc/pagedesc.hxx b/sw/inc/pagedesc.hxx
index 11bb347..ddc7e65 100644
--- a/sw/inc/pagedesc.hxx
+++ b/sw/inc/pagedesc.hxx
@@ -151,9 +151,9 @@ class SW_DLLPUBLIC SwPageDesc final
struct StashedPageDesc
{
std::shared_ptr<SwFrameFormat> m_pStashedFirst;
std::shared_ptr<SwFrameFormat> m_pStashedLeft;
std::shared_ptr<SwFrameFormat> m_pStashedFirstLeft;
std::optional<SwFrameFormat> m_oStashedFirst;
std::optional<SwFrameFormat> m_oStashedLeft;
std::optional<SwFrameFormat> m_oStashedFirstLeft;
};
mutable StashedPageDesc m_aStashedHeader;
diff --git a/sw/source/core/doc/docfmt.cxx b/sw/source/core/doc/docfmt.cxx
index d6c943d..1c696be 100644
--- a/sw/source/core/doc/docfmt.cxx
+++ b/sw/source/core/doc/docfmt.cxx
@@ -1553,21 +1553,21 @@ void SwDoc::CopyPageDesc( const SwPageDesc& rSrcDesc, SwPageDesc& rDstDesc,
{
if (pStashedFormatSrc->GetDoc() != this)
{
SwFrameFormat* pNewFormat = new SwFrameFormat(GetAttrPool(), "CopyDesc", GetDfltFrameFormat());
SwFrameFormat newFormat(GetAttrPool(), "CopyDesc", GetDfltFrameFormat());
SfxItemSet aAttrSet(pStashedFormatSrc->GetAttrSet());
aAttrSet.ClearItem(RES_HEADER);
aAttrSet.ClearItem(RES_FOOTER);
pNewFormat->DelDiffs( aAttrSet );
pNewFormat->SetFormatAttr( aAttrSet );
newFormat.DelDiffs(aAttrSet);
newFormat.SetFormatAttr(aAttrSet);
if (bHeader)
CopyHeader(*pStashedFormatSrc, *pNewFormat);
CopyHeader(*pStashedFormatSrc, newFormat);
else
CopyFooter(*pStashedFormatSrc, *pNewFormat);
CopyFooter(*pStashedFormatSrc, newFormat);
rDstDesc.StashFrameFormat(*pNewFormat, bHeader, bLeft, bFirst);
rDstDesc.StashFrameFormat(newFormat, bHeader, bLeft, bFirst);
}
else
{
diff --git a/sw/source/core/layout/pagedesc.cxx b/sw/source/core/layout/pagedesc.cxx
index 2b78823..f9679bb 100644
--- a/sw/source/core/layout/pagedesc.cxx
+++ b/sw/source/core/layout/pagedesc.cxx
@@ -83,13 +83,13 @@ SwPageDesc::SwPageDesc( const SwPageDesc &rCpy )
, m_FootnoteInfo( rCpy.GetFootnoteInfo() )
, m_pdList( nullptr )
{
m_aStashedHeader.m_pStashedFirst = rCpy.m_aStashedHeader.m_pStashedFirst;
m_aStashedHeader.m_pStashedLeft = rCpy.m_aStashedHeader.m_pStashedLeft;
m_aStashedHeader.m_pStashedFirstLeft = rCpy.m_aStashedHeader.m_pStashedFirstLeft;
m_aStashedHeader.m_oStashedFirst = rCpy.m_aStashedHeader.m_oStashedFirst;
m_aStashedHeader.m_oStashedLeft = rCpy.m_aStashedHeader.m_oStashedLeft;
m_aStashedHeader.m_oStashedFirstLeft = rCpy.m_aStashedHeader.m_oStashedFirstLeft;
m_aStashedFooter.m_pStashedFirst = rCpy.m_aStashedFooter.m_pStashedFirst;
m_aStashedFooter.m_pStashedLeft = rCpy.m_aStashedFooter.m_pStashedLeft;
m_aStashedFooter.m_pStashedFirstLeft = rCpy.m_aStashedFooter.m_pStashedFirstLeft;
m_aStashedFooter.m_oStashedFirst = rCpy.m_aStashedFooter.m_oStashedFirst;
m_aStashedFooter.m_oStashedLeft = rCpy.m_aStashedFooter.m_oStashedLeft;
m_aStashedFooter.m_oStashedFirstLeft = rCpy.m_aStashedFooter.m_oStashedFirstLeft;
if (rCpy.m_pTextFormatColl && rCpy.m_aDepends.IsListeningTo(rCpy.m_pTextFormatColl))
{
@@ -110,13 +110,13 @@ SwPageDesc & SwPageDesc::operator = (const SwPageDesc & rSrc)
m_FirstMaster = rSrc.m_FirstMaster;
m_FirstLeft = rSrc.m_FirstLeft;
m_aStashedHeader.m_pStashedFirst = rSrc.m_aStashedHeader.m_pStashedFirst;
m_aStashedHeader.m_pStashedLeft = rSrc.m_aStashedHeader.m_pStashedLeft;
m_aStashedHeader.m_pStashedFirstLeft = rSrc.m_aStashedHeader.m_pStashedFirstLeft;
m_aStashedHeader.m_oStashedFirst = rSrc.m_aStashedHeader.m_oStashedFirst;
m_aStashedHeader.m_oStashedLeft = rSrc.m_aStashedHeader.m_oStashedLeft;
m_aStashedHeader.m_oStashedFirstLeft = rSrc.m_aStashedHeader.m_oStashedFirstLeft;
m_aStashedFooter.m_pStashedFirst = rSrc.m_aStashedFooter.m_pStashedFirst;
m_aStashedFooter.m_pStashedLeft = rSrc.m_aStashedFooter.m_pStashedLeft;
m_aStashedFooter.m_pStashedFirstLeft = rSrc.m_aStashedFooter.m_pStashedFirstLeft;
m_aStashedFooter.m_oStashedFirst = rSrc.m_aStashedFooter.m_oStashedFirst;
m_aStashedFooter.m_oStashedLeft = rSrc.m_aStashedFooter.m_oStashedLeft;
m_aStashedFooter.m_oStashedFirstLeft = rSrc.m_aStashedFooter.m_oStashedFirstLeft;
m_aDepends.EndListeningAll();
if (rSrc.m_pTextFormatColl && rSrc.m_aDepends.IsListeningTo(rSrc.m_pTextFormatColl))
@@ -416,30 +416,30 @@ void SwPageDesc::ChgFirstShare( bool bNew )
void SwPageDesc::StashFrameFormat(const SwFrameFormat& rFormat, bool bHeader, bool bLeft, bool bFirst)
{
assert(rFormat.GetRegisteredIn());
std::shared_ptr<SwFrameFormat>* pFormat = nullptr;
std::optional<SwFrameFormat>* pFormat = nullptr;
if (bHeader)
{
if (bLeft && !bFirst)
pFormat = &m_aStashedHeader.m_pStashedLeft;
pFormat = &m_aStashedHeader.m_oStashedLeft;
else if (!bLeft && bFirst)
pFormat = &m_aStashedHeader.m_pStashedFirst;
pFormat = &m_aStashedHeader.m_oStashedFirst;
else if (bLeft && bFirst)
pFormat = &m_aStashedHeader.m_pStashedFirstLeft;
pFormat = &m_aStashedHeader.m_oStashedFirstLeft;
}
else
{
if (bLeft && !bFirst)
pFormat = &m_aStashedFooter.m_pStashedLeft;
pFormat = &m_aStashedFooter.m_oStashedLeft;
else if (!bLeft && bFirst)
pFormat = &m_aStashedFooter.m_pStashedFirst;
pFormat = &m_aStashedFooter.m_oStashedFirst;
else if (bLeft && bFirst)
pFormat = &m_aStashedFooter.m_pStashedFirstLeft;
pFormat = &m_aStashedFooter.m_oStashedFirstLeft;
}
if (pFormat)
{
*pFormat = std::make_shared<SwFrameFormat>(rFormat);
pFormat->emplace(rFormat);
}
else
{
@@ -451,24 +451,24 @@ void SwPageDesc::StashFrameFormat(const SwFrameFormat& rFormat, bool bHeader, bo
const SwFrameFormat* SwPageDesc::GetStashedFrameFormat(bool bHeader, bool bLeft, bool bFirst) const
{
std::shared_ptr<SwFrameFormat>* pFormat = nullptr;
std::optional<SwFrameFormat>* pFormat = nullptr;
if (bLeft && !bFirst)
{
pFormat = bHeader ? &m_aStashedHeader.m_pStashedLeft : &m_aStashedFooter.m_pStashedLeft;
pFormat = bHeader ? &m_aStashedHeader.m_oStashedLeft : &m_aStashedFooter.m_oStashedLeft;
}
else if (!bLeft && bFirst)
{
pFormat = bHeader ? &m_aStashedHeader.m_pStashedFirst : &m_aStashedFooter.m_pStashedFirst;
pFormat = bHeader ? &m_aStashedHeader.m_oStashedFirst : &m_aStashedFooter.m_oStashedFirst;
}
else if (bLeft && bFirst)
{
pFormat = bHeader ? &m_aStashedHeader.m_pStashedFirstLeft : &m_aStashedFooter.m_pStashedFirstLeft;
pFormat = bHeader ? &m_aStashedHeader.m_oStashedFirstLeft : &m_aStashedFooter.m_oStashedFirstLeft;
}
if (pFormat)
{
return pFormat->get();
return pFormat->has_value() ? &**pFormat : nullptr;
}
else
{
@@ -483,15 +483,15 @@ bool SwPageDesc::HasStashedFormat(bool bHeader, bool bLeft, bool bFirst) const
{
if (bLeft && !bFirst)
{
return m_aStashedHeader.m_pStashedLeft != nullptr;
return m_aStashedHeader.m_oStashedLeft.has_value();
}
else if (!bLeft && bFirst)
{
return m_aStashedHeader.m_pStashedFirst != nullptr;
return m_aStashedHeader.m_oStashedFirst.has_value();
}
else if (bLeft && bFirst)
{
return m_aStashedHeader.m_pStashedFirstLeft != nullptr;
return m_aStashedHeader.m_oStashedFirstLeft.has_value();
}
else
{
@@ -503,15 +503,15 @@ bool SwPageDesc::HasStashedFormat(bool bHeader, bool bLeft, bool bFirst) const
{
if (bLeft && !bFirst)
{
return m_aStashedFooter.m_pStashedLeft != nullptr;
return m_aStashedFooter.m_oStashedLeft.has_value();
}
else if (!bLeft && bFirst)
{
return m_aStashedFooter.m_pStashedFirst != nullptr;
return m_aStashedFooter.m_oStashedFirst.has_value();
}
else if (bLeft && bFirst)
{
return m_aStashedFooter.m_pStashedFirstLeft != nullptr;
return m_aStashedFooter.m_oStashedFirstLeft.has_value();
}
else
{
@@ -527,15 +527,15 @@ void SwPageDesc::RemoveStashedFormat(bool bHeader, bool bLeft, bool bFirst)
{
if (bLeft && !bFirst)
{
m_aStashedHeader.m_pStashedLeft.reset();
m_aStashedHeader.m_oStashedLeft.reset();
}
else if (!bLeft && bFirst)
{
m_aStashedHeader.m_pStashedFirst.reset();
m_aStashedHeader.m_oStashedFirst.reset();
}
else if (bLeft && bFirst)
{
m_aStashedHeader.m_pStashedFirstLeft.reset();
m_aStashedHeader.m_oStashedFirstLeft.reset();
}
else
{
@@ -546,15 +546,15 @@ void SwPageDesc::RemoveStashedFormat(bool bHeader, bool bLeft, bool bFirst)
{
if (bLeft && !bFirst)
{
m_aStashedFooter.m_pStashedLeft.reset();
m_aStashedFooter.m_oStashedLeft.reset();
}
else if (!bLeft && bFirst)
{
m_aStashedFooter.m_pStashedFirst.reset();
m_aStashedFooter.m_oStashedFirst.reset();
}
else if (bLeft && bFirst)
{
m_aStashedFooter.m_pStashedFirstLeft.reset();
m_aStashedFooter.m_oStashedFirstLeft.reset();
}
else
{
