[Libreoffice-commits] core.git: include/svx svx/source

[Libreoffice Gerrit user]

 include/svx/svdmodel.hxx       |   28 ++++++++++++++++++++++++++++
 svx/source/svdraw/svdmodel.cxx |   22 +++++++++++++++++++++-
 svx/source/svdraw/svdobj.cxx   |   23 +++++++++++++++++++++++
 3 files changed, 72 insertions(+), 1 deletion(-)

New commits:
commit f0dcfe008d58053e52c51b10e51c58eae27c1f0b
Author:     Armin Le Grand <Armin.Le.Grand at cib.de>
AuthorDate: Fri Jul 27 16:04:07 2018 +0200
Commit:     Armin Le Grand <Armin.Le.Grand at cib.de>
CommitDate: Mon Jul 30 17:52:30 2018 +0200

    Added SdrObjectLifetimeWatchDog for debug mode
    
    Change-Id: I79f65b7511d400c3e7071e45c3f0b1d385bcd8d4
    Reviewed-on: https://gerrit.libreoffice.org/58197
    Tested-by: Jenkins
    Reviewed-by: Armin Le Grand <Armin.Le.Grand at cib.de>

diff --git a/include/svx/svdmodel.hxx b/include/svx/svdmodel.hxx
index ccc1323abe56..19cb27a3570b 100644
--- a/include/svx/svdmodel.hxx
+++ b/include/svx/svdmodel.hxx
@@ -49,6 +49,11 @@ class OutputDevice;
 #include <rtl/ref.hxx>
 #include <deque>
 
+#ifdef DBG_UTIL
+// SdrObjectLifetimeWatchDog
+#include <unordered_set>
+#endif
+
 #define DEGREE_CHAR u'\x00B0'   /* U+00B0 DEGREE SIGN */
 
 class SdrOutliner;
