Hey Piotrek, Memory leaks shouldn't happen if the refcounter implementation does its job correctly. Also, the memory block associated to the m_pickedObjects variable is automatically asked to be freed when your class is destroyed. You could also eliminate the "CRefArray::Clear()" call which seems useless here (you could maybe gain some useconds) since the object could be destroyed when assignment is done.
jo 2012/5/2 piotrek marczak <[email protected]> > Thank you for detailed reply! > I didn’t know about refcount concept. It seems much better (faster) > solution than copying data. > So, If I understood correctly your example, my function, called multiple > times, won’t lead to memory leaks. > *From:* jo benayoun <[email protected]> > *Sent:* Wednesday, May 02, 2012 10:50 AM > *To:* [email protected] > *Subject:* Re: CRefArray doesn't respect C++ copy semantics > > Hey Piotrek, > > We don't know about the CRefArray implementation, so I will just mention > how things would happen using common programming practices in cpp. > > Heavy data structures are usually implemented by separating the actual > datas from the interface the client is using (methods). > The datas are encapsulated in really small containers often called POD > because they are similar to C structures. > Considering operations like copy by value or assignment are really heavy > during runtime (creation of a new object and then deep copy of the datas), > a common practice is to share those POD objects between interfaces > (often, copies or assigned variables are only temporary during functions > calls or just for holding a content while manipulating multiple variables). > An another mechanism (refcounter, smart pointers, ...) is often used in > complement to keep a track of those objects which can be shared between > multiple classes to be sure they are deleted correctly. > > The idea, is what you are manipulating with your code is not the real > datas, but just interfaces. COM objects use similar concepts. > Some libraries make those mechanisms transparent (often the case in cpp) > and some others (often in c due to the language itself), you have to do it > by yourself (like with the python c api). > > What would happen in the case of your example is: > > when the class is instanciated : > -> m_pickedObjects = new CRefArray () > -> m_pickedObjects->data = new Data > -> m_pickedObjects->data->refcount = 1. > > at the beginning of your block > -> l_moveableObjects = new CRefArray > -> l_moveableObjects->data = new Data > -> l_moveableObjects->data->refcount = 1. > > you assigned l_moveableObjects to m_pickedObjects > -> m_pickedObjects->data->refcount - 1 > -> m_pickedObjects->data->refcount == 0 ? delete m_pickedObjects->data : > do nothing > -> m_pickedObjects->data = l_moveableObjects->data > -> l_moveableObjects->data->refcount + 1 (now equal to 2, because > shared between 2 objects, m_picked and l_moveable) > > at the end of your block > -> l_moveableObjects->data->refcount - 1 > -> l_moveableObjects ->data->refcount == 0 ? delete l_moveableObjects > ->data : do nothing (in this case, the refcount is equal to 1, > because still shared by m_picked) > > Also, the big differences between the copy constructor and the assignement > operator are: > - the copy constructor is a constructor, and when its called, it means a > new object is being created. > - the assignement is just an overloaded operator (=method) returning an > object (new or not, usually a reference of itself) > - during an assignment, if the object passed as rvalue is itself (in the > case 'm_picked = m_picked'), nothing is done and we just return a reference > of itself (*this) > - during a copy constructor call, if the object passed as parameter is > itself, a new object is still created. > > I hope Marc Andre will correct me if I'm wrong to suppose CRefArray > implementation is following similar guidelines. > :) > jo > > > > > > > > > > > > > > > > > > > > > 2012/5/1 piotrek marczak <[email protected]> > >> void UpdateCollisionObjects(**ToolContext &in_ctxt) >> { >> m_pickedObjects .Clear(); >> CRefArray l_moveableObjects; >> >> // fill l_moveableObjects with cref()s... >> >> m_pickedObjects = l_moveableObjects; >> } >> >> So why this one is working? If it's just referencing to local class, not >> making deep copy, and local class is destroyed on function's end? >> >> -----Oryginalna wiadomość----- From: Marc-Andre Belzile >> Sent: Tuesday, May 01, 2012 3:09 PM >> To: [email protected].**com <[email protected]> >> Subject: RE: CRefArray doesn't respect C++ copy semantics >> >> >> I say .... CRefArray copy ctor is buggy! >> >> thanks for reporting. >> >> -mab >> >> From: >> softimage-bounces@listproc.**autodesk.com<[email protected]>[mailto: >> softimage-bounces@**listproc.autodesk.com<[email protected]>] >> On Behalf Of Nicolas Burtnyk >> Sent: Monday, April 30, 2012 9:05 PM >> To: [email protected].**com <[email protected]> >> Subject: Re: CRefArray doesn't respect C++ copy semantics >> >> I opened a case with support for this. We'll see what they say... >> >> On Mon, Apr 30, 2012 at 5:59 PM, Alok Gandhi <[email protected]<** >> mailto:[email protected]**>> wrote: >> You are right Jo, I mentioned the wrong prototype, what I wanted to quote >> was the Copy Constructor and the assignment operator. Sorry about the >> confusion. I clicked on the wrong link. Of course what Nicolas was using >> was a copy constructor, since both the class methods one after another in >> the docs, I copy-pasted in haste the wrong one. Anyways, yes Nicolas what >> you are saying is true regarding the unexpected behavior. >> >> The problem is they're treating CRefArray like it's a reference to an >>> array of CRefs rather than just a straight-up array of CRefs. >>> >> That is exactly what is happening here for sure, I concur. >> >> Maybe the new dev team could explain this better for the benefit of all >> the developers as this could seriously cause big time debugging headaches. >> Thanks for finding this out Nicolas ! >> >> Error! Filename not specified. >> >> On 4/30/2012 8:03 PM, jo benayoun wrote: >> in this case, >> >> """ >> CRefArray a1; >> a1.Add(CRef()); >> a1.Add(CRef()); >> CRefArray a2(a1); >> a2.Add(CRef()); >> """ >> >> The copy constructor is invoked and is not the one you mentioned Alok but >> this one "CRefArray(const CRefArray &other)" which have different behaviors >> and purposes than the overloaded assignment operator. >> >> """Copy constructor is called every time a copy of an object is made. >> When you pass an object by value, either into a function or as a function's >> return value, a temporary copy of that object is made. >> >> Assignment operator is called whenever you assign to an object. >> Assignment operator must check to see if the right-hand side of the >> assignment operator is the object itself. It executes only the two sides >> are not equal >> """ >> >> Referring to the docs : "Constructs a CRefArray<http://download.** >> autodesk.com/global/docs/**softimage2012/en_us/sdkguide/** >> si_cpp/classXSI_1_1CRefArray.**html<http://download.autodesk.com/global/docs/softimage2012/en_us/sdkguide/si_cpp/classXSI_1_1CRefArray.html>> >> object from another CRefArray<http://download.**autodesk.com/global/docs/ >> **softimage2012/en_us/sdkguide/**si_cpp/classXSI_1_1CRefArray.**html<http://download.autodesk.com/global/docs/softimage2012/en_us/sdkguide/si_cpp/classXSI_1_1CRefArray.html>> >> object." which is the expected behavior. >> >> For completeness, the copy constructor in the case of an array, a string, >> a ptr or whatever container has a main purpose to "pass" an implicit shared >> memory block to save memory specially in >> the case of "passing arguments by value". A deep copy is done only at the >> first call of a method non-const which should create a brand new underlying >> object (concept called copy-on-write). >> In this case, it seems its not what happens ... which is a bug in all >> case unless its a wanted behavior and it should be specified in the doc ! >> A more comprehensible example is the python "list" example: >> doing an assignment "mylist = myotherlist" creates a shallow copy and >> returns the "myotherlist" object to the mylist which is not the case of >> calling the ctor directly with "mylist = list(myotherlist)". That's a >> behavior that could be implemented here. >> >> """ >> mylist = [1, 2] >> print mylist >> myotherlist = mylist >> mylist.append(6) >> print mylist >> print myotherlist >> myoolist = list(mylist) >> mylist.append(9) >> print mylist >> print myotherlist >> print myoolist >> """ >> >> >> "Set<http://download.autodesk.**com/global/docs/softimage2012/** >> en_us/sdkguide/si_cpp/**classXSI_1_1CRefArray.html#** >> acb58b1fecf704752ebcf59a50444c**f37<http://download.autodesk.com/global/docs/softimage2012/en_us/sdkguide/si_cpp/classXSI_1_1CRefArray.html#acb58b1fecf704752ebcf59a50444cf37>> >> (const CValueArray<http://download.**autodesk.com/global/docs/** >> softimage2012/en_us/sdkguide/**si_cpp/classXSI_1_**1CValueArray.html<http://download.autodesk.com/global/docs/softimage2012/en_us/sdkguide/si_cpp/classXSI_1_1CValueArray.html>> >> &in_valarray)" >> I dont see any overloaded cast method nor ctors for a CRefArray to >> CValueArray. Even if there was, it would mean that your CValueArray have to >> be built from a CRefArray before being passed by reference. Which is an >> overhead instead of using the copy ctor. >> "const" is a keyword that we use to assure to the compiler, we will not >> try to modify the underlying memory block nor call any procedures that >> could do this. Of course, the compiler takes this as serious and do >> optimizations in consequences which is a good thing for us. >> >> jo >> >> >> >> >> >> >> >> >> >> >> >> >> >> 2012/4/30 piotrek marczak <[email protected]<**mailto: >> piotrek.marczak@gmail.**com <[email protected]>>> >> Maybe a2.Set(a1) or a2+=a1 would work? >> >> newbie question >> isn't "const" keyword a hint that we won't change input array? >> From: Alok Gandhi<mailto:alok.gandhi@**modusfx.com<[email protected]> >> > >> Sent: Tuesday, May 01, 2012 1:31 AM >> To: [email protected].**com <[email protected]> >> <mailto:softimage@listproc.**autodesk.com<[email protected]> >> > >> Subject: Re: CRefArray doesn't respect C++ copy semantics >> >> The docs say that: >> CRefArray<http://download.**autodesk.com/global/docs/** >> softimage2012/en_us/sdkguide/**si_cpp/classXSI_1_1CRefArray.**html<http://download.autodesk.com/global/docs/softimage2012/en_us/sdkguide/si_cpp/classXSI_1_1CRefArray.html>>& >> operator= >> >> ( >> >> const CRefArray<http://download.**autodesk.com/global/docs/** >> softimage2012/en_us/sdkguide/**si_cpp/classXSI_1_1CRefArray.**html<http://download.autodesk.com/global/docs/softimage2012/en_us/sdkguide/si_cpp/classXSI_1_1CRefArray.html>> >> & >> >> in_refArray >> >> ) >> >> >> Assigns a CRefArray<http://download.**autodesk.com/global/docs/** >> softimage2012/en_us/sdkguide/**si_cpp/classXSI_1_1CRefArray.**html<http://download.autodesk.com/global/docs/softimage2012/en_us/sdkguide/si_cpp/classXSI_1_1CRefArray.html>> >> object to this one. >> Parameters: >> in_refArray >> >> A constant CRefArray<http://download.**autodesk.com/global/docs/** >> softimage2012/en_us/sdkguide/**si_cpp/classXSI_1_1CRefArray.**html<http://download.autodesk.com/global/docs/softimage2012/en_us/sdkguide/si_cpp/classXSI_1_1CRefArray.html>> >> object. >> >> Returns: >> A new reference object. >> >> So what I think is happening is that the copy constructor is doing >> exactly what it is supposed to do and returns the new CRefArray object >> which still points to a1, 'assigns' is the operative word here. To keep >> them separate I would rather do: >> >> CRefArray a1; >> a1.Add(CRef()); >> a1.Add(CRef()); >> CRefArray a2; >> >> for(int i=0; i<a1.GetCount(); i++) >> { >> a2.Add(a1[i]); >> } >> >> a2.Add(CRef()); >> >> //a2.Add(CRef()); >> LONG n1 = a1.GetCount(); // expected n1 == 2 >> LONG n2 = a2.GetCount(); // expected n2 == 3 >> >> which gives me correctly: >> >> # VERBOSE : cRefArrayTest_Execute called >> # VERBOSE : Count a1: 2 >> # VERBOSE : Count a2: 3 >> >> >> Error! Filename not specified. >> >> On 4/30/2012 7:13 PM, Nicolas Burtnyk wrote: >> Yeah, exactly as I unfortunately discovered :( >> On Mon, Apr 30, 2012 at 3:49 PM, Alok Gandhi <[email protected]<** >> mailto:[email protected]**>> wrote: >> A quick test gives me following result: >> >> # VERBOSE : cRefArrayTest_Execute called >> # VERBOSE : Count a1: 3 >> # VERBOSE : Count a2: 3 >> >> Error! Filename not specified. >> >> On 4/30/2012 6:24 PM, Nicolas Burtnyk wrote: >> I ran into this today while trying to figure out why my code was broken. >> Thought I'd pass this along and hopefully save someone some wasted time >> in the future... >> >> CRefArray a1; >> a1.Add(CRef()); >> a1.Add(CRef()); >> CRefArray a2(a1); >> a2.Add(CRef()); >> LONG n1 = a1.GetCount(); // expected n1 == 2 >> LONG n2 = a2.GetCount(); // expected n2 == 3 >> >> I expected a2 to be a copy of a1 before the last add and so I assumed a1 >> would have 2 elements. >> Instead, I was surprised to find that n1 == n2 == 3! >> >> >> >> No virus found in this message. >> Checked by AVG - www.avg.com<http://www.avg.com**> >> Version: 2012.0.1831 / Virus Database: 2090/4557 - Release Date: 10/17/11 >> Internal Virus Database is out of date. >> >> >> >> No virus found in this message. >> Checked by AVG - www.avg.com<http://www.avg.com**> >> Version: 2012.0.1831 / Virus Database: 2090/4557 - Release Date: 10/17/11 >> Internal Virus Database is out of date. >> >> >> >> No virus found in this message. >> Checked by AVG - www.avg.com<http://www.avg.com**> >> Version: 2012.0.1831 / Virus Database: 2090/4557 - Release Date: 10/17/11 >> Internal Virus Database is out of date. >> >> >
