From e120df092340c1ab45a58298a655f9416a658323 Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@esi-group.com>
Date: Tue, 4 Jun 2019 12:29:55 +0200
Subject: [PATCH] ENH: improved handling of regIOobject deletion (#1276)
- remove registration and ownership before deleting a regIOobject
from within objectRegistry to avoid possible recursion.
- regIOobject destructor now removes any registered object from
registry regardless if ownedByRegistry or not. It also removes
always removes the ownership flag to avoid possible recursion.
- the regIOobject::checkOut() now unconditionally clears file watches.
These will only be there if the object is registered (a no-op for an
unregistered object), but this additional safety is needed to manage
case where the registration has been modified elsewhere (eg, by the
objectRegistry).
---
.../db/objectRegistry/objectRegistry.C | 35 ++++----------
src/OpenFOAM/db/regIOobject/regIOobject.C | 48 +++++++++++++------
2 files changed, 43 insertions(+), 40 deletions(-)
diff --git a/src/OpenFOAM/db/objectRegistry/objectRegistry.C b/src/OpenFOAM/db/objectRegistry/objectRegistry.C
index fc3c4c8651f..fe9cdb2a9e4 100644
--- a/src/OpenFOAM/db/objectRegistry/objectRegistry.C
+++ b/src/OpenFOAM/db/objectRegistry/objectRegistry.C
@@ -333,14 +333,9 @@ bool Foam::objectRegistry::checkOut(const word& key) const
void Foam::objectRegistry::clear()
{
- // Free anything owned by the registry
- // This needs to be done in two stages:
- // - collect objects-to-be-removed
- // - actually delete objects
- // since the destructor of the regIOobject will actually delete its
- // entry from the objectRegistry which messes up the iterator.
-
- DynamicList<regIOobject*> owned;
+ // Free anything owned by the registry, but first unset both
+ // 'ownedByRegistry' and 'registered' flags to ensure that the
+ // regIOobject destructor will not affect the registry
for (iterator iter = begin(); iter != end(); ++iter)
{
@@ -348,26 +343,16 @@ void Foam::objectRegistry::clear()
if (ptr && ptr->ownedByRegistry())
{
- // TBD: may wish to have ptr->clearWatches();
if (objectRegistry::debug)
{
- Pout<< "objectRegistry::clear : " << ptr->name()
- << " watches :" << flatOutput(ptr->watchIndices()) << nl;
-
+ Pout<< "objectRegistry::clear : " << ptr->name() << nl;
}
- owned.append(ptr);
+ ptr->release(true); // Relinquish ownership and registration
+ delete ptr; // Delete also clears fileHandler watches
}
}
- for (regIOobject* objectPtr : owned)
- {
- // Make sure that the destructor of the regIOobject does a checkout
- objectPtr->release();
-
- delete objectPtr;
- }
-
HashTable<regIOobject*>::clear();
}
@@ -381,18 +366,18 @@ void Foam::objectRegistry::clearStorage()
bool Foam::objectRegistry::erase(const iterator& iter)
{
- // Free anything owned by the registry
+ // Remove from registry - see notes in objectRegistry::clear()
if (iter.found())
{
regIOobject* ptr = iter.val();
- bool ok = HashTable<regIOobject*>::erase(iter);
+ const bool ok = HashTable<regIOobject*>::erase(iter);
if (ptr && ptr->ownedByRegistry())
{
- // TBD: may wish to have ptr->clearWatches();
- delete ptr;
+ ptr->release(true); // Relinquish ownership and registration
+ delete ptr; // Delete also clears fileHandler watches
}
return ok;
diff --git a/src/OpenFOAM/db/regIOobject/regIOobject.C b/src/OpenFOAM/db/regIOobject/regIOobject.C
index a933fd799d8..b9abc11951b 100644
--- a/src/OpenFOAM/db/regIOobject/regIOobject.C
+++ b/src/OpenFOAM/db/regIOobject/regIOobject.C
@@ -156,19 +156,36 @@ Foam::regIOobject::~regIOobject()
{
if (objectRegistry::debug)
{
- Pout<< "Destroying regIOobject called " << name()
- << " of type " << type()
- << " registered " << registered_
- << " owned " << ownedByRegistry_
- << " in directory " << path()
+ Pout<< "Destroy regIOobject: " << name()
+ << " type=" << type()
+ << " registered=" << registered_
+ << " owned=" << ownedByRegistry_
+ << " directory=" << path()
<< endl;
}
- // Check out of objectRegistry if not owned by the registry
- if (!ownedByRegistry_)
- {
- checkOut();
- }
+ // Deletion of a regIOobject should remove itself from its registry
+ // (ie, checkOut), but there are different paths for destruction to occur.
+ // The complications are only when the object is ownedByRegistry.
+ //
+ // 1. The objectRegistry clear()/erase() is called (and object is
+ // 'ownedByRegistry').
+ //
+ // - Mark as unowned/unregistered prior to deletion.
+ // This ensures that this checkOut() only clears file watches and
+ // does nothing else.
+ //
+ // 2. The regIOobject is deleted directly (and also 'ownedByRegistry').
+ //
+ // - Mark as unowned (but keep as registered) prior to triggering
+ // checkOut(). By being 'unowned', the registry will not attempt a
+ // second deletion when the object name is removed from the registry.
+
+ // Revoke any registry ownership: we are already deleting
+ ownedByRegistry_ = false;
+
+ // Remove registered object from objectRegistry
+ checkOut();
}
@@ -212,15 +229,16 @@ bool Foam::regIOobject::checkIn()
bool Foam::regIOobject::checkOut()
{
+ forAllReverse(watchIndices_, i)
+ {
+ fileHandler().removeWatch(watchIndices_[i]);
+ }
+ watchIndices_.clear();
+
if (registered_)
{
registered_ = false;
- forAllReverse(watchIndices_, i)
- {
- fileHandler().removeWatch(watchIndices_[i]);
- }
- watchIndices_.clear();
return db().checkOut(*this);
}
--
GitLab