Let's discuss how to avoid memory leak in a C like language and possibly make this thread informative - page 2

New comment
 
Alain Verleyen #:

MQL has nothing to do with C in term of memory.

MQL is a managed language, the coder can't create real memory leak. If you forget to "delete" a pointer, the platform will inform you about it.

There is no effective reason to not use pointers in MQL. They are not real pointers anyway.

See this topic : https://www.mql5.com/en/forum/450676/page3#comment_48103170

Let me complement this accurate answer provided by the mod (didn't read the provided link, though, sorry if i'm being repetitive). If you only create a pointer once and forget to delete it, it's fine. However, if you create multiple pointers during the execution of a MQL5 program (EA, script, indicator, etc.) and forget to delete them, that's where lies the real problem.


The terminal only informs you that you leaked memory when a program is deinitialized. At this point, this information is sort of useless, since as Alain said, there'll be no real memory leak (it is actually useful if you call new multiple times and for some reason forget to delete any of them; it ends up being useful so you can detect where it was leaked and avoid unnecessary memory usage during the program lifetime).

Think in an example like this:


#property copyright "Copyright 2024, MetaQuotes Ltd."
#property link      "https://www.mql5.com"
#property version   "1.00"

#property script_show_inputs

#include <Trade/Trade.mqh>

void OnStart()
{   
    int i = 0;
    
    Print("Sizeof CTrade = ", sizeof(CTrade));
    
    while(true)
    {
        if(i++ < 1000000)
        {
            CTrade* trade = new CTrade;
        }
        
        if(TerminalInfoInteger(TERMINAL_KEYSTATE_DELETE))
        {
            break;
        }
    }
}


This is a script that creates 1 million CTrade objects and never deletes them. If you try it yourself, your memory usage will increase up to 1gb using this script. Until we press the delete key, the memory will not be released, since the script would still be running and we're not deleting it ourselves.


The best way to do this is: avoid creating pointers (that is, using the new operator) if you don't know how to properly use them. It'll just increase the complexity and reduce the speed of your project.


Imagine in C/C++: the new operator allocates some space in the heap. If you don't use new, variables are allocated on the stack, and then automatically deleted at the end of the function scope. In MQL5, this behavior happens exactly the same way. One good reason to use pointers in MQL5 would be if you want to get the ownership of the object somewhere else in your program so you don't need to create it again. If there isn't a good reason, don't use them and even avoid some overhead that comes from allocating memory on the heap.


If you do need to use pointers, though, avoid creating and deleting them them multiple times (i.e., create pointers inside the OnInit function is preferrable, not inside OnTick or inside loops). If you think it's necessary to do so, rethink your code design. If there's no solution to it, remember that after a call to the new operator there should always have a call to the delete operator. Even if the memory gets released at the end of the program's execution, it's a good habit to delete the pointer in the OnDeinit call or whenever the pointer is not needed anymore.


Imagine an EA that behaves just like that script and that forgets to delete the pointer every single tick. No PC can take that.


Even though pointers in MQL5 don't refer to an address in memory, they allocate some space in memory. Not deleting them can cause a ridiculous memory consumption until your MQL5 program gets removed.


ps: this is my opinion on pointers and certainly is not the absolute truth. Do your own research/tests about it and how they work and learn how to properly use them in an efficient way that suits your needs.

 
Emanuel Cavalcante Amorim Filho #:

avoid creating pointers (that is, using the new operator) if you don't know how to properly use them. It'll just increase the complexity and reduce the speed of your project.

With this logic you will never learn how to use dynamic objects in MQL. The one who does nothing makes no mistakes.

"Avoid using your washing machine if you know how to turn it on properly" - You will be washing by hand as long as you use this kind of logic.

Emanuel Cavalcante Amorim Filho #:
It'll just increase the complexity and reduce the speed of your project.

What's better, storing a pointer or passing a million parameters to methods? Implementing lists without pointers?

Reduce speed compared to what? A very questionable statement.

 
Vladislav Boyko #:

With this logic you will never learn how to use dynamic objects in MQL. The one who does nothing makes no mistakes.

"Avoid using your washing machine if you know how to turn it on properly" - You will be washing by hand as long as you use this kind of logic.

What's better, storing a pointer or passing a million parameters to methods? Implementing lists without pointers?

Reduce speed compared to what? A very questionable statement.

Use a smart pointer (implemented as a class object that wraps a 'raw' (or 'bare') mql pointer).
 

By the way, here is an interesting library. Personally, I didn’t have a need to use this, since the MT4 tester reports undeleted objects. But I have saved this in my browser bookmarks in case I develop for MT5.

Code Base

Reporting Memory Leaks in Strategy Tester

Dr Matthias Hammelsbeck, 2023.08.12 12:15

Monitoring of memory leaks in the strategy tester
 
Vladislav Boyko #:

With this logic you will never learn how to use dynamic objects in MQL. The one who does nothing makes no mistakes.

"Avoid using your washing machine if you know how to turn it on properly" - You will be washing by hand as long as you use this kind of logic.

What's better, storing a pointer or passing a million parameters to methods? Implementing lists without pointers?

Reduce speed compared to what? A very questionable statement.

Let me also complement my last answer then, cause I was not quite clear: learn how to use it before you do use it in real projects. Thats my actual take on it. People are inclined to try stuff they have no idea what it is before they actually study about it - myself included. I did a lot of mistakes that almost cost me my job when I was younger when working with c++ pointers. As pretty much everything in programming, pointers do have its value, but it comes with a cost - memory management and overhead by calling new/delete. A bad usage of pointers is worse than not using it at all, since you can in most cases just pass a reference to an object - a few cases can be seen in this forum where crashes happen just because a single pointer is not deleted or the EA speed is drastically reduced due to creating and deleting objects multiple times where it is not needed.

I reafirm what I said (at least what I meant): avoid pointers. Learn their best use cases on smaller projects and if you do need to use them on a real project, use them carefully and be aware of what you need to do to make your program safe to errors that could make you lose hundreds or thousands of dolars in a trade. Bugs are part of a programmer's life, but most of them are avoidable in most cases if you study better about the best use cases for language features (i.e. pointers). From what I can see, pointers are not something that people can easily understand (including in MQL5 where pointers do not represent what they actually represent in other languages) and that's my worry about it: you are definitely gonna create bad code when you start pointers for the first time, just don't do it when you are gonna sell your program or when your money is at stake. 

Again, if you don't know how to use pointers, it will 90% (absolutely unproven hyperbolic statistic) of the time reduce your program execution speed. If you do know (like when you want pass the address/descriptor of an object to take its ownership somewhere else, for example), it is really a powerful instrument to any dev and much of the power of lower level languages comes from this feature, for sure.

long story short, I do agree with you that mistakes are a important part of learning process and development, but learn how to use it on smaller projects and understand the concept behind it before deploying on a real project and messing up your programs (specially EAs, where each execution tick counts a lot and a simple bug may cause you a big problem)

 

Guys, your points and discussions really benefits and guide me quite far, and now it's finally my turn to append some info for future visitors.

Although I had always being careful, I was lucky enough to still hit some sort of 'memory leak' after I introduce an event-driven alike concept for WebRequest (call one request per sixth clock/tick).

This bug has haunted me for a few days, because the trigger is sort of pseudo-random (tho, as if my logical arrangement, they shouldn't go wrong, that's why I was haunted so long, you will find out it soon). This bug consist of 2 aspects that contributed to it.

  • your copy constructor does not always get called
  • improper memory management(object with array); theoretically removing an entry simply by hiding that area (concept of deleting data on HDD) but did not really purge it, thus leaving a loophole

Now, you consider any sort of ADT, as long as it is using array underneath to store elements, and read this piece of code || image:

