From 451c55b41dc9352ee90a157b7549abb8d672215a Mon Sep 17 00:00:00 2001 From: wmayer Date: Thu, 25 Apr 2024 15:01:21 +0200 Subject: [PATCH] Material: Fix several memory leaks caused by cyclic references The class MaterialLibrary has a shared pointer to its Material objects and the class Material has a shared pointer to the MaterialLibrary. The class MaterialManager owns all MaterialLibrary and Material objects in static containers. To resolve the cyclic references the method cleanup() has been added. The class ModelLibrary has a shared pointer to its Model objects and the class Model has a shared pointer to the ModelLibrary. The class ModelManager owns all ModelLibrary and Model objects in static containers. To resolve the cyclic references the method cleanup() has been added. The Materials module registers a function to App::CleanupProcess that calls the above cleanup() methods. NOTE: This registration is only done in debug mode mainly to satisfy memory checkers --- src/Mod/Material/App/AppMaterial.cpp | 9 +++++++++ src/Mod/Material/App/MaterialManager.cpp | 15 +++++++++++++++ src/Mod/Material/App/MaterialManager.h | 1 + src/Mod/Material/App/ModelManager.cpp | 15 +++++++++++++++ src/Mod/Material/App/ModelManager.h | 1 + 5 files changed, 41 insertions(+) diff --git a/src/Mod/Material/App/AppMaterial.cpp b/src/Mod/Material/App/AppMaterial.cpp index 6b4b137c9c..99ec73f60f 100644 --- a/src/Mod/Material/App/AppMaterial.cpp +++ b/src/Mod/Material/App/AppMaterial.cpp @@ -27,7 +27,10 @@ #include #include +#include + #include "MaterialFilterPy.h" +#include "MaterialLoader.h" #include "MaterialManagerPy.h" #include "MaterialPy.h" #include "ModelManagerPy.h" @@ -61,6 +64,12 @@ PyObject* initModule() PyMOD_INIT_FUNC(Materials) { +#ifdef FC_DEBUG + App::CleanupProcess::registerCleanup([](){ + Materials::MaterialManager::cleanup(); + Materials::ModelManager::cleanup(); + }); +#endif PyObject* module = Materials::initModule(); Base::Console().Log("Loading Material module... done\n"); diff --git a/src/Mod/Material/App/MaterialManager.cpp b/src/Mod/Material/App/MaterialManager.cpp index afb0c9024b..74ccb970ec 100644 --- a/src/Mod/Material/App/MaterialManager.cpp +++ b/src/Mod/Material/App/MaterialManager.cpp @@ -77,6 +77,21 @@ void MaterialManager::initLibraries() } } +void MaterialManager::cleanup() +{ + if (_libraryList) { + _libraryList->clear(); + } + + if (_materialMap) { + for (auto& it : *_materialMap) { + // This is needed to resolve cyclic dependencies + it.second->setLibrary(nullptr); + } + _materialMap->clear(); + } +} + void MaterialManager::saveMaterial(const std::shared_ptr& library, const std::shared_ptr& material, const QString& path, diff --git a/src/Mod/Material/App/MaterialManager.h b/src/Mod/Material/App/MaterialManager.h index e8c1c5b13f..34e762789d 100644 --- a/src/Mod/Material/App/MaterialManager.h +++ b/src/Mod/Material/App/MaterialManager.h @@ -54,6 +54,7 @@ public: MaterialManager(); ~MaterialManager() override = default; + static void cleanup(); static std::shared_ptr defaultMaterial(); static QString defaultMaterialUUID(); diff --git a/src/Mod/Material/App/ModelManager.cpp b/src/Mod/Material/App/ModelManager.cpp index 23be586253..dad296fb36 100644 --- a/src/Mod/Material/App/ModelManager.cpp +++ b/src/Mod/Material/App/ModelManager.cpp @@ -72,6 +72,21 @@ bool ModelManager::isModel(const QString& file) return false; } +void ModelManager::cleanup() +{ + if (_libraryList) { + _libraryList->clear(); + } + + if (_modelMap) { + for (auto& it : *_modelMap) { + // This is needed to resolve cyclic dependencies + it.second->setLibrary(nullptr); + } + _modelMap->clear(); + } +} + void ModelManager::refresh() { _modelMap->clear(); diff --git a/src/Mod/Material/App/ModelManager.h b/src/Mod/Material/App/ModelManager.h index 9ded3b3c78..15b433930d 100644 --- a/src/Mod/Material/App/ModelManager.h +++ b/src/Mod/Material/App/ModelManager.h @@ -44,6 +44,7 @@ public: ModelManager(); ~ModelManager() override = default; + static void cleanup(); void refresh(); std::shared_ptr>> getModelLibraries()