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 <kf6...@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 > _______________________________________________ Policies and (un)subscribe information available here: http://wiki.secondlife.com/wiki/OpenSource-Dev Please read the policies before posting to keep unmoderated posting privileges
