UserData Memory Leaking Problem
-
On 29/06/2013 at 10:53, xxxxxxxx wrote:
Unfortunately, Maxon is working with a standard of C++ that goes back at least a decade.
This is not true any more. C4D R14 SDK uses some C++11 features, especially in c4d_misc !!!
IMHO Exceptions in C++ are bad Idea in many case.
Yes of course STD containers require Exceptions and this is main problem with it.> And how much low level memory management we have to handle ourselves.
If you do it manually then it is dangerous and cumbersome, but you do not need to do this just use RAII all the time.
This is pretty easy if you can use C++11, but of course also possible in C++03 too.BaseContainer data = *bc->GetClone(COPYFLAGS_0, NULL); ///this is really bad idea!!! MemLeak !BaseContainer *data = bc->GetClone(COPYFLAGS_0, NULL); /// a little bit better, but still dangerous. ... a lot of code ... gDelete(data ); ///possible MemLeak !!!AutoPtr<BaseContainer> data( bc->GetClone(COPYFLAGS_0, NULL) ); ///Using RAII it is hard to make MemLeak.BaseContainer data; bc->CopyTo(&data,COPYFLAGS_0,NULL); /// here is another possilbe solutionOs course the question why do you need BaseContainer data at all if you do not use it in you code ???
-
On 29/06/2013 at 11:00, xxxxxxxx wrote:
Double post, sorry
-
On 29/06/2013 at 11:02, xxxxxxxx wrote:
On a side note: Is there a functionality in Cinema _somewhere_ that allows userdata
to get sorted? I don't know of any. Yeah, of course it would be useful if there was a
an SDK function that automatically does this, but you could list up plenty of other
methods that would be useful when being included directly in the SDK. Maxon can't just
put all functions that someone may need at some point. It would blow up the frame. Everything
in the SDK is what Maxon uses themselves. Any other functionality can be implemented
by third parties, which is basically the principle of plugins.Memory handling has always been necessary in C and C++. And the SDK is very precise
when it comes to information on when memory is yours or the applications memory.That's my opinion.
-Niklas -
On 29/06/2013 at 13:05, xxxxxxxx wrote:
Remo,
Thanks for your suggestions. I tried them but I had some troubles.
This is a section of the code block that I have in my plugin that copies one UD values to another UD://Get the data from the third UD entry DescID pos(DescLevel(ID_USERDATA, DTYPE_SUBCONTAINER, 0), 3); const BaseContainer *dataPos = ud->Find(pos); //Get the data for this specific UD item BaseContainer bcPos = *dataPos->GetClone(COPYFLAGS_0, NULL); //Get a copy of the UD's BaseContainer GeData dPos; tag->GetParameter(pos, dPos, DESCFLAGS_GET_0); //Get the values for the gizmos in the source UD we want to copy and store them in d String name = dataPos->GetString(DESC_NAME); //Get the name of the source UD we want to copy LONG type = dataPos->GetLong(DESC_CUSTOMGUI); //Get the data type setting LONG unit = dataPos->GetLong(DESC_UNIT); //Get the units setting(Real, Percent. etc..) //Set the first UD values to the same as the third DescID first(DescLevel(ID_USERDATA, DTYPE_SUBCONTAINER, 0), 1); ud->Set(first, bcPos, NULL); tag->SetParameter(DescID(first), GeData(dPos), DESCFLAGS_SET_0);Trying your suggestions this is what happened:
1.) AutoPtr<BaseContainer> data( bc->GetClone(COPYFLAGS_0, NULL) );
This compiles and runs for me. But it still leaks memory the same way as before.2.) BaseContainer data;
bc->CopyTo(&data,COPYFLAGS_0,NULL);
I could not get this to compile. const BaseContainer *dataPos = ud->Find(pos); will not let me use this code.
I get this error: can't convert *BaseConatiner to baseContainer.3.)BaseContainer *data = bc->GetClone(COPYFLAGS_0, NULL);
//the rest of my copy code
gDelete(data ); //<--Crashes C4D!!
This one crashes C4D. And I assume that's happening because I'm trying to free it in the same scope as I've created it?
I also tried making the BaseContainer *data variable a class member variable. Then freeing it in the destructor. But C4D still crashes on me.To give you guys a better idea of what I'm battling. Here is the link to my plugin with the source code:https://sites.google.com/site/scottayersmedia/PoseLibrary_R13.zip
I know it's a lot of code to look through. I'm sorry for that.
But I think I have everything working properly except for the UserData section in the GeDialog's Command() method. So you shouldn't need to look through the entire thing.
If I can just get this one last memory problem solved with the UserData. I think the plugin will finally be memory leak free.Thanks for the help,
-ScottA -
On 29/06/2013 at 15:20, xxxxxxxx wrote:
Heap allocated memory does have nothing to do with the scope. Have you confirmed that Cinema
crashes at exactly the gDelete(data); line? It's quite possible that you still try to access the container
although you have already deleted it.Why not use the copy-constructor or assignment operator? They both copy the values from the
original to the new/assigned container. You are already making use of it. To be more precise, in
your code, you are actually doing two copys of the container, which is not necessary.BaseContainer data(bc); BaseContainer data = bc;Do both create legal copies "data" of the container "bc" on the stack (which means you do not
have to take care of the deallocation).-Niklas
-
On 29/06/2013 at 16:42, xxxxxxxx wrote:
I'm not sure If I understand Nik.
The typical problems I'm running into is that certain UserData functions (like BrowseGetNext) require a const container. And others require a pointer based *BaseContainer. And others use a standard non-pointer based BaseContainer.
This is one reason why it's very confusing to use these things together. They all have their little specific restrictions imposed on them. And it gets very confusing.
For example:
The reason I don't just use the bc variable in my custom udSequential() method. Is because Set->() requires a pointer based *BaseContainer().
So the only reason for BaseContainer data = *bc->GetClone(COPYFLAGS_0, NULL); to exist in that method is so I can use it in the: ud->Set(dscID, data, NULL); line of code.When it comes to the code in my Command() method in the GeDialog.
I just noticed that I'm also doing that same thing. But I don't really need to do that, because in that code block I'm only needing to copy the parameter values of a UD (the names, units, field values). And I really don't need to copy or Set->() the ID# stuff.
So I think I can safely remove these from my code:
BaseContainer bcPos = *dataPos->GetClone(COPYFLAGS_0, NULL);
BaseContainer bcRot = *dataRot->GetClone(COPYFLAGS_0, NULL);
Thanks for making me see that.But even with that code deleted. I'm still getting memory leaks from:
const BaseContainer *dataPos = ud->Find(pos);
const BaseContainer *dataRot = ud->Find(rot);And if I try to use gDelete(dataPos); & gDelete(dataRot);
C4D freezes(crashes).-ScottA
-
On 29/06/2013 at 17:39, xxxxxxxx wrote:
Doh!
Doh!
Doh!
I'm such a moron.
-First. Thank you Remo.
This does indeed fix the memory leak problem: AutoPtr<BaseContainer> data( bc->GetClone(COPYFLAGS_0, NULL) );
The reason it didn't work for me before was because I was using it in the wrong place in my code.-Second. Thank you Niklas.
You helped me to see some code I could remove from one of my code blocks.I think that finally squashes all the memory leaks In this plugin.
I'll have to test if for a while to make sure.
Good God this thing was a total nightmare to develop. But I sure did learn a lot from it.Thanks guys,
-ScottA -
On 29/06/2013 at 19:08, xxxxxxxx wrote:
Originally posted by xxxxxxxx
Memory handling has always been necessary in C and C++. And the SDK is very precise
when it comes to information on when memory is yours or the applications memory.That is because many of us long-time developers pushed them to make these clarities. Earlier versions were very difficult to work with because of this lack of ownership information. They have definitely improved upon that extremely well!
-
On 29/06/2013 at 21:20, xxxxxxxx wrote:
Originally posted by xxxxxxxx
On a side note: Is there a functionality in Cinema _somewhere_ that allows userdata to get sorted?
i am not sure what you do mean with sorted in that context, but ther is always bc.sort()
-
On 30/06/2013 at 00:07, xxxxxxxx wrote:
Originally posted by xxxxxxxx
[...]
BaseContainer bcPos = *dataPos->GetClone(COPYFLAGS_0, NULL);
BaseContainer bcRot = *dataRot->GetClone(COPYFLAGS_0, NULL);
Thanks for making me see that.Glad I could help you. Btw, these should result in memory leaks, too!
And sorry, my examples above were not completely correct. Assuming "bc" is a pointer to a
container, you have to dereference it of course!> BaseContainer data = *bc;
>
> BaseContainer data(*bc);Originally posted by xxxxxxxx
That is because many of us long-time developers pushed them to make these clarities. Earlier versions were very difficult to work with because of this lack of ownership information. They have definitely improved upon that extremely well!
I see. You know, I'm not using the C++ SDK for that long I guess..
Originally posted by xxxxxxxx
Originally posted by xxxxxxxx
On a side note: Is there a functionality in Cinema _somewhere_ that allows userdata to get sorted?
i am not sure what you do mean with sorted in that context, but ther is always bc.sort()
I didn't mean sorting a container, but the collection of user-data on an object (which is what
Scott is trying to do). Everything in the SDK is at least used once by Maxon in development. There
is _no_ part in Cinema that results in user-data being re-arranged and sorted. Why should they
include such a functionality in the API? Besides the fact there are quite numerous ways to sort the
data (id, name, value, type, grouped or not, etc) which they can't just handle all because they were
still missing something. A third-party can write its own when necessary and exactly define the data
being sorted the way they need.-Niklas
-
On 30/06/2013 at 09:38, xxxxxxxx wrote:
Originally posted by xxxxxxxx
Why should they include such a functionality in the API?
Because UserData is one of the most used areas of cinema4D. That's why.
When you have an area of the program that virtually everyone uses. Shouldn't that area be one of the most developed sections of the SDK?At bare minimum. I think there should be:
-Built-in functions to set and change ID#s
-Built in sorting for the UD entries(that also removes gaps in ID#s)It's seems very strange to me that they are not in there. And that I had to write such tasks by hand.
-ScottA