@@ -152,6 +157,29 @@ struct SdrModelImpl;
 
 class SVX_DLLPUBLIC SdrModel : public SfxBroadcaster, public virtual tools::WeakBase
 {
+private:
+#ifdef DBG_UTIL
+    // SdrObjectLifetimeWatchDog:
+    // Use maAllIncarnatedObjects to keep track of all SdrObjects incarnated using this SdrModel
+    // (what is now possible after the paradigm change that a SdrObject stays at a single SdrModel
+    // for it's whole lifetime).
+    // The two methods are exclusive, debug-only, only-accessible-by SdrObject accesses to else
+    // hidden/non-existing maAllIncarnatedObjects.
+    // SdrObject::SdrObject uses impAddIncarnatedSdrObjectToSdrModel, while SdrObject::~SdrObject
+    // uses impRemoveIncarnatedSdrObjectToSdrModel.
+    // There are two places which may trigger OSL_FAIL warnings:
+    // - impRemoveIncarnatedSdrObjectToSdrModel when the to-be-removed SdrObject is not member of SdrModel
+    // - SdrModel::~SdrModel after all SdrObjects *should* be cleaned-up.
+    // SdrModel::~SdrModel will also - for convenience - Free the non-deleted SdrObjects if there
+    // are any.
+    // Using std::unordered_set will use quasi constant access times, so this watchdog will not
+    // be expensive. Nonetheless, only use with debug code. It may be seductive to use this in
+    // product code, too, especially if it will indeed trigger - but it's intention is clearly
+    // to find/identify MemoryLeaks caused by SdrObjects
+    friend void impAddIncarnatedSdrObjectToSdrModel(const SdrObject& rSdrObject, SdrModel& rSdrModel);
+    friend void impRemoveIncarnatedSdrObjectToSdrModel(const SdrObject& rSdrObject, SdrModel& rSdrModel);
+    std::unordered_set< const SdrObject* >  maAllIncarnatedObjects;
+#endif
 protected:
     std::vector<SdrPage*> maMaPag;     // master pages
     std::vector<SdrPage*> maPages;
diff --git a/svx/source/svdraw/svdmodel.cxx b/svx/source/svdraw/svdmodel.cxx
index 2761af39bea4..a63401dc88e7 100644
--- a/svx/source/svdraw/svdmodel.cxx
+++ b/svx/source/svdraw/svdmodel.cxx
@@ -204,7 +204,12 @@ void SdrModel::ImpCtor(
 SdrModel::SdrModel(
     SfxItemPool* pPool,
     ::comphelper::IEmbeddedHelper* pPers)
-:   maMaPag(),
+:
+#ifdef DBG_UTIL
+    // SdrObjectLifetimeWatchDog:
+    maAllIncarnatedObjects(),
+#endif
+    maMaPag(),
     maPages()
 {
     ImpCtor(pPool,pPers);
@@ -228,6 +233,21 @@ SdrModel::~SdrModel()
 
     ClearModel(true);
 
+#ifdef DBG_UTIL
+    // SdrObjectLifetimeWatchDog:
+    if(!maAllIncarnatedObjects.empty())
+    {
+        SAL_WARN("svx","SdrModel::~SdrModel: Not all incarnations of SdrObjects deleted, possible memory leak (!)");
+        // copy to std::vector - calling SdrObject::Free will change maAllIncarnatedObjects
+        const std::vector< const SdrObject* > maRemainingObjects(maAllIncarnatedObjects.begin(), maAllIncarnatedObjects.end());
+        for(auto pSdrObject : maRemainingObjects)
+        {
+            SdrObject* pCandidate(const_cast<SdrObject*>(pSdrObject));
+            SdrObject::Free(pCandidate);
+        }
+    }
+#endif
+
     pLayerAdmin.reset();
 
     pTextChain.reset();
diff --git a/svx/source/svdraw/svdobj.cxx b/svx/source/svdraw/svdobj.cxx
index 0559d7516666..6cd4947da101 100644
--- a/svx/source/svdraw/svdobj.cxx
+++ b/svx/source/svdraw/svdobj.cxx
@@ -332,6 +332,20 @@ void SdrObject::SetBoundRectDirty()
     aOutRect = tools::Rectangle();
 }
 
+#ifdef DBG_UTIL
+// SdrObjectLifetimeWatchDog:
+void impAddIncarnatedSdrObjectToSdrModel(const SdrObject& rSdrObject, SdrModel& rSdrModel)
+{
+    rSdrModel.maAllIncarnatedObjects.insert(&rSdrObject);
+}
+void impRemoveIncarnatedSdrObjectToSdrModel(const SdrObject& rSdrObject, SdrModel& rSdrModel)
+{
+    if(!rSdrModel.maAllIncarnatedObjects.erase(&rSdrObject))
+    {
+        SAL_WARN("svx","SdrObject::~SdrObject: Destructed incarnation of SdrObject not member of this SdrModel (!)");
+    }
+}
+#endif
 
 SdrObject::SdrObject(SdrModel& rSdrModel)
 :   mpFillGeometryDefiningShape(nullptr)
@@ -372,6 +386,10 @@ SdrObject::SdrObject(SdrModel& rSdrModel)
     bIs3DObj=false;
     bMarkProt=false;
     bIsUnoObj=false;
+#ifdef DBG_UTIL
+    // SdrObjectLifetimeWatchDog:
+    impAddIncarnatedSdrObjectToSdrModel(*this, getSdrModelFromSdrObject());
+#endif
 }
 
 SdrObject::~SdrObject()
@@ -396,6 +414,11 @@ SdrObject::~SdrObject()
     pGrabBagItem.reset();
     mpProperties.reset();
     mpViewContact.reset();
+
+#ifdef DBG_UTIL
+    // SdrObjectLifetimeWatchDog:
+    impRemoveIncarnatedSdrObjectToSdrModel(*this, getSdrModelFromSdrObject());
+#endif
 }
 
 void SdrObject::Free( SdrObject*& _rpObject )