Commit 48d654cf authored by Mark Olesen's avatar Mark Olesen
Browse files

ENH: avoid memory leaks for HashPtrTable, PtrMap insertion (issue #749)

- disallow insert() of raw pointers, since a failed insertion
  (ie, entry already existed) results in an unmanaged pointer.

  Either insert using an autoPtr, or set() with raw pointers or autoPtr.

- IOobjectList::add() now takes an autoPtr instead of an object reference

- IOobjectList::remove() now returns an autoPtr instead of a raw pointer
parent 46b76862
......@@ -426,7 +426,7 @@ Foam::multiphaseSystem::multiphaseSystem
iter(),
*phases_.lookup(iter.key().first()),
*phases_.lookup(iter.key().second())
).ptr()
)
);
}
......@@ -664,7 +664,7 @@ Foam::tmp<Foam::volVectorField> Foam::multiphaseSystem::Svm
Foam::autoPtr<Foam::multiphaseSystem::dragCoeffFields>
Foam::multiphaseSystem::dragCoeffs() const
{
autoPtr<dragCoeffFields> dragCoeffsPtr(new dragCoeffFields);
auto dragCoeffsPtr = autoPtr<dragCoeffFields>::New();
forAllConstIter(dragModelTable, dragModels_, iter)
{
......@@ -706,7 +706,7 @@ Foam::multiphaseSystem::dragCoeffs() const
}
}
dragCoeffsPtr().insert(iter.key(), Kptr);
dragCoeffsPtr().set(iter.key(), Kptr);
}
return dragCoeffsPtr;
......
......@@ -70,7 +70,7 @@ HeatAndMassTransferPhaseSystem
// Initially assume no mass transfer
dmdt_.insert
dmdt_.set
(
pair,
new volScalarField
......@@ -88,7 +88,7 @@ HeatAndMassTransferPhaseSystem
)
);
dmdtExplicit_.insert
dmdtExplicit_.set
(
pair,
new volScalarField
......@@ -107,7 +107,7 @@ HeatAndMassTransferPhaseSystem
volScalarField H1(heatTransferModels_[pair][pair.first()]->K());
volScalarField H2(heatTransferModels_[pair][pair.second()]->K());
Tf_.insert
Tf_.set
(
pair,
new volScalarField
......@@ -257,18 +257,12 @@ template<class BasePhaseSystem>
Foam::autoPtr<Foam::phaseSystem::heatTransferTable>
Foam::HeatAndMassTransferPhaseSystem<BasePhaseSystem>::heatTransfer() const
{
autoPtr<phaseSystem::heatTransferTable> eqnsPtr
(
new phaseSystem::heatTransferTable()
);
auto eqnsPtr = autoPtr<phaseSystem::heatTransferTable>::New();
auto& eqns = *eqnsPtr;
phaseSystem::heatTransferTable& eqns = eqnsPtr();
forAll(this->phaseModels_, phasei)
for (const phaseModel& phase : this->phaseModels_)
{
const phaseModel& phase = this->phaseModels_[phasei];
eqns.insert
eqns.set
(
phase.name(),
new fvScalarMatrix(phase.thermo().he(), dimEnergy/dimTime)
......
......@@ -107,18 +107,12 @@ template<class BasePhaseSystem>
Foam::autoPtr<Foam::phaseSystem::heatTransferTable>
Foam::HeatTransferPhaseSystem<BasePhaseSystem>::heatTransfer() const
{
autoPtr<phaseSystem::heatTransferTable> eqnsPtr
(
new phaseSystem::heatTransferTable()
);
auto eqnsPtr = autoPtr<phaseSystem::heatTransferTable>::New();
auto& eqns = *eqnsPtr;
phaseSystem::heatTransferTable& eqns = eqnsPtr();
forAll(this->phaseModels_, phasei)
for (const phaseModel& phase : this->phaseModels_)
{
const phaseModel& phase = this->phaseModels_[phasei];
eqns.insert
eqns.set
(
phase.name(),
new fvScalarMatrix(phase.thermo().he(), dimEnergy/dimTime)
......
......@@ -62,22 +62,16 @@ Foam::InterfaceCompositionPhaseChangePhaseSystem<BasePhaseSystem>::
massTransfer() const
{
// Create a mass transfer matrix for each species of each phase
autoPtr<phaseSystem::massTransferTable> eqnsPtr
(
new phaseSystem::massTransferTable()
);
auto eqnsPtr = autoPtr<phaseSystem::massTransferTable>::New();
auto& eqns = *eqnsPtr;
phaseSystem::massTransferTable& eqns = eqnsPtr();
forAll(this->phaseModels_, phasei)
for (const phaseModel& phase : this->phaseModels_)
{
const phaseModel& phase = this->phaseModels_[phasei];
const PtrList<volScalarField>& Yi = phase.Y();
forAll(Yi, i)
{
eqns.insert
eqns.set
(
Yi[i].name(),
new fvScalarMatrix(Yi[i], dimMass/dimTime)
......
......@@ -87,7 +87,7 @@ MomentumTransferPhaseSystem
const phasePair& pair =
*(this->phasePairs_[dragModelIter.key()]);
Kds_.insert
Kds_.set
(
pair,
new volScalarField
......@@ -103,7 +103,7 @@ MomentumTransferPhaseSystem
const phasePair& pair =
*(this->phasePairs_[virtualMassModelIter.key()]);
Vms_.insert
Vms_.set
(
pair,
new volScalarField
......@@ -373,18 +373,12 @@ Foam::autoPtr<Foam::phaseSystem::momentumTransferTable>
Foam::MomentumTransferPhaseSystem<BasePhaseSystem>::momentumTransfer() const
{
// Create a momentum transfer matrix for each phase
autoPtr<phaseSystem::momentumTransferTable> eqnsPtr
(
new phaseSystem::momentumTransferTable()
);
auto eqnsPtr = autoPtr<phaseSystem::momentumTransferTable>::New();
auto& eqns = *eqnsPtr;
phaseSystem::momentumTransferTable& eqns = eqnsPtr();
forAll(this->phaseModels_, phasei)
for (const phaseModel& phase : this->phaseModels_)
{
const phaseModel& phase = this->phaseModels_[phasei];
eqns.insert
eqns.set
(
phase.name(),
new fvVectorMatrix(phase.U(), dimMass*dimVelocity/dimTime)
......@@ -492,11 +486,8 @@ template<class BasePhaseSystem>
Foam::autoPtr<Foam::PtrList<Foam::volVectorField>>
Foam::MomentumTransferPhaseSystem<BasePhaseSystem>::Fs() const
{
autoPtr<PtrList<volVectorField>> tFs
(
new PtrList<volVectorField>(this->phases().size())
);
PtrList<volVectorField>& Fs = tFs();
auto tFs = autoPtr<PtrList<volVectorField>>::New(this->phases().size());
auto& Fs = *tFs;
// Add the lift force
forAllConstIters(liftModels_, modelIter)
......@@ -570,11 +561,9 @@ Foam::MomentumTransferPhaseSystem<BasePhaseSystem>::phiDs
const PtrList<volScalarField>& rAUs
) const
{
autoPtr<PtrList<surfaceScalarField>> tphiDs
(
new PtrList<surfaceScalarField>(this->phases().size())
);
PtrList<surfaceScalarField>& phiDs = tphiDs();
auto tphiDs =
autoPtr<PtrList<surfaceScalarField>>::New(this->phases().size());
auto& phiDs = *tphiDs;
// Add the turbulent dispersion force
forAllConstIters(turbulentDispersionModels_, turbulentDispersionModelIter)
......
......@@ -52,7 +52,7 @@ ThermalPhaseChangePhaseSystem
}
// Initially assume no mass transfer
iDmdt_.insert
iDmdt_.set
(
pair,
new volScalarField
......@@ -186,22 +186,16 @@ Foam::autoPtr<Foam::phaseSystem::massTransferTable>
Foam::ThermalPhaseChangePhaseSystem<BasePhaseSystem>::massTransfer() const
{
// Create a mass transfer matrix for each species of each phase
autoPtr<phaseSystem::massTransferTable> eqnsPtr
(
new phaseSystem::massTransferTable()
);
auto eqnsPtr = autoPtr<phaseSystem::massTransferTable>::New();
auto& eqns = *eqnsPtr;
phaseSystem::massTransferTable& eqns = eqnsPtr();
forAll(this->phaseModels_, phasei)
for (const phaseModel& phase : this->phaseModels_)
{
const phaseModel& phase = this->phaseModels_[phasei];
const PtrList<volScalarField>& Yi = phase.Y();
forAll(Yi, i)
{
eqns.insert
eqns.set
(
Yi[i].name(),
new fvScalarMatrix(Yi[i], dimMass/dimTime)
......
Test-hashPtrTable.C
Test-HashPtrTable.C
EXE = $(FOAM_USER_APPBIN)/Test-hashPtrTable
EXE = $(FOAM_USER_APPBIN)/Test-HashPtrTable
......@@ -3,7 +3,7 @@
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2011 OpenFOAM Foundation
\\/ M anipulation | Copyright (C) 2017 OpenCFD Ltd.
\\/ M anipulation | Copyright (C) 2017-2018 OpenCFD Ltd.
-------------------------------------------------------------------------------
License
This file is part of OpenFOAM.
......@@ -27,6 +27,7 @@ Description
\*---------------------------------------------------------------------------*/
#include <iostream>
#include "autoPtr.H"
#include "HashPtrTable.H"
using namespace Foam;
......@@ -42,7 +43,7 @@ void printTable(const HashPtrTable<T>& table)
Info<< iter.key() << " = ";
if (ptr)
{
Info<< *ptr;
Info<< *ptr << " (" << long(ptr) << ")";
}
else
{
......@@ -77,11 +78,11 @@ void printTable(const HashPtrTable<T>& table)
int main()
{
HashPtrTable<double> myTable;
myTable.insert("abc", new double(42.1));
myTable.insert("def", nullptr);
myTable.insert("pi", new double(3.14159));
myTable.insert("natlog", new double(2.718282));
myTable.insert("sqrt2", new double(1.414214));
myTable.set("abc", new double(42.1));
myTable.set("def", nullptr);
myTable.set("pi", new double(3.14159));
myTable.set("natlog", new double(2.718282));
myTable.insert("sqrt2", autoPtr<double>::New(1.414214));
// Info<< myTable << endl;
printTable(myTable);
......@@ -105,6 +106,25 @@ int main()
printTable(myTable);
HashPtrTable<double> moved(std::move(copy));
Info<< nl << "test movable" << nl;
Info<<"input:" << nl;
printTable(copy);
Info<<"output:" << nl;
printTable(moved);
HashPtrTable<double> other;
Info<<"move assign" << nl;
other = std::move(moved);
printTable(other);
Info<<"old" << nl;
printTable(moved);
return 0;
}
......
Test-PtrMap.C
EXE = $(FOAM_USER_APPBIN)/Test-PtrMap
/*---------------------------------------------------------------------------*\
========= |
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2018 OpenCFD Ltd.
\\/ M anipulation |
-------------------------------------------------------------------------------
License
This file is part of OpenFOAM.
OpenFOAM is free software: you can redistribute it and/or modify it
under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
OpenFOAM 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 General Public License
for more details.
You should have received a copy of the GNU General Public License
along with OpenFOAM. If not, see <http://www.gnu.org/licenses/>.
Description
\*---------------------------------------------------------------------------*/
#include <iostream>
#include "PtrMap.H"
using namespace Foam;
template<class T>
void printTable(const PtrMap<T>& table)
{
Info<< table.size() << nl << "(" << nl;
forAllConstIters(table, iter)
{
const T* ptr = iter.object();
Info<< iter.key() << " = ";
if (ptr)
{
Info<< *ptr << " (" << long(ptr) << ")";
}
else
{
Info<< "nullptr";
}
Info<< nl;
}
Info<< ")" << endl;
// Values only, with for-range
Info<< "values (";
for (auto val : table)
{
Info<< ' ';
if (val)
{
Info<< *val;
}
else
{
Info<< "nullptr";
}
}
Info<< " )" << nl;
}
// * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
// Main program:
int main()
{
PtrMap<double> myTable;
myTable.set(1, new double(42.1));
myTable.set(2, nullptr);
myTable.set(3, new double(3.14159));
myTable.set(4, new double(2.718282));
myTable.set(4, new double(1.414214));
// Info<< myTable << endl;
printTable(myTable);
PtrMap<double> copy(myTable);
// Info<< copy << endl;
printTable(copy);
Info<< copy << endl;
Info<<"\nerase some existing and non-existing entries" << nl;
auto iter = myTable.find(3);
myTable.erase(iter);
iter = myTable.find(1000); // unknown key
myTable.erase(iter);
myTable.erase(1);
iter = myTable.find(100000); // unknown key
printTable(myTable);
PtrMap<double> moved(std::move(copy));
Info<< nl << "test movable" << nl;
Info<<"input:" << nl;
printTable(copy);
Info<<"output:" << nl;
printTable(moved);
PtrMap<double> other;
Info<<"move assign" << nl;
other = std::move(moved);
printTable(other);
Info<<"old" << nl;
printTable(moved);
return 0;
}
// ************************************************************************* //
......@@ -813,28 +813,33 @@ int main(int argc, char *argv[])
// Read all fields in time and constant directories
IOobjectList objects(mesh, runTime.timeName());
IOobjectList timeObjects(IOobjectList(mesh, mesh.facesInstance()));
forAllConstIter(IOobjectList, timeObjects, iter)
{
if
(
iter()->headerClassName() == volScalarField::typeName
|| iter()->headerClassName() == volVectorField::typeName
|| iter()->headerClassName() == volSphericalTensorField::typeName
|| iter()->headerClassName() == volTensorField::typeName
|| iter()->headerClassName() == volSymmTensorField::typeName
|| iter()->headerClassName() == surfaceScalarField::typeName
|| iter()->headerClassName() == surfaceVectorField::typeName
|| iter()->headerClassName()
== surfaceSphericalTensorField::typeName
|| iter()->headerClassName() == surfaceSymmTensorField::typeName
|| iter()->headerClassName() == surfaceTensorField::typeName
)
IOobjectList timeObjects(mesh, mesh.facesInstance());
// Transfer specific types
forAllIters(timeObjects, iter)
{
objects.add(*iter());
autoPtr<IOobject> objPtr(timeObjects.remove(iter));
const auto& obj = *objPtr;
if
(
obj.headerClassName() == volScalarField::typeName
|| obj.headerClassName() == volVectorField::typeName
|| obj.headerClassName() == volSphericalTensorField::typeName
|| obj.headerClassName() == volTensorField::typeName
|| obj.headerClassName() == volSymmTensorField::typeName
|| obj.headerClassName() == surfaceScalarField::typeName
|| obj.headerClassName() == surfaceVectorField::typeName
|| obj.headerClassName() == surfaceSphericalTensorField::typeName
|| obj.headerClassName() == surfaceSymmTensorField::typeName
|| obj.headerClassName() == surfaceTensorField::typeName
)
{
objects.add(objPtr);
}
}
}
// Read vol fields and subset.
wordList scalarNames(objects.names(volScalarField::typeName));
......
Subproject commit 5828d4510816948b034aa9afdf0b7b05659a9f59
Subproject commit d31e13e12b4d9b727b28d39bc34086f2cb75acfa
......@@ -3,7 +3,7 @@
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2011-2016 OpenFOAM Foundation
\\/ M anipulation | Copyright (C) 2017 OpenCFD Ltd.
\\/ M anipulation | Copyright (C) 2017-2018 OpenCFD Ltd.
-------------------------------------------------------------------------------
License
This file is part of OpenFOAM.
......@@ -28,20 +28,6 @@ License
// * * * * * * * * * * * * * * * * Constructors * * * * * * * * * * * * * * //
template<class T, class Key, class Hash>
Foam::HashPtrTable<T, Key, Hash>::HashPtrTable()
:
parent_type()
{}
template<class T, class Key, class Hash>
Foam::HashPtrTable<T, Key, Hash>::HashPtrTable(const label size)
:
parent_type(size)
{}
template<class T, class Key, class Hash>
Foam::HashPtrTable<T, Key, Hash>::HashPtrTable
(
......@@ -55,16 +41,26 @@ Foam::HashPtrTable<T, Key, Hash>::HashPtrTable
const T* ptr = iter.object();
if (ptr)
{
this->insert(iter.key(), new T(*ptr));
this->set(iter.key(), new T(*ptr));
}
else
{
this->insert(iter.key(), nullptr);
this->set(iter.key(), nullptr);
}
}
}
template<class T, class Key, class Hash>
Foam::HashPtrTable<T, Key, Hash>::HashPtrTable
(
HashPtrTable<T, Key, Hash>&& ht
)
:
parent_type(std::move(ht))
{}
// * * * * * * * * * * * * * * * * Destructor * * * * * * * * * * * * * * * //
template<class T, class Key, class Hash>
......@@ -77,13 +73,13 @@ Foam::HashPtrTable<T, Key, Hash>::~HashPtrTable()
// * * * * * * * * * * * * * * * Member Functions * * * * * * * * * * * * * //
template<class T, class Key, class Hash>
T* Foam::HashPtrTable<T, Key, Hash>::remove(iterator& iter)
Foam::autoPtr<T> Foam::HashPtrTable<T, Key, Hash>::remove(iterator& iter)
{
if (iter.found())
{
T* ptr = iter.object();
autoPtr<T> aptr(iter.object());
this->parent_type::erase(iter);
return ptr;
return aptr;
}
return nullptr;
......@@ -140,11 +136,10 @@ void Foam::HashPtrTable<T, Key, Hash>::operator=
const HashPtrTable<T, Key, Hash>& rhs
)
{