[vtk-developers] Proposal: Simplify vtkDebugLeaks registration

[Moreland, Kenneth]

Why not use typeid(this) instead of GetClassName? As long as this has one virtual function, I believe typeid will give the id for the base class.

-Ken

-----Original Message-----
From: vtk-developers [mailto:vtk-developers-bounces at vtk.org] On Behalf Of David Lonie
Sent: Thursday, September 8, 2016 3:42 PM
To: VTK Developers <vtk-developers at vtk.org>
Subject: [EXTERNAL] [vtk-developers] Proposal: Simplify vtkDebugLeaks registration

Hi all,

We've been seeing some issues surrounding the use of vtkDebugLeaks and template classes lately. The main problem stems from the difficulties of ensuring that ::GetClassName() returns a unique string for different instantiations of a templated object. This is important to ensure that things like SafeDownCast work correctly.

At the moment, ::GetClassName() returns typeid(this).name() for templated types, which is typically very different from the string passed into vtkObjectFactory::ConstructInstance(...) by the object factory macros used to implement the ::New() methods.

Right now we jump through some hoops to try to make sure that vtkDebugLeaks::ConstructClass is called with the ::GetClassName() string, since that is used when vtkObjectBase::UnregisterInternal removes an object from the leak tracker. But we've missed some spots and there are quite a few classes that don't bother with the macros, and thus have custom implementations of ::New() that need to explicitly register themselves with DebugLeaks. It's getting messy and problematic trying to keep all of this consistent.

I propose that we simplify DebugLeaks to make the registration happen automatically. I see a couple of ways to do this:

1) Just track the objects directly by registering their addresses in vtkObjectBase's constructor. This would increase the memory footprint of vtkDebugLeaks, but I think that's not a huge issue since vtkDebugLeaks is a testing/development tool that isn't used in production AFAIK. This would also allow a more detailed printout of the leaked objects since we could call Print() on them.

2) Continue registering name strings, but do so from a
vtkObjectBase::InternalInitialize() (or similar) method that calls vtkDebugLeaks::ConstructClass(this->GetClassName(). This would still need to be called explicitly in custom ::New() implementations, but it would be easier to maintain, update, and keep consistent/correct thanks to the encapsulation to a single method.

Note that we can't just call
vtkDebugLeaks::ConstructClass(this->GetClassName()) from vtkObjectBase's constructor due to how virtual methods in constructors work. ::GetClassName() will always return "vtkObjectBase" when called from vtkObjectBase's constructor.

Any thoughts/objections/suggestions?

Dave
_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Search the list archives at: http://markmail.org/search/?q=vtk-developers

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/vtk-developers