update templates to remove possible leak

src/core/ObjectFactoryTemplates.hpp

@@ -40,7 +40,7 @@ class GridObjectHolder: public CoreObject {
     ~GridObjectHolder()
     {
         for (auto& so : objArray) {
-            if (so.getParent()) {
+            if ((so.getParent() != nullptr) && (so.getParent()->getID() > 0)) {
                 so.getParent()->remove(&so);
             }
         }
@@ -87,29 +87,40 @@ class ObjectPrepper {
     ObjectPrepper(count_t objCount, CoreObject* example) { prepObjects(objCount, example); }
     void prepObjects(count_t objCount, CoreObject* example)
     {
+        if ((objCount == 0) || (example == nullptr)) {
+            useBlock = false;
+            return;
+        }
+
         auto root = example->getRoot();
+        if (root == nullptr) {
+            useBlock = false;
+            return;
+        }
+
         useBlock = true;
-        if ((obptr) && (root != nullptr)) {
-            if (obptr->getParent() != root) {
-                root->add(obptr.get());
-            }
+        if ((obptr) && (obptr->getParent() != root)) {
+            root->add(obptr.get());
         }
-        if (remaining() < objCount) {
-            if ((obptr) && (obptr->remaining() > 0)) {
-                targetprepped = objCount - obptr->remaining();
-            } else {
-                obptr = makeOwningPtr<GridObjectHolder<Ntype>>(targetprepped);
-                if (root != nullptr) {
-                    if (!obptr) {
-                        root->log(root, PrintLevel::WARNING, "unable to create container object");
-                        useBlock = false;
-                    } else {
-                        root->add(obptr.get());
-                    }
-                } else {
-                    useBlock = false;
-                }
-            }
+
+        const auto availableObjects = remaining();
+        if (availableObjects >= objCount) {
+            return;
+        }
+
+        const auto objectDeficit = objCount - availableObjects;
+        if ((obptr) && (obptr->remaining() > 0)) {
+            targetprepped = objectDeficit;
+            return;
+        }
+
+        targetprepped = 0;
+        obptr = makeOwningPtr<GridObjectHolder<Ntype>>(objectDeficit);
+        if (!obptr) {
+            root->log(root, PrintLevel::WARNING, "unable to create container object");
+            useBlock = false;
+        } else {
+            root->add(obptr.get());
         }
     }
     Ntype* getNewObject(std::string_view objName = {})

src/optimization/optObjectFactory.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include "core/CoreOwningPtr.hpp"
 #include "gridOptObjects.h"
 #include <functional>
 #include <map>
@@ -139,7 +140,14 @@ class OptObjectFactory: public OptFactory {
 
   private:
     bool mUseBlock = false;
-    GridOptObjectHolder<Ntype, gdType>* mObjectHolder = nullptr;
+    CoreOwningPtr<GridOptObjectHolder<Ntype, gdType>> mObjectHolder;
+
+    void attachHolderToRoot(CoreObject* root)
+    {
+        if ((root != nullptr) && mObjectHolder && (mObjectHolder->getParent() != root)) {
+            root->add(mObjectHolder.get());
+        }
+    }
 
   public:
     OptObjectFactory(std::string_view component,
@@ -232,9 +240,26 @@ class OptObjectFactory: public OptFactory {
 
     virtual void prepObjects(count_t count, CoreObject* obj) override
     {
+        if ((count == 0) || (obj == nullptr)) {
+            mUseBlock = false;
+            return;
+        }
+
         auto root = obj->getRoot();
-        mObjectHolder = new GridOptObjectHolder<Ntype, gdType>(count);
-        root->add(mObjectHolder);
+        if (root == nullptr) {
+            mUseBlock = false;
+            mObjectHolder = nullptr;
+            return;
+        }
+
+        if (mObjectHolder && (mObjectHolder->remaining() >= count)) {
+            attachHolderToRoot(root);
+            mUseBlock = true;
+            return;
+        }
+
+        mObjectHolder = makeOwningPtr<GridOptObjectHolder<Ntype, gdType>>(count);
+        attachHolderToRoot(root);
         mUseBlock = true;
     }
     virtual count_t remainingPrepped() const override

test/componentTests/simulationTests.cpp

@@ -5,14 +5,175 @@
  */
 
 #include "../gtestHelper.h"
+#include "core/CoreOwningPtr.hpp"
+#include "core/ObjectFactoryTemplates.hpp"
 #include "gmlc/utilities/vectorOps.hpp"
+#include "griddyn/griddyn-config.h"
+#include <algorithm>
 #include <cstdio>
 #include <gtest/gtest.h>
 #include <iostream>
 #include <set>
 #include <string>
 #include <utility>
+#include <vector>
+
+#ifdef GRIDDYN_ENABLE_OPTIMIZATION_LIBRARY
+#    include "optimization/optObjectFactory.h"
+#endif
 
 class SimulationTests: public GridDynSimulationTestFixture, public ::testing::Test {};
 
 TEST_F(SimulationTests, SimulationOrderingTests) {}
+
+namespace {
+class CountingRootObject: public griddyn::CoreObject {
+  public:
+    int addCount = 0;
+    std::vector<griddyn::CoreObject*> addedObjects;
+
+    void add(griddyn::CoreObject* obj) override
+    {
+        if (obj == nullptr) {
+            return;
+        }
+
+        ++addCount;
+        obj->addOwningReference();
+        obj->setParent(this);
+        addedObjects.push_back(obj);
+    }
+
+    void remove(griddyn::CoreObject* obj) override
+    {
+        const auto foundObject = std::find(addedObjects.begin(), addedObjects.end(), obj);
+        if (foundObject == addedObjects.end()) {
+            return;
+        }
+
+        griddyn::removeReference(*foundObject, this);
+        addedObjects.erase(foundObject);
+    }
+
+    ~CountingRootObject() override
+    {
+        for (auto* obj : addedObjects) {
+            griddyn::removeReference(obj, this);
+        }
+    }
+};
+
+class FactoryTestGridObject: public griddyn::CoreObject {
+  public:
+    FactoryTestGridObject() = default;
+    explicit FactoryTestGridObject(const std::string& objectName): CoreObject(objectName) {}
+};
+}  // namespace
+
+TEST(CoreFactoryTests, PrepObjectsIgnoresInvalidRequests)
+{
+    CountingRootObject root;
+    griddyn::TypeFactory<FactoryTestGridObject> factory("core-factory-prep-invalid-test", "object");
+
+    factory.prepObjects(0, &root);
+    EXPECT_EQ(root.addCount, 0);
+    EXPECT_EQ(factory.remainingPrepped(), 0U);
+
+    factory.prepObjects(2, nullptr);
+    EXPECT_EQ(root.addCount, 0);
+    EXPECT_EQ(factory.remainingPrepped(), 0U);
+}
+
+TEST(CoreFactoryTests, PrepObjectsCreatesNonEmptyHolderOnce)
+{
+    CountingRootObject root;
+    griddyn::TypeFactory<FactoryTestGridObject> factory("core-factory-prep-test", "object");
+
+    factory.prepObjects(3, &root);
+    EXPECT_EQ(root.addCount, 1);
+    EXPECT_EQ(factory.remainingPrepped(), 3U);
+
+    factory.prepObjects(2, &root);
+    EXPECT_EQ(root.addCount, 1);
+    EXPECT_EQ(factory.remainingPrepped(), 3U);
+
+    griddyn::CoreOwningPtr<FactoryTestGridObject> object{factory.makeTypeObject()};
+    ASSERT_TRUE(static_cast<bool>(object));
+    EXPECT_EQ(factory.remainingPrepped(), 2U);
+
+    factory.prepObjects(2, &root);
+    EXPECT_EQ(root.addCount, 1);
+    EXPECT_EQ(factory.remainingPrepped(), 2U);
+
+    factory.prepObjects(5, &root);
+    EXPECT_EQ(root.addCount, 1);
+    EXPECT_EQ(factory.remainingPrepped(), 5U);
+
+    std::vector<griddyn::CoreOwningPtr<FactoryTestGridObject>> objects;
+    objects.emplace_back(factory.makeTypeObject());
+    objects.emplace_back(factory.makeTypeObject());
+    EXPECT_EQ(root.addCount, 1);
+    EXPECT_EQ(factory.remainingPrepped(), 3U);
+
+    objects.emplace_back(factory.makeTypeObject());
+    EXPECT_EQ(root.addCount, 2);
+    EXPECT_EQ(factory.remainingPrepped(), 2U);
+}
+
+#ifdef GRIDDYN_ENABLE_OPTIMIZATION_LIBRARY
+namespace {
+class FactoryTestOptObject: public griddyn::GridOptObject {
+  public:
+    FactoryTestOptObject() = default;
+    explicit FactoryTestOptObject(griddyn::CoreObject* obj): sourceObject(obj) {}
+
+    griddyn::CoreObject* sourceObject = nullptr;
+
+    void add(griddyn::CoreObject* obj) override { sourceObject = obj; }
+};
+}  // namespace
+
+TEST(OptimizationFactoryTests, PrepObjectsReusesAttachedHolder)
+{
+    CountingRootObject root;
+    FactoryTestGridObject gridObject;
+    griddyn::OptObjectFactory<FactoryTestOptObject, FactoryTestGridObject> factory(
+        "factory-prep-test", "object");
+
+    factory.prepObjects(3, &root);
+    EXPECT_EQ(root.addCount, 1);
+    EXPECT_EQ(factory.remainingPrepped(), 3U);
+
+    factory.prepObjects(2, &root);
+    EXPECT_EQ(root.addCount, 1);
+    EXPECT_EQ(factory.remainingPrepped(), 3U);
+
+    griddyn::CoreOwningPtr<FactoryTestOptObject> optObject{factory.makeTypeObject(&gridObject)};
+    ASSERT_TRUE(static_cast<bool>(optObject));
+    EXPECT_EQ(optObject->sourceObject, &gridObject);
+    EXPECT_EQ(factory.remainingPrepped(), 2U);
+
+    factory.prepObjects(2, &root);
+    EXPECT_EQ(root.addCount, 1);
+    EXPECT_EQ(factory.remainingPrepped(), 2U);
+
+    factory.prepObjects(3, &root);
+    EXPECT_EQ(root.addCount, 2);
+    EXPECT_EQ(factory.remainingPrepped(), 3U);
+}
+
+TEST(OptimizationFactoryTests, PrepObjectsIgnoresInvalidRequests)
+{
+    CountingRootObject root;
+    griddyn::OptObjectFactory<FactoryTestOptObject, FactoryTestGridObject> factory(
+        "factory-prep-invalid-test", "object");
+
+    factory.prepObjects(0, &root);
+    EXPECT_EQ(root.addCount, 0);
+    EXPECT_EQ(factory.remainingPrepped(), 0U);
+
+    factory.prepObjects(2, nullptr);
+    EXPECT_EQ(root.addCount, 0);
+    EXPECT_EQ(factory.remainingPrepped(), 0U);
+}
+#endif