Commit 91bf6288 authored by Mark Olesen's avatar Mark Olesen

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)
parent 1dea317a
......@@ -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,29 +267,16 @@ 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)
{
if (objectRegistry::debug)
{
Pout<< "objectRegistry::checkOut(regIOobject&) : "
<< name() << " : could not find " << io.name()
<< " in registry " << name()
<< endl;
}
Pout<< "objectRegistry::checkOut(regIOobject&) : "
<< name() << " : could not find " << io.name() << " in registry"
<< 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);
......
......@@ -152,8 +152,7 @@ public:
explicit objectRegistry(const IOobject& io, const label nObjects=128);
//- Destructor, performs a checkOut() for all objects that are
//- ownedByRegistry
//- Destructor, with checkOut() for all objects that are 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);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment