Merge pull request #13642 from wwmayer/fix_memory_leaks

Fix memory leaks

src/App/Application.cpp

@@ -84,6 +84,7 @@
 
 #include "Annotation.h"
 #include "Application.h"
+#include "CleanupProcess.h"
 #include "ComplexGeoData.h"
 #include "DocumentObjectFileIncluded.h"
 #include "DocumentObjectGroup.h"
@@ -1699,6 +1700,8 @@ void Application::destruct()
     cleanupUnits();
 #endif
 
+    CleanupProcess::callCleanup();
+
     // not initialized or double destruct!
     assert(_pcSingleton);
     delete _pcSingleton;

src/App/CMakeLists.txt

@@ -271,6 +271,7 @@ SET(FreeCADApp_CPP_SRCS
     ApplicationPy.cpp
     AutoTransaction.cpp
     Branding.cpp
+    CleanupProcess.cpp
     Color.cpp
     ColorModel.cpp
     ComplexGeoData.cpp
@@ -296,6 +297,7 @@ SET(FreeCADApp_HPP_SRCS
     Application.h
     AutoTransaction.h
     Branding.h
+    CleanupProcess.h
     Color.h
     ColorModel.h
     ComplexGeoData.h

src/App/CleanupProcess.cpp

@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+
+/***************************************************************************
+ *   Copyright (c) 2024 Werner Mayer <wmayer[at]users.sourceforge.net>     *
+ *                                                                         *
+ *   This file is part of FreeCAD.                                         *
+ *                                                                         *
+ *   FreeCAD is free software: you can redistribute it and/or modify it    *
+ *   under the terms of the GNU Lesser General Public License as           *
+ *   published by the Free Software Foundation, either version 2.1 of the  *
+ *   License, or (at your option) any later version.                       *
+ *                                                                         *
+ *   FreeCAD is distributed in the hope that it will be useful, but        *
+ *   WITHOUT ANY WARRANTY; without even the implied warranty of            *
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU      *
+ *   Lesser General Public License for more details.                       *
+ *                                                                         *
+ *   You should have received a copy of the GNU Lesser General Public      *
+ *   License along with FreeCAD. If not, see                               *
+ *   <https://www.gnu.org/licenses/>.                                      *
+ *                                                                         *
+ **************************************************************************/
+
+#include "PreCompiled.h"
+#include <list>
+
+#include "CleanupProcess.h"
+
+
+using namespace App;
+
+namespace
+{
+    static std::list<std::function<void()>> cleanup_funcs;  // NOLINT
+}
+
+void CleanupProcess::registerCleanup(const std::function<void()>& func)
+{
+    cleanup_funcs.push_back(func);
+}
+
+void CleanupProcess::callCleanup()
+{
+    for (const auto& func : cleanup_funcs) {
+        func();
+    }
+}

src/App/CleanupProcess.h

@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+
+/***************************************************************************
+ *   Copyright (c) 2024 Werner Mayer <wmayer[at]users.sourceforge.net>     *
+ *                                                                         *
+ *   This file is part of FreeCAD.                                         *
+ *                                                                         *
+ *   FreeCAD is free software: you can redistribute it and/or modify it    *
+ *   under the terms of the GNU Lesser General Public License as           *
+ *   published by the Free Software Foundation, either version 2.1 of the  *
+ *   License, or (at your option) any later version.                       *
+ *                                                                         *
+ *   FreeCAD is distributed in the hope that it will be useful, but        *
+ *   WITHOUT ANY WARRANTY; without even the implied warranty of            *
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU      *
+ *   Lesser General Public License for more details.                       *
+ *                                                                         *
+ *   You should have received a copy of the GNU Lesser General Public      *
+ *   License along with FreeCAD. If not, see                               *
+ *   <https://www.gnu.org/licenses/>.                                      *
+ *                                                                         *
+ **************************************************************************/
+
+#ifndef APP_CLEANUPPROCESS_H
+#define APP_CLEANUPPROCESS_H
+
+#include <functional>
+#include <FCGlobal.h>
+
+namespace App
+{
+
+/*!
+ * \brief The CleanupProcess class
+ */
+class AppExport CleanupProcess
+{
+public:
+    /*!
+     * \brief registerCleanup
+     * \param func
+     * This adds a callback function that will be called when the application
+     * is about to be shut down.
+     * @note A callback function is only about to free resources. Accessing
+     * stuff of the application like parameter groups should be avoided.
+     */
+    static void registerCleanup(const std::function<void()>& func);
+    /*!
+     * \brief callCleanup
+     * Calls the functions that are registered with \a registerCleanup.
+     */
+    static void callCleanup();
+};
+
+}
+
+#endif  // APP_CLEANUPPROCESS_H

src/Base/Quantity.cpp

@@ -532,6 +532,28 @@ int QuantityLexer();
 // NOLINTNEXTLINE
 #include "QuantityLexer.c"
 #endif  // DOXYGEN_SHOULD_SKIP_THIS
+
+class StringBufferCleaner
+{
+public:
+    explicit StringBufferCleaner(YY_BUFFER_STATE buffer)
+        : my_string_buffer {buffer}
+    {}
+    ~StringBufferCleaner()
+    {
+        // free the scan buffer
+        yy_delete_buffer(my_string_buffer);
+    }
+
+    StringBufferCleaner(const StringBufferCleaner&) = delete;
+    StringBufferCleaner(StringBufferCleaner&&) = delete;
+    StringBufferCleaner& operator=(const StringBufferCleaner&) = delete;
+    StringBufferCleaner& operator=(StringBufferCleaner&&) = delete;
+
+private:
+    YY_BUFFER_STATE my_string_buffer;
+};
+
 }  // namespace QuantityParser
 
 #if defined(__clang__)
@@ -545,12 +567,11 @@ Quantity Quantity::parse(const QString& string)
     // parse from buffer
     QuantityParser::YY_BUFFER_STATE my_string_buffer =
         QuantityParser::yy_scan_string(string.toUtf8().data());
+    QuantityParser::StringBufferCleaner cleaner(my_string_buffer);
     // set the global return variables
     QuantResult = Quantity(DOUBLE_MIN);
     // run the parser
     QuantityParser::yyparse();
-    // free the scan buffer
-    QuantityParser::yy_delete_buffer(my_string_buffer);
 
     return QuantResult;
 }

