Bug mql4 build 616 - memory leak reported but object is allocated once and freed once.

ydrol 2014.03.14 22:55

Raised with ServiceDesk - posted here for visibility (just in case someone is hunting down memory leaks).

If there is error in my test case please let me know :)

// Singleton Object reported leaked but is only allocated exactly once and deleted exactly once...
 
#property strict

class Singleton {
   static Singleton *INSTANCE;
 
   int dummyInt;
   
   // Private constructor
   Singleton() {
      Print(" constructor ",GetPointer(this));
   }
   //Private Destructor
   ~Singleton() {
      Print(" destructor ",GetPointer(this));
      INSTANCE = NULL;
   }
   public:
      static Singleton *getInstance() {
         if (INSTANCE == NULL) {
            INSTANCE = new Singleton();
            Print(" getInstance .. created",INSTANCE);
         }
         Print(" getInstance .. return",INSTANCE);
         return INSTANCE;
      }
      static void destroy() {
         
         Print(" destroy",INSTANCE);
         if (INSTANCE != NULL) {
          
            delete INSTANCE;
            INSTANCE = NULL;
         }
      }
};
// Initialize static value EDIT (see https://www.mql5.com/en/forum/150365 )
Singleton *Singleton::INSTANCE = NULL;
   
int OnInit()
  {
   Singleton::getInstance();
   return(INIT_SUCCEEDED);
  }

void OnDeinit(const int reason)
  {
   Singleton::destroy();
  }

void OnTick()
  {
  }

OUTPUT:

23:47:34 Expert static2 EURUSD,M1: loaded successfully
23:47:34 TestGenerator: spread set to 20
23:47:34 static2 test started
23:47:34 2013.07.10 00:00 static2 EURUSD,M1: constructor 1
23:47:34 2013.07.10 00:00 static2 EURUSD,M1: getInstance .. created1
23:47:34 2013.07.10 00:00 static2 EURUSD,M1: getInstance .. return1
23:47:34 2013.07.16 23:59 static2 EURUSD,M1: destroy1
23:47:34 2013.07.16 23:59 static2 EURUSD,M1: destructor 1
23:47:34 2013.07.16 23:59 static2 EURUSD,M1: 1 undeleted objects left
23:47:34 2013.07.16 23:59 static2 EURUSD,M1: 1 object of type Singleton left
23:47:34 2013.07.16 23:59 static2 EURUSD,M1: 20 bytes of leaked memory
23:47:34 EURUSD,M1: 397152 tick events (8181 bars, 398152 bar states) processed within 78 ms (total time 172 ms)

Ex Ovo Omnia 2014.03.15 05:37 #1

Remove the line INSTANCE = NULL; from the destructor.

Alain Verleyen 2014.03.15 06:48 #2

ydrol:

Raised with ServiceDesk - posted here for visibility (just in case someone is hunting down memory leaks).

If there is error in my test case please let me know :)

// Singleton Object reported leaked but is only allocated exactly once and deleted exactly once...
 
#property strict

class Singleton {
   static Singleton *INSTANCE;
 
   int dummyInt;
   
   // Private constructor
   Singleton() {
      Print(" constructor ",GetPointer(this));
   }
   //Private Destructor
   ~Singleton() {
      Print(" destructor ",GetPointer(this));
   }
   public:
      static Singleton *getInstance() {
         if (CheckPointer(INSTANCE) == POINTER_INVALID) {
            INSTANCE = new Singleton();
            Print(" getInstance .. created",INSTANCE);
         }
         Print(" getInstance .. return",INSTANCE);
         return INSTANCE;
      }
      static void destroy() {
         
         Print(" destroy",INSTANCE);
         if (CheckPointer(INSTANCE) == POINTER_DYNAMIC) {
          
            delete INSTANCE;
            INSTANCE = NULL;
         }
      }
};
// Initialize static value
Singleton *Singleton::INSTANCE = NULL;
   
int OnInit()
  {
   Singleton::getInstance();
   return(INIT_SUCCEEDED);
  }

void OnDeinit(const int reason)
  {
   Singleton::destroy();
  }

void OnTick()
  {
  }

ydrol 2014.03.15 17:48 #3

Thanks all, Both changes work, and are better code, But one 'new' followed by one 'delete' should not report leaked memory?

Both of these changes are just (correctly) avoiding the bug. Or is there something I am not understanding about this language?

To put it another way:

Please explain to me why my example is reporting leaked memory :) I'm happy to change my code so that this leakage is not reported, but I also want to understand the rules the the mql compiler is following if this is not a bug.

Econometrics: one step ahead Please Help Errors, bugs, questions

Alain Verleyen 2014.03.15 18:10 #4

ydrol:

Thanks all, Both changes work, and are better code, But one 'new' followed by one 'delete' should not report leaked memory?

Both of these changes are just (correctly) avoiding the bug. Or is there something I am not understanding about this language?

To put it another way:

You can't initialize a static variable inside a class.

The static member of a class can be explicitly initialized with a desired value. To do this, it must be defined and initialized at the global level.

So no INSTANCE = NULL in the destructor. Why that leads to a memory leak, only the developer of the compiler can answer I think.

ydrol 2014.03.15 18:12 #5

I didn't ?

// Initialize static value
Singleton *Singleton::INSTANCE = NULL;

Alain Verleyen 2014.03.15 18:24 #6

ydrol:

I didn't ?

From your code, initialization of a static inside a non-static method, isn't clear ?

   //Private Destructor
   ~Singleton() {
      Print(" destructor ",GetPointer(this));
      INSTANCE = NULL;
   }

EDIT :

By the way, this line is superfluous.

Singleton *Singleton::INSTANCE = NULL;

ydrol 2014.03.15 18:30 #7

From your code, initialization of a static inside a non-static method, isn't clear ?

Hmm, That is not 'initialising' - that is just changing the value. Changing the value of a static from inside a non-static method must surely be allowed?

ydrol 2014.03.15 18:32 #8

By the way, this line is superfluous.

Singleton *Singleton::INSTANCE = NULL;

Re EDIT nope - I was told by support that Statics must be initialized in the manner I did ...See this bug

ydrol 2014.03.15 18:41 #9

Regarding initialisation of statics... they must be initialized the way I did, if not you encounter this bug.

Support team have told me they will be changing the documentation because this is now wrong:

From MQL4 manual

It is not required to initialize static class members explicitly at the global level, they will be initialized automatically when you start the program. The static member of the integer and real type is automatically initialized with zero, to type string NULL is assigned. For static members of the complex type the default constructor is called; if there is none, then a constructor with default parameters is called.

Support team said (regarding static initialization bug - not this leak report bug) . bits in [..] added for clarity

ydrol 2014.03.13 17:08
Ah my apologies. That does work. [ Adding...
int StaticInt::v=0;
] So the documentation needs to be changed. And every static member needs to be initialized. [Even if to 0 or NULL]
Support Team 2014.03.14 10:03

We will change the documentation
...

ydrol 2014.03.13 17:31
For my first example I think the MQL compiler should warn the value is not initialized etc.
Support Team 2014.03.14 10:03
[compiler] Warning added, please wait for updates.

Note the above is nothing (directly) to do with the false memory leak report bug discussed in this thread :)

So IMO there is a compiler bug, I've reported it, I've had some 'workarounds' posted here, which are nicer code,, but there is still an underlying bug that needs to be fixed? lets see what happens.

Asynchronous loading of charts Errors, bugs, questions The new build does

Alain Verleyen 2014.03.15 18:54 #10

ydrol:
Re EDIT nope - I was told by support that Statics must be initialized in the manner I did ... I'll paste the info here to avoid confusion ...

I am aware of your other topics. A static need to be initialized this way or within a static method, try it you will see. There is no bug.

1 2

New comment