UserData Memory Leaking Problem



  • On 29/06/2013 at 09:44, xxxxxxxx wrote:

    Unfortunately, Maxon is working with a standard of C++ that goes back at least a decade.  No exception handling which makes use of STL very 'dangerous' if not fully handled in the try/throw/catch paradigm.  They are only slowly implementing systemic memory handling such as garbage collection.  But I think the main problem is in transitioning an archaic system to something more recent leaves those artifacts - sort of like Windows with its MS-DOS underpinnings and many issues.  You will have to live with them, learn how to adapt to them, and get the best results while showing a grinding grin.  Check every allocation and pointer.  Be methodical in your logic and program flow. Hope to hell that the gremlins don't find you!



  • On 29/06/2013 at 10:37, xxxxxxxx wrote:

    Yeah I agree.
    Just about everything in the C4D C++ is fairly dangerous and requires a lot of low level memory handling. But if there's one area of the SDK that I expected it to be fully fleshed out and have lots of built-in functions. It would be the UserData section.
    I'm rather shocked at how such an important part of the program has so little SDK code written for it. And how much low level memory management we have to handle ourselves.

    Anyway,
    Back to the problem. I have to figure out how I can use multiple ->GetClone() lines of code in my plugins. And not get memory leaks.

    -ScottA



  • 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 solution  
    

    Os 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


Log in to reply