Skip to content

Commit

Permalink
Refactor vtkDebugLeaks construction.
Browse files Browse the repository at this point in the history
vtkDebugLeaks registers instance by class name, which worked well for
many years. However, now that we have more templated code, this is
becoming difficult. For instance, "template <typename T> vtkBuffer<T>"
would be identified as "vtkBuffer<T>" when registering with
vtkDebugLeaks, but deregistered with the compiler dependent
typeid(vtkBuffer<T>).name() string returned from GetClassName().

This patch moves vtkDebugLeaks registrations to the method
vtkObjectBase::InitializeObjectBase(), which must be called after the
vtkObjectBase is instantiated. This ensures that objects are
registered using the same string as when they are destroyed. In
general, a call to "new vtkSomeClass" must be followed by a call to
InitializeObjectBase on the new instance. The common ::New()
implementation macros in vtkObjectFactory will ensure that
registration happens correctly.

Two notable exceptions are vtkCommand and vtkInformationKey
subclasses. These do not require any specific handling for
vtkDebugLeaks registration.

See discussion at:

https://vtk.1045678.n5.nabble.com/Proposal-Simplify-vtkDebugLeaks-registration-td5740222.html
  • Loading branch information
David C. Lonie committed Oct 3, 2016
1 parent e0b081e commit e5c793d
Show file tree
Hide file tree
Showing 51 changed files with 192 additions and 242 deletions.
2 changes: 1 addition & 1 deletion Common/Core/Testing/Cxx/TestDataArrayIterators.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class MyArray : public vtkTypedDataArray<float>
vtkFloatArray *Data;
public:
vtkTypeMacro(MyArray, vtkTypedDataArray<float>)
static MyArray *New() { return new MyArray; }
static MyArray *New() { VTK_STANDARD_NEW_BODY(MyArray) }
void Init(vtkFloatArray *array)
{
this->Data = array;
Expand Down
14 changes: 7 additions & 7 deletions Common/Core/Testing/Cxx/TestGarbageCollector.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@
class vtkTestReferenceLoop: public vtkObject
{
public:
static vtkTestReferenceLoop* New() { return new vtkTestReferenceLoop; }
static vtkTestReferenceLoop* New()
{
vtkTestReferenceLoop *ret = new vtkTestReferenceLoop;
ret->InitializeObjectBase();
return ret;
}
vtkTypeMacro(vtkTestReferenceLoop, vtkObject);

void Register(vtkObjectBase* o) VTK_OVERRIDE { this->RegisterInternal(o, 1); }
Expand All @@ -34,17 +39,12 @@ class vtkTestReferenceLoop: public vtkObject
vtkTestReferenceLoop()
{
this->Other = new vtkTestReferenceLoop(this);
#ifdef VTK_DEBUG_LEAKS
vtkDebugLeaks::ConstructClass("vtkTestReferenceLoop");
#endif
this->Other->InitializeObjectBase();
}
vtkTestReferenceLoop(vtkTestReferenceLoop* other)
{
this->Other = other;
this->Other->Register(this);
#ifdef VTK_DEBUG_LEAKS
vtkDebugLeaks::ConstructClass("vtkTestReferenceLoop");
#endif
}
~vtkTestReferenceLoop() VTK_OVERRIDE
{
Expand Down
19 changes: 10 additions & 9 deletions Common/Core/Testing/Cxx/TestObjectFactory.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class vtkTestPoints : public vtkPoints
}

vtkTypeMacro(vtkTestPoints,vtkPoints);
static vtkTestPoints* New() { return new vtkTestPoints; }
static vtkTestPoints* New() { VTK_STANDARD_NEW_BODY(vtkTestPoints) }
vtkTestPoints() { }
private:
vtkTestPoints(const vtkTestPoints&);
Expand All @@ -50,7 +50,7 @@ class vtkTestPoints2 : public vtkPoints

// Methods from vtkObject
vtkTypeMacro(vtkTestPoints2,vtkPoints);
static vtkTestPoints2* New() { return new vtkTestPoints2; }
static vtkTestPoints2* New() { VTK_STANDARD_NEW_BODY(vtkTestPoints2) }
vtkTestPoints2() { }
private:
vtkTestPoints2(const vtkTestPoints2&);
Expand All @@ -65,7 +65,12 @@ class VTK_EXPORT TestFactory : public vtkObjectFactory
{
public:
TestFactory();
static TestFactory* New() { return new TestFactory;}
static TestFactory* New()
{
TestFactory *f = new TestFactory;
f->InitializeObjectBase();
return f;
}
const char* GetVTKSourceVersion() VTK_OVERRIDE { return VTK_SOURCE_VERSION; }
const char* GetDescription() VTK_OVERRIDE { return "A fine Test Factory"; }

Expand All @@ -74,11 +79,6 @@ class VTK_EXPORT TestFactory : public vtkObjectFactory
void operator=(const TestFactory&);
};






TestFactory::TestFactory()
{
this->RegisterOverride("vtkPoints",
Expand All @@ -97,7 +97,8 @@ void TestNewPoints(vtkPoints* v, const char* expectedClassName)
if(strcmp(v->GetClassName(), expectedClassName) != 0)
{
failed = 1;
cout << "Test Failed" << endl;
cout << "Test Failed:\nExpected classname: " << expectedClassName
<< "\nCreated classname: " << v->GetClassName() << endl;
}
}

Expand Down
42 changes: 31 additions & 11 deletions Common/Core/vtkDebugLeaks.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,38 @@

=========================================================================*/
/**
* @class vtkDebugLeaks
* @brief identify memory leaks at program termination
* @class vtkDebugLeaks
* @brief identify memory leaks at program termination
* vtkDebugLeaks is used to report memory leaks at the exit of the program. It
* uses vtkObjectBase::InitializeObjectBase() (called via vtkObjectFactory
* macros) to intercept the construction of all VTK objects. It uses the
* UnRegisterInternal method of vtkObjectBase to intercept the destruction of
* all objects.
*
* vtkDebugLeaks is used to report memory leaks at the exit of the program.
* It uses the vtkObjectFactory to intercept the construction of all VTK
* objects. It uses the UnRegister method of vtkObject to intercept the
* destruction of all objects. A table of object name to number of instances
* is kept. At the exit of the program if there are still VTK objects around
* it will print them out. To enable this class add the flag
* -DVTK_DEBUG_LEAKS to the compile line, and rebuild vtkObject and
* vtkObjectFactory.
*/
* If not using the vtkObjectFactory macros to implement New(), be sure to call
* vtkObjectBase::InitializeObjectBase() explicitly on the constructed
* instance. The rule of thumb is that wherever "new [some vtkObjectBase
* subclass]" is called, vtkObjectBase::InitializeObjectBase() must be called
* as well.
*
* There are exceptions to this:
*
* - vtkCommand subclasses traditionally do not fully participate in
* vtkDebugLeaks registration, likely because they typically do not use
* vtkTypeMacro to configure GetClassName. InitializeObjectBase should not be
* called on vtkCommand subclasses, and all such classes will be automatically
* registered with vtkDebugLeaks as "vtkCommand or subclass".
*
* - vtkInformationKey subclasses are not reference counted. They are allocated
* statically and registered automatically with a singleton "manager" instance.
* The manager ensures that all keys are cleaned up before exiting, and
* registration/deregistration with vtkDebugLeaks is bypassed.
*
* A table of object name to number of instances is kept. At the exit of the
* program if there are still VTK objects around it will print them out. To
* enable this class add the flag -DVTK_DEBUG_LEAKS to the compile line, and
* rebuild vtkObject and vtkObjectFactory.
*/

#ifndef vtkDebugLeaks_h
#define vtkDebugLeaks_h
Expand Down
13 changes: 7 additions & 6 deletions Common/Core/vtkDenseArray.txx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#ifndef vtkDenseArray_txx
#define vtkDenseArray_txx

#include "vtkObjectFactory.h"

///////////////////////////////////////////////////////////////////////////////
// vtkDenseArray::MemoryBlock

Expand Down Expand Up @@ -72,12 +74,11 @@ T* vtkDenseArray<T>::StaticMemoryBlock::GetAddress()
template<typename T>
vtkDenseArray<T>* vtkDenseArray<T>::New()
{
vtkObject* ret = vtkObjectFactory::CreateInstance(typeid(ThisT).name());
if(ret)
{
return static_cast<ThisT*>(ret);
}
return new ThisT();
// Don't use object factory macros on templated classes. It'll confuse the
// object factory.
vtkDenseArray<T> *ret = new vtkDenseArray<T>;
ret->InitializeObjectBase();
return ret;
}

template<typename T>
Expand Down
6 changes: 2 additions & 4 deletions Common/Core/vtkDynamicLoader.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,13 @@
#include "vtkDynamicLoader.h"

#include "vtkDebugLeaks.h"
#include "vtkObjectFactory.h"


//-----------------------------------------------------------------------------
vtkDynamicLoader* vtkDynamicLoader::New()
{
#ifdef VTK_DEBUG_LEAKS
vtkDebugLeaks::ConstructClass("vtkDynamicLoader");
#endif
return new vtkDynamicLoader;
VTK_STANDARD_NEW_BODY(vtkDynamicLoader)
}


Expand Down
9 changes: 1 addition & 8 deletions Common/Core/vtkIndent.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,7 @@
//------------------------------------------------------------------------------
vtkIndent* vtkIndent::New()
{
// First try to create the object from the vtkObjectFactory
vtkObject* ret = vtkObjectFactory::CreateInstance("vtkIndent");
if(ret)
{
return reinterpret_cast<vtkIndent*>(ret);
}
// If the factory was unable to create the object, then create it here.
return new vtkIndent;
return new vtkIndent; // not a VTK object, don't use object factory macros
}


Expand Down
2 changes: 1 addition & 1 deletion Common/Core/vtkInformationDoubleKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void vtkInformationDoubleKey::Set(vtkInformation* info, double value)
{
// Allocate a new value.
vtkInformationDoubleValue* v = new vtkInformationDoubleValue;
this->ConstructClass("vtkInformationDoubleValue");
v->InitializeObjectBase();
v->Value = value;
this->SetAsObjectBase(info, v);
v->Delete();
Expand Down
2 changes: 1 addition & 1 deletion Common/Core/vtkInformationDoubleVectorKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ void vtkInformationDoubleVectorKey::Set(vtkInformation* info,
}
vtkInformationDoubleVectorValue* v =
new vtkInformationDoubleVectorValue;
this->ConstructClass("vtkInformationDoubleVectorValue");
v->InitializeObjectBase();
v->Value.insert(v->Value.begin(), value, value+length);
this->SetAsObjectBase(info, v);
v->Delete();
Expand Down
2 changes: 1 addition & 1 deletion Common/Core/vtkInformationIdTypeKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void vtkInformationIdTypeKey::Set(vtkInformation* info, vtkIdType value)
{
// Allocate a new value.
vtkInformationIdTypeValue* v = new vtkInformationIdTypeValue;
this->ConstructClass("vtkInformationIdTypeValue");
v->InitializeObjectBase();
v->Value = value;
this->SetAsObjectBase(info, v);
v->Delete();
Expand Down
2 changes: 1 addition & 1 deletion Common/Core/vtkInformationIntegerKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ void vtkInformationIntegerKey::Set(vtkInformation* info, int value)
{
// Allocate a new value.
vtkInformationIntegerValue* v = new vtkInformationIntegerValue;
this->ConstructClass("vtkInformationIntegerValue");
v->InitializeObjectBase();
v->Value = value;
this->SetAsObjectBase(info, v);
v->Delete();
Expand Down
2 changes: 1 addition & 1 deletion Common/Core/vtkInformationIntegerPointerKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ void vtkInformationIntegerPointerKey::Set(vtkInformation* info, int* value,
// Allocate a new value.
vtkInformationIntegerPointerValue* v =
new vtkInformationIntegerPointerValue;
this->ConstructClass("vtkInformationIntegerPointerValue");
v->InitializeObjectBase();
v->Value = value;
v->Length = length;
this->SetAsObjectBase(info, v);
Expand Down
2 changes: 1 addition & 1 deletion Common/Core/vtkInformationIntegerVectorKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ void vtkInformationIntegerVectorKey::Set(vtkInformation* info,
// Allocate a new value.
vtkInformationIntegerVectorValue* v =
new vtkInformationIntegerVectorValue;
this->ConstructClass("vtkInformationIntegerVectorValue");
v->InitializeObjectBase();
v->Value.insert(v->Value.begin(), value, value+length);
this->SetAsObjectBase(info, v);
v->Delete();
Expand Down
7 changes: 0 additions & 7 deletions Common/Core/vtkInformationKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,6 @@ void vtkInformationKey::ReportAsObjectBase(vtkInformation* info,
}

//----------------------------------------------------------------------------
#ifdef VTK_DEBUG_LEAKS
void vtkInformationKey::ConstructClass(const char* name)
{
vtkDebugLeaks::ConstructClass(name);
}
#else
void vtkInformationKey::ConstructClass(const char*)
{
}
#endif
5 changes: 2 additions & 3 deletions Common/Core/vtkInformationKeyVectorKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,8 @@ void vtkInformationKeyVectorKey::Set(vtkInformation* info,
{
if(value)
{
vtkInformationKeyVectorValue* v =
new vtkInformationKeyVectorValue;
this->ConstructClass("vtkInformationKeyVectorValue");
vtkInformationKeyVectorValue* v = new vtkInformationKeyVectorValue;
v->InitializeObjectBase();
v->Value.insert(v->Value.begin(), value, value+length);
this->SetAsObjectBase(info, v);
v->Delete();
Expand Down
2 changes: 1 addition & 1 deletion Common/Core/vtkInformationObjectBaseVectorKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ vtkInformationObjectBaseVectorValue *
if(base==NULL)
{
base=new vtkInformationObjectBaseVectorValue;
this->ConstructClass("vtkInformationObjectBaseVectorValue"); // For debug info
base->InitializeObjectBase();
this->SetAsObjectBase(info, base);
base->Delete();
}
Expand Down
2 changes: 1 addition & 1 deletion Common/Core/vtkInformationStringKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void vtkInformationStringKey::Set(vtkInformation* info, const char* value)
{
// Allocate a new value.
vtkInformationStringValue* v = new vtkInformationStringValue;
this->ConstructClass("vtkInformationStringValue");
v->InitializeObjectBase();
v->Value = value;
this->SetAsObjectBase(info, v);
v->Delete();
Expand Down
2 changes: 1 addition & 1 deletion Common/Core/vtkInformationStringVectorKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ void vtkInformationStringVectorKey::Set(vtkInformation* info, const char* value,
{
vtkInformationStringVectorValue* v =
new vtkInformationStringVectorValue;
this->ConstructClass("vtkInformationStringVectorValue");
v->InitializeObjectBase();
while(static_cast<int>(v->Value.size()) <= index)
{
v->Value.push_back("");
Expand Down
2 changes: 1 addition & 1 deletion Common/Core/vtkInformationUnsignedLongKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ void vtkInformationUnsignedLongKey::Set(vtkInformation* info,
{
// Allocate a new value.
vtkInformationUnsignedLongValue* v = new vtkInformationUnsignedLongValue;
this->ConstructClass("vtkInformationUnsignedLongValue");
v->InitializeObjectBase();
v->Value = value;
this->SetAsObjectBase(info, v);
v->Delete();
Expand Down
2 changes: 1 addition & 1 deletion Common/Core/vtkInformationVariantKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void vtkInformationVariantKey::Set(vtkInformation* info, const vtkVariant& value
{
// Allocate a new value.
vtkInformationVariantValue* v = new vtkInformationVariantValue;
this->ConstructClass("vtkInformationVariantValue");
v->InitializeObjectBase();
v->Value = value;
this->SetAsObjectBase(info, v);
v->Delete();
Expand Down
2 changes: 1 addition & 1 deletion Common/Core/vtkInformationVariantVectorKey.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ void vtkInformationVariantVectorKey::Set(vtkInformation* info, const vtkVariant*
}
vtkInformationVariantVectorValue* v =
new vtkInformationVariantVectorValue;
this->ConstructClass("vtkInformationVariantVectorValue");
v->InitializeObjectBase();
v->Value.insert(v->Value.begin(), value, value+length);
this->SetAsObjectBase(info, v);
v->Delete();
Expand Down
8 changes: 4 additions & 4 deletions Common/Core/vtkMath.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ class vtkMathInternal : public vtkObjectBase
vtkBaseTypeMacro(vtkMathInternal, vtkObjectBase);
static vtkMathInternal* New()
{
#ifdef VTK_DEBUG_LEAKS
vtkDebugLeaks::ConstructClass("vtkMathInternal");
#endif
return new vtkMathInternal();
// Can't use object factory macros, since they cast to vtkObject*.
vtkMathInternal *ret = new vtkMathInternal;
ret->InitializeObjectBase();
return ret;
}

vtkMinimalStandardRandomSequence *Uniform;
Expand Down
8 changes: 4 additions & 4 deletions Common/Core/vtkObject.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "vtkCommand.h"
#include "vtkDebugLeaks.h"
#include "vtkGarbageCollector.h"
#include "vtkObjectFactory.h"
#include "vtkTimeStamp.h"
#include "vtkWeakPointer.h"

Expand Down Expand Up @@ -126,10 +127,9 @@ class vtkSubjectHelper

vtkObject *vtkObject::New()
{
#ifdef VTK_DEBUG_LEAKS
vtkDebugLeaks::ConstructClass("vtkObject");
#endif
return new vtkObject;
vtkObject *ret = new vtkObject;
ret->InitializeObjectBase();
return ret;
}


Expand Down
8 changes: 8 additions & 0 deletions Common/Core/vtkObjectBase.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ vtkObjectBase::~vtkObjectBase()
}
}

//----------------------------------------------------------------------------
void vtkObjectBase::InitializeObjectBase()
{
#ifdef VTK_DEBUG_LEAKS
vtkDebugLeaks::ConstructClass(this->GetClassName());
#endif // VTK_DEBUG_LEAKS
}

//----------------------------------------------------------------------------
#ifdef VTK_WORKAROUND_WINDOWS_MANGLE
# undef GetClassName
Expand Down
Loading

0 comments on commit e5c793d

Please sign in to comment.