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

New comment
 

A static need to be initialized this way or within a static method, try it you will see.

No worries, but if you look at the other topic, the bug there, is that initialization by static method does not work, if that method is called before OnInit() - ie called from the default constructor of another Object that is defined globally. It has to be initialised explicitly first, and the next release of the compiler will hopefully have updated documentation and issue a warning if we just try to initialize via static method.

I'm not entirely sure how that's going to work but I'll wait and see :)

These might all seem like edge cases at present but once people start churning out lots of OOP MQL4, it will happen a lot, IMO. - however next compiler should have a fix for that one.

 
ydrol:

No worries, but if you look at the other topic, the bug there, is that initialization by static method does not work, if that method is called before OnInit() - ie called from the constructor of another Object. It has to be initialised explicitly first, and the next release of the compiler will hopefully have updated documentation and issue a warning if we just try to initialize via static method.

I'm not entirely sure how that's going to work but I'll wait and see :)

Ok, I will wait and see with you.

 

You exchanged the reference to the object - which you are destroying - before the destructor finishes. At the end of destructor run there is no pointer to the allocated memory in the variable you "delete".

It could or couldn't work, no one can learn until he tries, but generally it is very poor design to replace reference to self from the referenced object itself.

 

Ovo I hear you partly, but

1. I wouldn't say very poor just not very good :)

2. The delete is called before the destructor. It really shouldn't matter, nor cause the compiler problem. IMO INSTANCE is just a variable. If you follow the steps through in sequence it is quite well defined in expected behaviour.

 

I think I know what is happening. The destructor is clearing a reference to itself, and this is somehow messing up the 'leak report'.

Clearing reference to self should not cause undefined behaviour.

I'll try to post up a simpler example ... still a bug IMO. Not serious. and code that shows 'phantom' leak could be improved ! or 'very poor' as some would say :)

 
ydrol:

I think I know what is happening. The destructor is clearing a reference to itself, and this is somehow messing up the 'leak report'.

Clearing reference to self should not cause undefined behaviour.

I'll try to post up a simpler example ... still a bug IMO. Not serious. and code that shows 'phantom' leak could be improved ! or 'very poor' as some would say :)

Looking forward to the simple example.

 
Ovo:

Looking forward to the simple example.


Apologies if I'm wrong, but I question the motivation :) Against my better judgement here is code that is simpler (but not much).

Not really much point in discussing this further in this manner. The code is crap, it confuses the leak report in the MQL4 compiler.

I'd like to see the leak report fixed and the mql4 compiler become a little bit better as a tool, you'd rather I write better code, which I will do henceforth!

#property strict
// This is not proper code to be used in anything. It is code to demonstrate a 'buggette'
// The bug is that the MQL4 compiler is reporting leaked memory when it hasnt leaked.
// It manifest when the destructor clears the static variable which is the same as was
// passed to the 'delete command'
// This is not good code style. It doesnt matter. The leak report is wrong regardless.
class MyClass {

public:
    static MyClass* INSTANCE;

    MyClass() { 
      Print("Constructor",GetPointer(this));
    }

    ~MyClass() { 
      Print("Destructor",GetPointer(this));
      // The following code is bad, naughty, very poor and probably kills kittens.
      // Still it does fool the compiler into reporting leaked memory.
      // When both constructor and destructor have been called exactly one.
      INSTANCE = NULL; 
    }
    
 public:

};

MyClass *MyClass::INSTANCE = NULL;


int OnInit()
  {
     
      MyClass::INSTANCE = new MyClass();
      // Calliing the destructor with the static variable here.
      delete MyClass::INSTANCE;
      // Note bug does not appear if I say
      // MyClass *x = MyClass::INSTANCE ; delete x; 
            
      return(INIT_SUCCEEDED);

}
void OnDeinit(const int reason)
  {

  }

void OnTick()
  {

  }
 

I use the OOP feature of MQL a lot, and I have to repeat that I noticed no real bug in the OOP implementation. There are many other bugs in other parts of MQL, probably most of them can be found in the graphical objects part. So if you want report something useful to the service desk, try to test the new graphics, it is the crap. The OOP implementation seems to work properly.

 

I don't use graphics so it's not important to me. One report Ive posted, has resulted in correction to documentation and a new warning to be added next release. (so there was something there even if it is of no importance to you).

12
New comment
Reason: