NT making a bogus call on my indicator

[ETFVoyageur]

Well, I may not be able to reliably reproduce it any more, but it is still happening. I have ruled out one more thing, though. I switched back to compiling in showing both lines and restarted when the problem happened again. At least we rule out the hypothesis that the issue has to do with fewer plots than getters. This also rules out accumulated random corruption. If there is a corruption problem, it has gotten re-corrupted pretty quickly.

Here is the trace. You have seen it before, so I did not bother to annotate it this time. The only difference is that the NT internal error is different -- this time it is OnCalculateMinMax(). Perhaps what we are seeing is garbage collection vagaries, and the internal code gets varying distances before some function encounters the problem.

08/05/15  #1  Exit:   VsaModelIndicator.DoStateChange()  (OnStateChange(Realtime) completed, State.Realtime)  CurrentBar=16501   ^SPX (Daily)
01/03/50  #1  Entry: VsaModelIndicator.DoStateChange()  (OnStateChange(Terminated) started, State.Terminated)  CurrentBar=16501   ^SPX (Daily)
01/03/50  [B]#1[/B]  Exit:   VsaModelIndicator.DoStateChange()  (OnStateChange([B]Terminated[/B]) completed, [B]State.Terminated[/B])  CurrentBar=16501   ^SPX (Daily)
[B]Indicator 'VsaModelIndicator': Error on calling 'OnCalculateMinMax' method on bar -1: Index was outside the bounds of the array.

[/B]// F5 chart refresh here

00:03:14  #2  Entry: VsaModelIndicator.VsaModelIndicator()  (Constructor started , State.SetDefaults)  CurrentBar=n/a   <no instrument>
00:03:14  #2  Exit:   VsaModelIndicator.VsaModelIndicator()  (Constructor completed, State.SetDefaults)  CurrentBar=n/a   <no instrument>
00:03:14  #2  Entry: VsaModelIndicator.DoStateChange()  (OnStateChange(SetDefaults) started, State.SetDefaults)  CurrentBar=n/a   <no instrument>
00:03:14  #2  Exit:   VsaModelIndicator.DoStateChange()  (OnStateChange(SetDefaults) completed, State.SetDefaults)  CurrentBar=n/a   <no instrument>
00:03:14  #1  Entry: VsaModelIndicator.[B]Clone()[/B]  (OnStateChange([B]Terminated[/B]) completed, [B]State.Finalized[/B])  CurrentBar=n/a   ^SPX (Daily)
                                    [B]Cloning object #2 from object #1.  State=Finalized[/B]
00:03:14  #1  Exit:   VsaModelIndicator.Clone()  (OnStateChange(Terminated) completed, State.Finalized)  CurrentBar=n/a   ^SPX (Daily)
00:03:14  #2  Entry: VsaModelIndicator.DoStateChange()  (OnStateChange(Configure) started, State.Configure)  CurrentBar=n/a   ^SPX (Daily)
00:03:14  #2  Exit:   VsaModelIndicator.DoStateChange()  (OnStateChange(Configure) completed, State.Configure)  CurrentBar=n/a   ^SPX (Daily)
00:03:14  #2  Entry: VsaModelIndicator.DoStateChange()  (OnStateChange(DataLoaded) started, State.DataLoaded)  CurrentBar=n/a   ^SPX (Daily)

It is clear to me that NT really does have a problem that can cause the instance to get destroyed prematurely, and then NT continues to use the destroyed instance. I'll try to do what I can to help with this, but I am not sure how much help I can be until I know better how to reproduce it. Is there something else I could be including in my trace printing that would be helpful?

(FWIW: I always leave my trace printing on while in debug mode. That way, when something happens I can look t the trace if that is helpful.)

--EV

[ETFVoyageur]

I have to wonder whether or not this is related to the autoscale including lines (sometimes) oddity I have reported in a recent thread. I wonder about that because both threads are talking about the same indicator and the internal error this thread reports is in OnCalculateMinMax().

--EV

[NinjaTrader_Matthew]

Hmm, I guess I still don't see what the problem is. You've had a few hypotheses and as I understand you feel the object is being destroyed too early and then it being cloned after it is terminated.

If an indicator runs into an exception, it's state is set internally to finalized. There is logic to copy the input from the original instance, which is why I believe clone is being called, but who knows you could be seeing behavior that I'm missing. If you can just clearly define the problem scenario and why this is a problem I'll be happy to debug further to clarify what I can.

[ETFVoyageur]

Hmm, I guess I still don't see what the problem is. You've had a few hypotheses and as I understand you feel the object is being destroyed too early and then it being cloned after it is terminated.

Excuse me? You do not see there is a problem when I show you that:

• The instance is getting destroyed prematurely
• Output from NT shows an internal failure in calls to things like OnRender() or OnCalculateMinMax() after the instance is terminated
• As a result, the indicator does not get drawn.
• Refreshing the chart results in Clone() is being called when State is State.Finalized

If an indicator runs into an exception, it's state is set internally to finalized.

NT is doing more than just "setting the state to finalized". It is first calling OnStateChange() with State==State.Terminated. It really is destroying the object before setting the state to Finalized. It is not just setting a flag.

There is logic to copy the input from the original instance, which is why I believe clone is being called,

We agree that is why Clone() is being called. The problem is that by that point there is no valid object to call Clone() on. The one-and-only instance that has been created has already been destroyed. Calling Clone() on that already-destroyed object is a serous bug.

but who knows you could be seeing behavior that I'm missing. If you can just clearly define the problem scenario and why this is a problem I'll be happy to debug further to clarify what I can.

I do understand that the most effective way to address finding the problem is for me to be able to reliably reproduce it, and I am trying to do so. I would expect that I must be doing something that NT does not expect (rightly or wrongly).

The fact remains that, no matter what my indicator is doing, it is always wrong (a bug) for NT to be calling methods on a destroyed object. There is no such thing as correct code calling a method on a destroyed object. The fact that Clone() gets called when State==State.Finalized should never happen.

The question I asked is whether there is anything I could be showing in my tracing that would help either of us better understand what is happening. I think that is a fair question, although the answer may be that there is nothing.

Let me add one more question: please explain why it is ever valid for NT to call Clone() when the object's State is State.Finalized. I assert that doing so is, in itself, a bug (no matter what the indicator is doing).

-- EV

[koganam]

...

Let me add one more question: please explain why it is ever valid for NT to call Clone() when the object's State is State.Finalized. I assert that doing so is, in itself, a bug (no matter what the indicator is doing).

-- EV

Could be a threading issue that is not being properly accounted for? Just speculating.

[NinjaTrader_Matthew]

Maybe to help clarify, try just throwing some exception which I believe should help reproduce the behavior you're seeing, without the original issue that spawned this discussion

if(State == State.Realtime)
	throw new Exception("Some problem occurred");

[ETFVoyageur]

Maybe to help clarify, try just throwing some exception which I believe should help reproduce the behavior you're seeing, without the original issue that spawned this discussion

if(State == State.Realtime)
    throw new Exception("Some problem occurred");

Thank you for the constructive suggestion. That, in fact, does it! I have attached a wizard-generated indicator in which I have made minor changes. Any line I changed is marked with "/* EV */. The steps to produce the problem are:

1. Bring up a chart with a security and no indicators
2. Add the indicator (VsaNew).
3. Click "OK". NT prints a message about the exception and the indicator does not get drawn. So far, so good.
4. F5 chart refresh. NT calls Clone() with State.Finalized. That is the bug.

Here is what shows up on the output (comment lines are my annotation):

// Bring up the Indicators dialog.  VsaModelIndicator tracing is normal, and irrelevant to this test
12:00:55  #1  Entry: VsaModelIndicator.VsaModelIndicator()  (Constructor started , State.SetDefaults)  CurrentBar=n/a   <no instrument>
12:00:55  #1  Exit:   VsaModelIndicator.VsaModelIndicator()  (Constructor completed, State.SetDefaults)  CurrentBar=n/a   <no instrument>
12:00:55  #1  Entry: VsaModelIndicator.DoStateChange()  (OnStateChange(SetDefaults) started, State.SetDefaults)  CurrentBar=n/a   <no instrument>
12:00:55  #1  Exit:   VsaModelIndicator.DoStateChange()  (OnStateChange(SetDefaults) completed, State.SetDefaults)  CurrentBar=n/a   <no instrument>

// Double-click VsaNew (the test indicator) clones the instance used to initially populate the dialog
// This is normal and correct.  So far, so good.
CLONE() CALLED    (State = State.SetDefaults)

// Click "OK"
// All is OK so far.    VsaModelIndicator tracing is normal, and irrelevant to this test
12:01:52  #1  Entry: VsaModelIndicator.DoStateChange()  (OnStateChange(Terminated) started, State.Terminated)  CurrentBar=n/a   <no instrument>
12:01:52  #1  Exit:   VsaModelIndicator.DoStateChange()  (OnStateChange(Terminated) completed, State.Terminated)  CurrentBar=n/a   <no instrument>

