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 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] Subject: RE: CRefArray doesn't respect C++ copy semantics I say .... CRefArray copy ctor is buggy! thanks for reporting. -mab From: [email protected] [mailto:[email protected]] On Behalf Of Nicolas Burtnyk Sent: Monday, April 30, 2012 9:05 PM To: [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> object from another CRefArray<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#acb58b1fecf704752ebcf59a50444cf37> (const CValueArray<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:[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:[email protected]> Sent: Tuesday, May 01, 2012 1:31 AM To: [email protected]<mailto:[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>& operator= ( const CRefArray<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> 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> 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.
