ENH: avoid potential leaks with objectRegistry::erase (#1180)

- forwards to the underlying HashTable erase, but frees things owned
  by the registry as well (ie, performs a checkOut)

src/OpenFOAM/db/objectRegistry/objectRegistry.C

@@ -35,6 +35,37 @@ namespace Foam
 }
 
 
+// * * * * * * * * * * * * * * * Local Functions * * * * * * * * * * * * * * //
+
+namespace Foam
+{
+
+// Templated implementation for erase() with iterator range.
+// Prefer not to expose directly.
+template<class InputIter>
+static label eraseImpl(objectRegistry& obr, InputIter first, InputIter last)
+{
+    label changed = 0;
+
+    for
+    (
+        const label nTotal = obr.size();
+        changed < nTotal && first != last; // Terminate early
+        ++first
+    )
+    {
+        if (obr.erase(*first))
+        {
+            ++changed;
+        }
+    }
+
+    return changed;
+}
+
+} // End namespace Foam
+
+
 // * * * * * * * * * * * * * Private Member Functions  * * * * * * * * * * * //
 
 bool Foam::objectRegistry::parentNotTime() const
@@ -193,24 +224,38 @@ bool Foam::objectRegistry::checkIn(regIOobject& io) const
             << endl;
     }
 
-    return const_cast<objectRegistry&>(*this).insert(io.name(), &io);
+    objectRegistry& obr = const_cast<objectRegistry&>(*this);
+
+    bool ok = obr.insert(io.name(), &io);
+
+    if (!ok && objectRegistry::debug)
+    {
+        WarningInFunction
+            << name() << " : attempted to checkIn object with name "
+            << io.name() << " which was already checked in"
+            << endl;
+    }
+
+    return ok;
 }
 
 
 bool Foam::objectRegistry::checkOut(regIOobject& io) const
 {
-    iterator iter = const_cast<objectRegistry&>(*this).find(io.name());
+    objectRegistry& obr = const_cast<objectRegistry&>(*this);
+
+    iterator iter = obr.find(io.name());
 
     if (iter.found())
     {
         if (objectRegistry::debug)
         {
             Pout<< "objectRegistry::checkOut(regIOobject&) : "
-                << name() << " : checking out " << iter.key()
+                << name() << " : checking out " << io.name()
                 << endl;
         }
 
-        if (iter() != &io)
+        if (iter.val() != &io)
         {
             if (objectRegistry::debug)
             {
@@ -222,30 +267,17 @@ bool Foam::objectRegistry::checkOut(regIOobject& io) const
 
             return false;
         }
-        else
-        {
-            regIOobject* object = iter();
 
-            bool hasErased = const_cast<objectRegistry&>(*this).erase(iter);
+        return obr.erase(iter);
-
-            if (io.ownedByRegistry())
-            {
-                delete object;
     }
 
-            return hasErased;
+
-        }
-    }
-    else
-    {
     if (objectRegistry::debug)
     {
         Pout<< "objectRegistry::checkOut(regIOobject&) : "
-                << name() << " : could not find " << io.name()
+            << name() << " : could not find " << io.name() << " in registry"
-                << " in registry " << name()
             << endl;
     }
-    }
 
     return false;
 }
@@ -275,6 +307,46 @@ void Foam::objectRegistry::clearStorage()
 }
 
 
+bool Foam::objectRegistry::erase(const iterator& iter)
+{
+    // Free anything owned by the registry
+
+    if (iter.found())
+    {
+        regIOobject* ptr = iter.val();
+
+        bool ok = HashTable<regIOobject*>::erase(iter);
+
+        if (ptr && ptr->ownedByRegistry())
+        {
+            delete ptr;
+        }
+
+        return ok;
+    }
+
+    return false;
+}
+
+
+bool Foam::objectRegistry::erase(const word& key)
+{
+    return erase(find(key));
+}
+
+
+Foam::label Foam::objectRegistry::erase(std::initializer_list<word> keys)
+{
+     return eraseImpl(*this, keys.begin(), keys.end());
+}
+
+
+Foam::label Foam::objectRegistry::erase(const UList<word>& keys)
+{
+    return eraseImpl(*this, keys.begin(), keys.end());
+}
+
+
 void Foam::objectRegistry::rename(const word& newName)
 {
     regIOobject::rename(newName);

src/OpenFOAM/db/objectRegistry/objectRegistry.H

@@ -152,8 +152,7 @@ public:
         explicit objectRegistry(const IOobject& io, const label nObjects=128);
 
 
-    //- Destructor, performs a checkOut() for all objects that are
+    //- Destructor, with checkOut() for all objects that are ownedByRegistry
-    //- ownedByRegistry
     virtual ~objectRegistry();
 
 
@@ -433,7 +432,8 @@ public:
         //- Add a regIOobject to registry
         bool checkIn(regIOobject& io) const;
 
-        //- Remove a regIOobject from registry
+        //- Remove a regIOobject from registry and frees memory if the
+        //- object is ownedByRegistry
         bool checkOut(regIOobject& io) const;
 
         //- Clear all entries from the registry
@@ -443,6 +443,26 @@ public:
         //- Clear all entries from the registry and the table itself.
         void clearStorage();
 
+        //- Erase an entry specified by the given iterator.
+        //  Performs a checkOut() if the object was ownedByRegistry.
+        //  \return True if the entry existed and was removed
+        bool erase(const iterator& iter);
+
+        //- Erase an entry specified by the given key
+        //  Performs a checkOut() if the object was ownedByRegistry.
+        //  \return True if the entry existed and was removed
+        bool erase(const word& key);
+
+        //- Remove entries given by the listed keys
+        //  Performs a checkOut() for all objects that are ownedByRegistry.
+        //  \return The number of items removed
+        label erase(std::initializer_list<word> keys);
+
+        //- Remove entries given by the listed keys
+        //  Performs a checkOut() for all objects that are ownedByRegistry.
+        //  \return The number of items removed
+        label erase(const UList<word>& keys);
+
         //- Rename
         virtual void rename(const word& newName);