Deleting objects from arrays correctly and preventing leaks

New comment
 

I have this problem, I do not know exactly how to write the code here - do I have to free arrays manually? do I have to manually delete the objects within an object Array? The problem is that even though I tried to do these things, I still get these error messages.. the difference is that I used to get them all the time before when restarting an EA (e.g. through the settings dialog) but now it seems to happen just sometimes after I add this code. Could someone point out where my problems are and how to fix them? thanks.


2019.05.04 19:17:21.453 Mapz (EURUSD,H1) 12 leaked strings left

2019.05.04 19:17:21.453 Mapz (EURUSD,H1) 6 undeleted objects left
2019.05.04 19:17:21.454 Mapz (EURUSD,H1) 6 objects of type SymbolMap left
2019.05.04 19:17:21.454 Mapz (EURUSD,H1) 336 bytes of leaked memory


They might not all pertain to SymbolMap objects.. but the 6 ones do. I'm not sure about the 12 strings, but just ignore that for now.


Here is the pertinent code:


class SymbolMapping
{
    private:
        SymbolMap maps[];

    public:

        // Constructor
        SymbolMapping();
        void DeleteMaps();
}


void SymbolMapping::DeleteMaps() {
    for (int i=0;i<ArraySize(maps);i++) {
        delete &(maps[i]);
    }
    ArrayFree(maps);
}

SymbolMapping::SymbolMapping(string s) {

}

SymbolMapping *mapping = new SymbolMapping();

//+------------------------------------------------------------------+
//| Expert deinitialization function                                 |
//+------------------------------------------------------------------+
void OnDeinit(const int reason)
{
//--- destroy timer
    EventKillTimer();

    mapping.DeleteMaps();
    delete mapping;
}

// the code is not totally complete, I just am trying to illustrate the problem areas




 
ycomp:

Here is the pertinent code:

do I have to free arrays manually?

do I have to manually delete the objects within an object Array?

I still get these error messages.. 

  1. When you post code please use the CODE button (Alt-S)! (For large amounts of code, attach it.) Please edit your (original) post.
              General rules and best pratices of the Forum. - General - MQL5 programming forum
              Messages Editor 

  2. ArrayFree(maps);
    No. All this does is release the memory held by the array, same as resizing to zero. This happens when the containing class is deleted.

  3. SymbolMap maps[];
    No. This is an array of objects, allocated when you resized the array. Destroyed, when you free it, resize it, or when the containing class is destroyed.

    It is not an array of handles:

    SymbolMap *maps[];
    If it was, then you would be responsible to resize the array and new the additional handles, or delete the handles before downsizing the array.

  4. SymbolMapping *mapping = new SymbolMapping();
    You allocated (on load.) Never do this. If you change TF for example, your EA is not unloaded and reloaded, it does through a deinit/init cycle. So you delete it (OnDeinit,) you refer to a bad handle (OnTick,) or try to delete it again … Allocate in OnInit, de-allocate in OnDeinit.

  5. SymbolMap maps[];
    This is an array of objects, not an array of handles.
    delete &(maps[i]);
    You can generate the address of the object, but you an not delete it because it was never allocated.
    mapping.DeleteMaps();
delete mapping;
    This is generally bad design. Mapping should take care of itself. If mapping has internal pointers, it is responsible for deleting them.
 
William Roeder:
  1. When you post code please use the CODE button (Alt-S)! (For large amounts of code, attach it.) Please edit your (original) post.
              General rules and best pratices of the Forum. - General - MQL5 programming forum
              Messages Editor

  2. No. All this does is release the memory held by the array, same as resizing to zero. This happens when the containing class is deleted.

  3. No. This is an array of objects, allocated when you resized the array. Destroyed, when you free it, resize it, or when the containing class is destroyed.

    It is not an array of handles:

    SymbolMap *maps[];
    If it was, then you would be responsible to resize the array and new the additional handles, or delete the handles before downsizing the array.

  4. You allocated (on load.) Never do this. If you change TF for example, your EA is not unloaded and reloaded, it does through a deinit/init cycle. So you delete it (OnDeinit,) you refer to a bad handle (OnTick,) or try to delete it again … Allocate in OnInit, de-allocate in OnDeinit.

  5. This is an array of objects, not an array of handles.You can generate the address of the object, but you an not delete it because it was never allocated.This is generally bad design. Mapping should take care of itself. If mapping has internal pointers, it is responsible for deleting them.

thanks for your analysis. I'm very new to classes in MQL.

Much of the code there I added to try to get rid of the error messages about leakage. It was working fine (making an array of objects and using them) before I tried to "fix things" 

So I'm thinking that my problem just is that I allocated on load and this is the cause of the problems? so I just fix that and I can remove all these explicit deletions.


and add a delete in the OnDeInit() for the mapping object?


update: I removed all the delete code, added a delete mapping in OnDeInit() and moved the assignment of mapping to within the OnInit() and everything seems to work fine now when i load new profiles (that was the way I was testing the problems previously... now I don't see any messages about leaks at all). 

 
William Roeder:
  1. No. This is an array of objects, allocated when you resized the array. Destroyed, when you free it, resize it, or when the containing class is destroyed.


This maps[] array exists within mapping, so my understanding is I don't need to do anything - the array should be freed automatically and its objects deleted automatically when the mapping object is deleted.


I do have another array of objects in my code however. This one exists within the OnTimer() function

It is not contained in any classes. It is simply an array of objects declared within OnTimer()

Do I need to explicitly call ArrayFree() so that the objects will be deleted? or will the array automatically be freed?

I take it from the help for ArrayFree, that this is not necessary. However, just wanted to make sure.

 
ycomp: Do I need to explicitly call ArrayFree() so that the objects will be deleted? or will the array automatically be freed?
Answered in #1.2
New comment
Reason: