Download MetaTrader 5

OnDeinit() race condition and recalculation issues

To add comments, please log in or register
Order a robot or an indicator for MetaTrader. The Freelance service will help you!
Andre Enger
651
Andre Enger 2016.11.10 11:04 

Hello,

I am writing an indicator which draws certain objects in the chart. Upon removing the indicator (or changing timeframes or input settings) an OnDeinit() procedure is therefore called to clean up resources by - among freeing arrays and nullifying variables etc. - also remove the drawn objects from the chart. A stripped down code example for illustration is found below:

//+------------------------------------------------------------------+
//|                                                          Bug.mq5 |
//|                        Copyright 2016, MetaQuotes Software Corp. |
//|                                             https://www.mql5.com |
//+------------------------------------------------------------------+
#property copyright "Copyright 2016, MetaQuotes Software Corp."
#property link      "https://www.mql5.com"
#property version   "1.00"
#property indicator_chart_window


//+------------------------------------------------------------------+
//| Custom indicator initialization function                         |
//+------------------------------------------------------------------+
int randomNumber;
int OnInit()
  {
//--- indicator buffers mapping
  MathSrand(GetTickCount());
  randomNumber=MathRand();//TimeCurrent();
//---
   return(INIT_SUCCEEDED);
  }
void OnDeinit(const int reason)
  {
   for(int i=ObjectsTotal(0,0,-1)-1; i>=0; i--)
     {
      string name=ObjectName(0,i,0,-1);
      if(StringFind(name,"My Trendline "+IntegerToString(randomNumber))!=-1)
         ObjectDelete(0,name);
     }
  }
//+------------------------------------------------------------------+
//| Custom indicator iteration function                              |
//+------------------------------------------------------------------+
int OnCalculate(const int rates_total,
                const int prev_calculated,
                const datetime &time[],
                const double &open[],
                const double &high[],
                const double &low[],
                const double &close[],
                const long &tick_volume[],
                const long &volume[],
                const int &spread[])
  {
//---
   if (rates_total < 10)
      return(rates_total);
   for (int i = MathMax(0, rates_total - 500); i < rates_total; i++)
   {
      ObjectCreate(0, "My Trendline "+IntegerToString(randomNumber) + i, OBJ_TREND, 0, time[rates_total - 5], close[rates_total - 5], time[rates_total - 2], close[rates_total - 2]);
      ObjectSetInteger(0,"My Trendline "+IntegerToString(randomNumber) + i,OBJPROP_SELECTABLE, true);
      ObjectSetString(0,"My Trendline "+IntegerToString(randomNumber) + i, OBJPROP_TOOLTIP, "Description");
   }
   return(rates_total);
  }
//+------------------------------------------------------------------+

If you run the indicator above you will see a red trendline on the most recent bars, the purpose is simply to illustrate the problem. Should you choose to remove the indicator or change timeframe, the currently drawn trendlines are removed purposefully.

The problem with my indicator is that race conditions can occur when changing timeframe, such that the OnDeinit() call is called from the "old" indicator instance after the "new" indicator instance has already drawn. This leads to the erroneous behavior that the indicator does not always work smoothly when changing timeframe. To alleviate the problem I thought adding a random number to the names of the objects an indicator creates would be a proper solution, hence the 

+IntegerToString(randomNumber)

part. However, even though this works most of the time, it turns out that if I restart the terminal, the objects from beforehand are no longer deleted. This can be tried of course by using the above code.

So I am wondering what to do, is this a bug in the Metatrader platform? would it not be best to ensure that race conditions on the OnDeinit() procedure does not occur in the first place? And is there other, proper solutions for this problem?

Thanks

Marco vd Heijden
Moderator
4704
Marco vd Heijden 2016.11.10 12:05  

Why don't you try

ObjectsDeleteAll()
In OnDeinit() function.
eevviill6
27
eevviill6 2016.11.10 12:06  

Did you chek  void OnDeinit(const int reason)

reason? 

 

I use construction like this. Create objects with "identif" in name.

string identif="my ind 45";

...

string name_delete;
for(int i=ObjectsTotal()-1;i>=0;i--)
{
name_delete=ObjectName(i);
if(StringFind(name_delete,identif)!=-1) ObjectDelete(name_delete);
}


 

Stanislav Korotky
17925
Stanislav Korotky 2016.11.10 12:12  
Andre Enger
651
Andre Enger 2016.11.10 12:24  

A fix did exist, as the problem is:

  • Deleting all objects in OnDeinit identified by a static identifier -> Error on race condition (deletes objects for next instance)
  • Deleting all objects in OnDeinit identified by a static identifier + an instance specific random identifier -> Error on restart (the instance specific random identifier is wiped from memory)

So if one deletes all objects by the static identifier only in the OnInit() procedure, the chart is cleaned upon a terminal restart. So the solution is

  • Deleting all objects in OnInit identified by a static identifier -> Clears chart when memory is cleared
  • Deleting all objects in OnDeinit identified by a static identifier + an instance specific random identifier -> Clears chart when timeframe or settings are changed
int OnInit()
  {
//--- indicator buffers mapping
   for(int i=ObjectsTotal(0,0,-1)-1; i>=0; i--)
     {
      string name=ObjectName(0,i,0,-1);
      if(StringFind(name,"My Trendline ")!=-1)
         ObjectDelete(0,name);
     }

  MathSrand(GetTickCount());
  randomNumber=MathRand();//TimeCurrent();
//---
   return(INIT_SUCCEEDED);
  }
void OnDeinit(const int reason)
  {
   for(int i=ObjectsTotal(0,0,-1)-1; i>=0; i--)
     {
      string name=ObjectName(0,i,0,-1);
      if(StringFind(name,"My Trendline "+IntegerToString(randomNumber))!=-1)
         ObjectDelete(0,name);
     }
  }
