sw/inc/pagedesc.hxx | 6 +--
sw/source/core/doc/docfmt.cxx | 12 +++---
sw/source/core/layout/pagedesc.cxx | 74 ++++++++++++++++++-------------------
3 files changed, 46 insertions(+), 46 deletions(-)
New commits:
commit 9179d7051872be45471b133caf44fec423144fce
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Fri Feb 23 17:18:04 2024 +0100
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Feb 26 10:33:22 2024 +0100
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 <qui...@gmail.com>
(cherry picked from commit 5c4ae1b19c51dcd62dad8e1d3e8beb87a0311352)
diff --git a/sw/inc/pagedesc.hxx b/sw/inc/pagedesc.hxx
index 11bb347aa1fb..ddc7e659a5bb 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 17eaf5418e61..9145c67ee539 100644
--- a/sw/source/core/doc/docfmt.cxx
+++ b/sw/source/core/doc/docfmt.cxx
@@ -1556,21 +1556,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 40a7b5865766..5bc08706b80a 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))
@@ -409,30 +409,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
{
@@ -444,24 +444,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
{
@@ -476,15 +476,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
{
@@ -496,15 +496,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
{
@@ -520,15 +520,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
{
@@ -539,15 +539,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
{