src/Mod/Material/App/AppMaterial.cpp

@@ -27,7 +27,10 @@
 #include <Base/Interpreter.h>
 #include <Base/PyObjectBase.h>
 
+#include <App/CleanupProcess.h>
+
 #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");

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<MaterialLibrary>& library,
                                    const std::shared_ptr<Material>& material,
                                    const QString& path,

src/Mod/Material/App/MaterialManager.h

@@ -54,6 +54,7 @@ public:
     MaterialManager();
     ~MaterialManager() override = default;
 
+    static void cleanup();
     static std::shared_ptr<Material> defaultMaterial();
     static QString defaultMaterialUUID();
 

src/Mod/Material/App/ModelLoader.cpp

@@ -235,7 +235,7 @@ void ModelLoader::addToTree(std::shared_ptr<ModelEntry> model,
     Model::ModelType type =
         (base == "Model") ? Model::ModelType_Physical : Model::ModelType_Appearance;
 
-    Model* finalModel = new Model(library, type, name, directory, uuid, description, url, doi);
+    Model finalModel(library, type, name, directory, uuid, description, url, doi);
 
     // Add inheritance list
     if (yamlModel[base]["Inherits"]) {
@@ -243,7 +243,7 @@ void ModelLoader::addToTree(std::shared_ptr<ModelEntry> model,
         for (auto it = inherits.begin(); it != inherits.end(); it++) {
             QString nodeName = QString::fromStdString((*it)["UUID"].as<std::string>());
 
-            finalModel->addInheritance(nodeName);
+            finalModel.addInheritance(nodeName);
         }
     }
 
@@ -299,11 +299,11 @@ void ModelLoader::addToTree(std::shared_ptr<ModelEntry> model,
                 property.setInheritance((*inheritances)[key]);
             }
 
-            finalModel->addProperty(property);
+            finalModel.addProperty(property);
         }
     }
 
-    (*_modelMap)[uuid] = library->addModel(*finalModel, directory);
+    (*_modelMap)[uuid] = library->addModel(finalModel, directory);
 }
 
 void ModelLoader::loadLibrary(std::shared_ptr<ModelLibrary> library)
@@ -330,16 +330,14 @@ void ModelLoader::loadLibrary(std::shared_ptr<ModelLibrary> library)
         }
     }
 
-    std::map<std::pair<QString, QString>, QString>* inheritances =
-        new std::map<std::pair<QString, QString>, QString>();
+    std::map<std::pair<QString, QString>, QString> inheritances;
     for (auto it = _modelEntryMap->begin(); it != _modelEntryMap->end(); it++) {
-        dereference(it->second, inheritances);
+        dereference(it->second, &inheritances);
     }
 
     for (auto it = _modelEntryMap->begin(); it != _modelEntryMap->end(); it++) {
-        addToTree(it->second, inheritances);
+        addToTree(it->second, &inheritances);
     }
-    // delete inheritances;
 }
 
 void ModelLoader::loadLibraries()

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();

src/Mod/Material/App/ModelManager.h

@@ -44,6 +44,7 @@ public:
     ModelManager();
     ~ModelManager() override = default;
 
+    static void cleanup();
     void refresh();
 
     std::shared_ptr<std::list<std::shared_ptr<ModelLibrary>>> getModelLibraries()