Andre Enger
651
Andre Enger 2016.11.10 12:30  
Marco vd Heijden:

Why don't you try

ObjectsDeleteAll()
In OnDeinit() function.

Because that would delete objects drawn manually or by other indicators also, which is undesirable.

Edit: BTW. it would in any case also lead to a possible race condition.

Alain Verleyen
Moderator
30743
Alain Verleyen 2016.11.10 15:23  
Andre Enger:
The problem with my indicator is that race conditions can occur when changing timeframe, such that the OnDeinit() call is called from the "old" indicator instance after the "new" indicator instance has already drawn.

What race conditions are you talking about ?

If you change timeframe or settings, OnDeinit() is called BEFORE OnInit() with the new settings/timeframe. I tried your indicator and doesn't have any problem. (MT5 build 1455).

Andre Enger:

Because that would delete objects drawn manually or by other indicators also, which is undesirable.

Edit: BTW. it would in any case also lead to a possible race condition.

Use it properly in place of your loop with StringFind() :

     ObjectsDeleteAll(0,"My Trendline ",0);
Andre Enger
651
Andre Enger 2016.11.10 16:43  
Alain Verleyen:

What race conditions are you talking about ?

If you change timeframe or settings, OnDeinit() is called BEFORE OnInit() with the new settings/timeframe. I tried your indicator and doesn't have any problem. (MT5 build 1455).

Use it properly in place of your loop with StringFind() :

     ObjectsDeleteAll(0,"My Trendline ",0);

The race condition is a bit hard to obtain, but just setting the number of trendlines to 5000 (from 500 in the i=MathMax(...)) and adding printf statements at the entry and exits of the oncalculate and ondeinit methods gave me the following on the same build, multicore CPU:

DS    0    16:37:24.706    Bug (EURUSD,H1)    Drawing done:
EH    0    16:37:25.294    Bug (EURUSD,H1)    Drawing:
OO    0    16:37:25.319    Bug (EURUSD,H1)    Drawing done:
HG    0    16:37:25.699    Bug (EURUSD,H1)    Drawing:
HK    0    16:37:25.734    Bug (EURUSD,H1)    Drawing done:
CP    0    16:37:26.716    Bug (EURUSD,H12)    Drawing:
FD    0    16:37:26.766    Bug (EURUSD,H12)    Drawing done:
DQ    0    16:37:26.782    Bug (EURUSD,H1)    Deinitializing:

EJ    0    16:37:27.304    Bug (EURUSD,H1)    Deinitializing done:
EJ    0    16:37:27.304    Bug (EURUSD,H12)    Drawing:
QR    0    16:37:27.326    Bug (EURUSD,H12)    Drawing done:
CF    0    16:37:27.326    Bug (EURUSD,H12)    Drawing:
HN    0    16:37:27.349    Bug (EURUSD,H12)    Drawing done:

So clearly here the new timeframe was started before the OnDeinit().

I was not aware of the ObjectsDeleteAll() overload that accepts a filtering substring.

Marco vd Heijden
Moderator
4704
Marco vd Heijden 2016.11.10 19:49  

Care to explain why you would need 5000 trendlines?

Drawing, as well as running mass printing will take some time to process.

Of course you can always increase the number until it crashes but do you really need that many ?

Stanislav Korotky
17925
Stanislav Korotky 2016.11.10 19:59  
Andre Enger:


I was not aware of the ObjectsDeleteAll() overload that accepts a filtering substring.

I tried ObjectsDeleteAll() just after the moment it has been added to the platform, and it was buggy, so I still prefer to use explicit loop through the objects as you did. Probably this all is related to asynchronous nature of event processing in the terminal: so calling the functions does not mean the objects will be deleted immediately during its execution. Instead the deletion is queued and processed after a while. I think this is bad, because may lead to problems you've encountered, this is why I suggested to write to the service desk, just to get some feedback from MQ. BTW, you could probably wait for actual object deletion in OnDeinit, that is call the function ObjectDelete and then loop until your objects can not be found anymore. The script have approx 3 seconds before it will be unloaded forcedly, but I think the objects should be wiped out in a second or so, so terminal situation will not occur.
Alain Verleyen
Moderator
30743
Alain Verleyen 2016.11.10 22:02  
Andre Enger:

...

So clearly here the new timeframe was started before the OnDeinit().

You are right, I was not aware of such things. BUT...

I was not aware of the ObjectsDeleteAll() overload that accepts a filtering substring.

However if you are using the best option available which is ObjectsDeleteAll() with a prefix, it doesn't happen !

2016.11.10 21:49:49.304    160715 (EURUSD,M5)    OnCalculate start
2016.11.10 21:49:49.321    160715 (EURUSD,M5)    OnCalculate end
2016.11.10 21:49:51.271    160715 (EURUSD,M5)    OnDeinit start
2016.11.10 21:49:51.281    160715 (EURUSD,M5)    OnDeinit end. 5000 Objects deleted.
2016.11.10 21:49:51.285    160715 (EURUSD,H1)    OnInit start
2016.11.10 21:49:51.285    160715 (EURUSD,H1)    OnInit end
2016.11.10 21:49:51.285    160715 (EURUSD,H1)    OnCalculate start
2016.11.10 21:49:51.312    160715 (EURUSD,H1)    OnCalculate end

5,000 objects deleted in 10 ms.

While with a loop (your original code), it takes 320 ms. (while the race condition doesn't always occurred).

2016.11.10 21:54:16.457    160715 (EURUSD,M5)    OnDeinit start
2016.11.10 21:54:16.778    160715 (EURUSD,M5)    OnDeinit end. 5000 Objects deleted.

32 x faster.

123
To add comments, please log in or register