void OnStart(){

      char requestDataAB[] = {'a','b','c'};
      char reqC[] = {'?','?'};
      char response[];
      string resHeaders;
      Request A("P","url","",3000,requestDataAB,response,resHeaders);
      Request B = A;
      Request C("P","url","",3000,reqC,response,resHeaders);
      B = A;//implicit copy, if able to force every = operator uses the logic of copy constructor, this bug can be avoided if le logic is flawless

      //current holding data
      A.requestData[0] = 'z';//zbc
      B.requestData[2] = '9';//ab9
      C.requestData;//??
      
      //ADT internal array size of 8, all default constructed, which all having default/user defined values, insertion via tailIndex
      //ADT will, log, integrity as two types - after *N(whole list) || before *2(received obj & tailIndex destinated existing obj)
      RequestChannel.transmit(&A);//tail at 0, A put to 0, A data implicitly copied over to array[0] obj, tail -> 1
      RequestChannel.transmit(&B);//tail at 1, B put to 1, B data implicitly copied over to array[1] obj, tail -> 2
      RequestChannel.pop();//tail -> 1, but B data still there, including le requestData[] which might longer than next received one
      //B length == 3; C length == 2; 
      // when copying, array[1].requestData before next transmit: [B][B][B], after transmit: [C][C][B]
      RequestChannel.transmit(&C);//tail at 1, C put to 1, C data implicitly copied over to array[1] obj, tail -> 2
      //array[1] is now ??9 , which were expected to hold a complete copy of C [C][C] instead of [C][C][B] if copy constructor were called to copy data
      
      //Conclude - in this ADT implementation, tailIndex just moving around array, and didn't call any destructor nor reset array,
      //    so it is going to cause memory-leak-alike behavior (dumb me overlook this scene)
      //Solution? - not sure, going to clear array upon retrieval instead of delete obj

}

result_of_implicit_copy_n_not_purging_element

Figure I:  ADT internal logging integrity for debug


In this (broken)ADT, elements are stored as `T elements[]` instead of an array of reference/pointer. If you were to implement something similar, whenever a retrieval/deletion is called, you should make sure that it clears the data on that index before returning (if it meant to). Or you might want to explore the =operator overriding option.

Enjoy coding.

 
Here's the revised version [ Request ] considering the latter option :
class Request
  {
private:

public:
         Request(void);
         Request(string imethod, string irequestUrl, string irequestHeaders, int itimeout, char &irequestData[], char &iresponse[], string iresponseHeaders);
         Request(const Request &);
         Request operator= (const Request &request){
            method = request.method;
            requestUrl = request.requestUrl;
            requestHeaders = request.requestHeaders;
            timeout = request.timeout;
            ArrayResize(requestData,ArraySize(request.requestData));
            ArrayCopy(requestData, request.requestData, 0, 0, ArraySize(request.requestData));
            ArrayCopy(response, request.response);
            responseHeaders = request.responseHeaders;
            return (this);
         };
        ~Request();
                    
                    
      //WebRequest("POST", requestUrl, requestHeaders, timeout, requestData, response, responseHeaders);
      string method;
      string requestUrl;
      string requestHeaders;
      int timeout;
      char requestData[];
      char response[];
      string responseHeaders;
      
      string toString(){
         return StringFormat("[Request]requestData:%s", CharArrayToString(requestData));
      }
      
      int send_request(){
         int code = WebRequest(method, requestUrl, requestHeaders, timeout, requestData, response, responseHeaders);
         if(code > 299 || code == -1){
            Print("Request - ",CharArrayToString(response));
            Print("Request - ",CharArrayToString(requestData));
         }
         return code;//HTTP status... -1 mql
      }
  };
//+------------------------------------------------------------------+
//|                                                                  |
//+------------------------------------------------------------------+
Request::Request()
  {
      ArrayInitialize(requestData, 0);
      ArrayInitialize(response, 0);
  }
Request::Request(string imethod, string irequestUrl, string irequestHeaders, int itimeout, char &irequestData[], char &iresponse[], string iresponseHeaders)
  {
      method = imethod;
      requestUrl = irequestUrl;
      requestHeaders = irequestHeaders;
      timeout = itimeout;
      ArrayResize(requestData,ArraySize(irequestData));
      ArrayCopy(requestData, irequestData, 0, 0, ArraySize(irequestData));
      ArrayCopy(response, iresponse);
      responseHeaders = iresponseHeaders;
  }
Request::Request(const Request &request)
  {
      method = request.method;
      requestUrl = request.requestUrl;
      requestHeaders = request.requestHeaders;
      timeout = request.timeout;
      ArrayResize(requestData,ArraySize(request.requestData));
      ArrayCopy(requestData, request.requestData, 0, 0, ArraySize(request.requestData));
      ArrayCopy(response, request.response);
      responseHeaders = request.responseHeaders;
  }
//+------------------------------------------------------------------+
//|                                                                  |
//+------------------------------------------------------------------+
Request::~Request()
  {
      ArrayFree(requestData);
  }
//+------------------------------------------------------------------+
12
New comment