// Clone() called to make a new instance for the chart.  This is normal and OK.  So far, so good
CLONE() CALLED    (State = State.SetDefaults)

// This is the artificial exception, thrown from State.DataLoaded
// The indicator's panel appears in the chart, but no indicator is drawn (expected behavior, given the exception)
Indicator 'VsaNew': Error on calling 'OnStateChange' method: Exception to test how NT handles it

// F5 to refresh the chart
// Resulted in Clone() being called with State.Finalized. 
// THIS IS THE BUG.  THIS SHOULD NEVER HAPPEN.
[B]CLONE() CALLED    (State = State.Finalized)[/B]
Indicator 'VsaNew': Error on calling 'OnStateChange' method: Exception to test how NT handles it

Thanks for the suggestion about how to reproduce the problem.


--EV

[NinjaTrader_Matthew]

Great, glad that worked and thanks for putting that together.

When the State is Terminated/Finalized, the object instance is not destroyed. It is not destroyed until the object is cloned. This is expected.

When your indicators runs into an error, it will call SetState(State.Finalized) and log the exception. Your indicator iterates through State.Terminated which is a framework model for you to clean up your device resources. The indicator instance then exists on the chart in State.Finalized with its instrument waiting for user to do something holding the reference to be cloned. There is nothing rendered since it is Finalized, but there is the original instance holding the instrument. When you F5 your indicator, the indicators input is cloned to the new instance, the old instance is removed, and new instance is loaded

It's the exception that was causing the indicator to go terminated/finalized. The exception was not a result attempting to call clone on an instance that doesn't exist (which should never happen).

[ETFVoyageur]

Great, glad that worked and thanks for putting that together.

When the State is Terminated/Finalized, the object instance is not destroyed. It is not destroyed until the object is cloned. This is expected.

When your indicators runs into an error, it will call SetState(State.Finalized) and log the exception. Your indicator iterates through State.Terminated which is a framework model for you to clean up your device resources. The indicator instance then exists on the chart in State.Finalized with its instrument waiting for user to do something holding the reference to be cloned. There is nothing rendered since it is Finalized, but there is the original instance holding the instrument. When you F5 your indicator, the indicators input is cloned to the new instance, the old instance is removed, and new instance is loaded

It's the exception that was causing the indicator to go terminated/finalized. The exception was not a result attempting to call clone on an instance that doesn't exist (which should never happen).

It seems to me that you are describing a design that has a logical problem. The problem I see is that Clone() is getting called after Terminate has been called. There is no telling what Terminate has done. In particular, it may well have destroyed things that Clone() refers to or should be passing along. The instance may not have technically been destroyed, as in the destructor or Dispose may not have been called, but it is functionally destroyed. As far as I can see, NT calling Terminate() before it calls Clone() is a bug.

The other thing I have found, in my real indicator, is that when Clone() is called in this circumstance some base objects may have already been destroyed. Specifically, any attempt to reference CurrentBar threw an exception.

Finally, as my previous traces have shown, NT gets internal errors (OnCalculateMinMax(), OnRender() when this situation happens. Perhaps that is expected, but it sure looks less than desirable to see their error output mixed in with my tracing.

--EV

[ETFVoyageur]

A brief note, in case I was not clear:

• In the normal case, Terminate is not called until after Clone() has been called
• In the error case, Terminate is called before Clone() is called.

The error case is a bug. Once Terminate has been called the object is functionally destroyed. There is no reason to believe the things that Clone() needs to clone still even exist.


--EV

[ETFVoyageur]

It's the exception that was causing the indicator to go terminated/finalized. The exception was not a result attempting to call clone on an instance that doesn't exist (which should never happen).

We are talking about two different exceptions.

• I can believe there was an exception somewhere that got the indicator into this state.
• The exception I am talking about is subsequent, though. When Clone() is called on a destroyed object it was getting an exception when trying to reference CurrentBar -- something that should have been just fine if the object was truly still live and not at least partially destroyed.

That is an example of what I have said -- the object may not have been destroyed in the C# sense (no destructor called), but it is functionally destroyed once Terminate() is called. Judging by access to CurrentBar throwing, that is true in the base object as well as in the derived indicator's part. Inspecting the object in VS showed a whole host of things that threw when accessed (VS shows that).

I continue to believe that calling Clone() after calling Terminate is a system error. This only happens in the error-handling case we are discussing. In normal (success) cases Terminate is called after Clone(), as it should be.

--EV

[ETFVoyageur]

When your indicators runs into an error, it will call SetState(State.Finalized) and log the exception. Your indicator iterates through State.Terminated which is a framework model for you to clean up your device resources. The indicator instance then exists on the chart in State.Finalized ...

I took that to mean that to mean that my indicator would get a call to OnStateChange()) with State==State.Finalized. It does not get such a call. Should it? Or are you just saying that the indicator's state will be set to State.Finalized after OnStateChange(State.Terminated) returns?

Also, now that I have looked at State, I see that there is a State.Undefined. What is that for? Will my indicator ever see it?

Seeing that State.Finalized and State.Undefined are not mentioned in the documentation makes me think that they are for base class use, and that OnStateChange() will never get them.. Is that right?

--EV

[NinjaTrader_DaveI]

Your indicator should not see State.Undefined -- this is something only used for internal purposes.

The core of this issue stems from the fact that SetState(State.Finalized) is immediately called when an exception is thrown, and a wide range of items are reset within State.Finalized. We do this intentionally for several reasons, such as the fact that it is not known whether a specific property or variable within the indicator actually caused the exception, or the fact that some resources cannot be shared between instances. This is done before the call to Clone(), which causes the new instance to contain default values.

There is a potential solution to work around this to maintain property/variable values after the exception. Since State.Terminated is called before State.Finalized, you have an opportunity there to manually save any state you wish within State.Terminated.

You can then override the Clone() method to pull your saved state into the new instance. So you have saved the state before it is reset, then "restored" it after the Clone(). This will be the key to bringing state over from before the exception.

[ETFVoyageur]

Your indicator should not see State.Undefined -- this is something only used for internal purposes.

The core of this issue stems from the fact that SetState(State.Finalized) is immediately called when an exception is thrown,

As I noted in an earlier post, my indicator never gets called whith OnStateChange() State.Finalized. Is that expected behavior? Should the indicator get that call, or is the expected behavior to just call Terminated and then set the state to State.Finalized after Terminated has returned?

and a wide range of items are reset within State.Finalized. We do this intentionally for several reasons, such as the fact that it is not known whether a specific property or variable within the indicator actually caused the exception, or the fact that some resources cannot be shared between instances. This is done before the call to Clone(), which causes the new instance to contain default values.

I presume that "which causes the new instance to contain default values" refers to those things in the Indicator base that have been reset.? "the fact that some resources cannot be shared between instances" sound unrelated to this issue -- it sounds like something that happens in all cases before calling Clone().

There is a potential solution to work around this to maintain property/variable values after the exception. Since State.Terminated is called before State.Finalized, you have an opportunity there to manually save any state you wish within State.Terminated.

Terminated's main purpose is to delete certain things. The documentation suggests "Use to clean up/dispose of resources". Such things will obviously no longer be available to Clone(). I gather your suggestion is that behavior could be augmented as I outline below.

You can then override the Clone() method to pull your saved state into the new instance. So you have saved the state before it is reset, then "restored" it after the Clone(). This will be the key to bringing state over from before the exception.

The bottom line is that you argue that Clone() is called on an object that is already at least partially destroyed, because it is unsafe to not at least partially partially destroy the object before calling Clone(). I say "destroyed" because the base class has reset it internals, meaning it is unsafe to access them (attempted access may throw) and unreliable even if safe (values have been reset to default values) and the indicator's Terminate has run, which will have destroyed appropriate items.

It looks to me that the consequence of all this is:

• The indicator needs to coordinate between its Clone() method and its Terminated code -- perhaps a bool HasBeenCloned flag
• In Terminated, if HasBeenCloned is false, squirrel away any base class information that Clone(), or anything Clone() calls, will want. Also, if HasBeenCloned is false then Terminated should not do its normal actions, so those things will still be there for Clone().
• In Clone() check to see whether State == Finalized. If so, use the squirreled away values instead of trying to access the base class for them, and do the normal Terminated actions.

The above workaround/hack can be done, at the cost of complicating the indicator code, and I guess that is what will have to be done. You need to adjust your documentation to point out that Clone() can be called on an instance that has already been logically destroyed.
=====

It seems to me that there is no benefit to the current scheme, and some downside, as compared to having another state (call it Limbo) that is set when NT gets the exception. The everything could proceed normally -- call Clone(), which would need to be careful if in Limbo state, followed by Terminated etc. It seems to me that would be a more accurate representation of what is going on.
=====

I understand the explanation you gave. That explanation fails to explain why I see errors from NT code OnRender() or OnCalculateMinMax() after Terminated has been called. Those look to me like there is an NT bug even in light of the explanation you give.
=====

In any case, thanks for the explanation. Given your explanation, including that it is normal for Clone() to be getting called on already logically destroyed instances (in the error case) I can work with that.

--EV