<div dir="ltr">I've pushed a WIP for this to <a href="https://gitlab.kitware.com/vtk/vtk/merge_requests/1982">https://gitlab.kitware.com/vtk/vtk/merge_requests/1982</a>. Reviewers needed.<div><br></div><div>In a nutshell, all vtkDebugLeaks registration now occurs in a new method vtkObjectBase::InitializeObjectBase, and *nowhere else*. This is called after object construction. The basic rule of thumb is "if you call new (the C++ operator, not the vtk static New() pattern), call InitializeObjectBase()". This ensures that all vtkObjectBase subclasses, including templates, will be registered / deregistered from vtkDebugLeaks consistently and removes the need to scatter "#ifdef VTK_DEBUG_LEAKS" blocks around. The vtkObjectFactory macros all take care of this for you, but if you write a custom ::New() implementation, you'll need to make sure to call this.</div><div><br></div><div>The vtkObjectFactory macros that will handle vtkDebugLeaks registration are:</div><div><br></div><div>// Simply "return new thisClass;", unless</div><div>// VTK_ALL_NEW_OBJECT_FACTORY is defined, then</div><div>// use VTK_OBJECT_FACTORY_NEW_BODY to implement.</div>
vtkStandardNewMacro(thisClass)<br>VTK_STANDARD_NEW_BODY(thisClass)<div><br></div><div>// Check object factory for override, return new thisClass if none found.<br><div>vtkObjectFactoryNewMacro(thisClass)<br>VTK_OBJECT_FACTORY_NEW_BODY(thisClass)</div><div><br></div><div>// Check object factory for override, print error message if none found.<br>vtkAbstractObjectFactoryNewMacro(thisClass)<br>VTK_ABSTRACT_OBJECT_FACTORY_NEW_BODY(thisClass)<br><div><br></div><div>In addition, all objects returned from vtkObjectFactory::CreateInstance will already be registered with vtkDebugLeaks.</div><div><br><div>There are two pre-existing exceptions to vtkDebugLeaks -- vtkCommand subclasses (all are registered as "vtkCommand or subclass" in the vtkCommand constructor), and vtkInformationKey subclasses (These are static objects that aren't reference counted). The patterns used for these classes prior to this change will still work afterwards.</div><div><br></div><div>I also added a preprocessor define to vtkObjectBase.h: VTK_HAS_INITIALIZE_OBJECT_BASE. You can use this in case an application needs to support versions of VTK both before and after this change while using vtkDebugLeaks.</div><div><br></div><div>I found that most classes that need updating are found by grepping the codebase for the string "new vtk" (assuming all of your vtkObject subclasses start with vtk) and also replacing manual registrations by grepping for "VTK_DEBUG_LEAKS". If the invocation isn't on a vtkCommand/vtkInformationKey subclass, either replace the call with a vtkObjectFactory macro or add a call to newObj->InitializeObjectBase() on the new object.</div><div><br></div><div>Dave</div><div><br></div>
</div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 9, 2016 at 9:27 AM, David Lonie <span dir="ltr"><<a href="mailto:david.lonie@kitware.com" target="_blank">david.lonie@kitware.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, Sep 8, 2016 at 6:11 PM, David Cole <<a href="mailto:DLRdave@aol.com">DLRdave@aol.com</a>> wrote:<br>
> A few comments:<br>
><br>
> That should be typeid(*this), and I'm uncertain if that works in the<br>
> context of a base class constructor before the derived class's<br>
> constructor is called. Does it? Doesn't seem like it should...<br>
<br>
</span>typeid will always resolve to the "current" class when called from a<br>
constructor/destructor.<br>
<br>
For science: <a href="http://codepad.org/WWweG8hk" rel="noreferrer" target="_blank">http://codepad.org/WWweG8hk</a><br>
<br>
#include <iostream><br>
<br>
struct Base<br>
{<br>
  Base() { std::cout << typeid(*this).name() << std::endl; }<br>
  virtual ~Base() { std::cout << "~" << typeid(*this).name() << std::endl; }<br>
<br>
  void Init() const { std::cout << typeid(*this).name() << std::endl; }<br>
};<br>
<br>
struct Derived : public Base<br>
{<br>
};<br>
<br>
int main()<br>
{<br>
  Derived d;<br>
  d.Init();<br>
  return 0;<br>
}<br>
<br>
Output:<br>
4Base<br>
7Derived<br>
~4Base<br>
<br>
The second suggestion in my first email would move the construction<br>
logic to something like the Init() method in that example, which<br>
resolves properly to the Derived class.<br>
<span class=""><br>
> If you do end up tracking every instance, and printing them all at<br>
> leak time, perhaps have a sane way of only printing **some** of them.<br>
> Usually, when I end up with a leak, it's connected to a vast network<br>
> of objects, and they all leak, with hundreds, if not thousands of<br>
> objects leaking.<br>
<br>
</span>I agree, digging through the ::Print() output for 100's of objects<br>
would not be useful for most cases! My plan for the object-based<br>
report would be to iterate through the leaked objects, collect<br>
classnames/counts, and print the summary just like it does now by<br>
default. Optionally it could check an environment variable to print a<br>
complete output with object details.<br>
<span class=""><br>
> Also, if you end up tracking every instance, it would be great to have<br>
> an API to the leaks manager to count and iterate all the outstanding<br>
> instances.<br>
<br>
</span>That would certainly be useful. Something like void<br>
vtkDebugLeaks::<wbr>GetCurrentObjects(<wbr>vtkObjectBaseCollection *col) should<br>
do the trick.<br>
<br>
Personally I'm leaning towards the second option (collect strings but<br>
use a centralized vtkObjectBase::InternalInit() method) as it's less<br>
intrusive/less work.<br>
<br>
Dave<br>
</blockquote></div><br></div>