[opensource-dev] Virtual Destructors

Ricky kf6kjg at gmail.com
Fri Jul 1 16:02:15 PDT 2011


Looks like the destructors might only be called at program
termination, so this may not be a big problem anyway.  However it IS
inconsistent and weird.  And I have it within reach to clean up if it
seems good to do so.

Ricky
Cron Stardust

On Fri, Jul 1, 2011 at 3:25 PM, Ricky <kf6kjg at gmail.com> wrote:
> Poking around in the llmanip* files working on VWR-25739, I started to
> get annoyed at the coding inconsistencies between those files.  So I
> started looking at what it would take to make the 3 subclasses
> (translate, scale, and rotate) consistent, when I tripped across the
> detail that llmaniptranslate.h has the destructor declared virtual
> while llmanipscale.h has it declared plainly, and llmaniprotate.h
> doesn't explicitly declare a destructor.
>
> When I looked up some reasons why a destructor should be virtual it
> seems that it should be virtual when the class is going to be used in
> a polymorphic way and will have delete called on a pointer to it.  IE:
> // MyClass is a ParentClass
> ParentClass* p = new MyClass();
> destroy p;
>
> Apparently this is about the only case for declaring the destructor
> virtual. (see http://blogs.msdn.com/b/oldnewthing/archive/2004/05/07/127826.aspx
> and especially http://www.erata.net/programming/virtual-destructors/ )
>  It also comes with a minor performance hit, but that's outside of
> scope.
>
> It turns out that LLManipScale _is_ being used in such a way in
> LLToolComp - as are LLManipScale and LLManipRotate:
> lltoolcomp.h line 92: LLManip* mManip;
> lltoolcomp.cpp line 194: mManip = new LLManipTranslate(this);
> lltoolcomp.cpp line 203: delete mManip;
> lltoolcomp.cpp line 321: mManip = new LLManipScale(this);
> lltoolcomp.cpp line 330: delete mManip;
> lltoolcomp.cpp line 520: mManip = new LLManipRotate(this);
> lltoolcomp.cpp line 530: delete mManip;
>
> So it looks like to me that there might be a memory leak in the scale
> and rotate classes, as their destructors might NOT be being called.
> Of course, Translate's destructor has only an empty definition, and
> Rotate doesn't even have one, but Scale does have a full-on
> destructor.  And because it is not virtual, it might not be being
> called.
>
> Looking over the history of the files gives me the following:
> The Rotate destructor was last touched by Steven Bennets on 2008-03-11
> in rev 341 - when LLLinkedList was culled in favor of another
> technique.
> The Translate destructor was emptied by James Cook on 2009-12-10 in
> rev 4496 - switched to a std::vector
> The Scale destructor seems to have never existed in revision history.
>
> Anyone with more familiarity with C++'s nuances in such cases have any
> thoughts/suggestions?
>
> Ricky
> Cron Stardust
>


More information about the opensource-dev mailing list