NT8 sometimes runs OnStateChange termination during OnBarUpdate

[QuantKey_Bruce]

In the beta phase, I had posted a thread on this issue at http://ninjatrader.com/support/forum...ad.php?t=83542 with some context. It seems that NT8 NinjaScript runs OnStateChange to change the state to State.Terminated concurrently with execution of OnBarUpdate.

I have updated my demonstration indicator to use a Print statement rather than throwing an exception, because it now seems that throwing an exception during the StateChange to State.Terminated can hang the chart indefinitely.

Here's the demonstration code:

public class TestPrematureTermination : Indicator
{
private bool IsUpdating = false;

protected override void OnStateChange()
{
if (State == State.SetDefaults)
{
Description = @"Test premature termination issue";
Name = "TestPrematureTermination";
}
else if (State == State.Terminated)
{
if (IsUpdating) Print("Broken");
}
}

protected override void OnBarUpdate()
{
IsUpdating = true;
System.Threading.Thread.Sleep(1);
IsUpdating = false;
}
}

If you simply place this on a chart - for instance, 30 days of 1 minute bars of ES - and hit F5 a few times, you will see the message Broken in the output window, indicating that the unmanaged resources are being destroyed during OnBarUpdate's execution.

Because the stated intention is for us to clean up unmanaged resources during OnStateChange to State.Terminated, this results in exceptions during the execution of OnBarUpdate which relies on those same resources. Shouldn't NT8 wait for OnBarUpdate to exit before changing the state to State.Terminated?

The alternative seems to be that we introduce our own object locking, but this has its own issues.

Please confirm that this is what you expect us to do, in order to avoid exceptions happening that can lock the chart.

[QuantKey_Bruce]

For clarity, here is an example of what I mean when I say "introduce our own object locking." Please let me know if this is the approach NT advocates to avoid the issue described above.

This approach appears to sidestep the problem described in this thread (and "Broken" is no longer printed unlike the demonstration code below), but it is unclear if there are other threading issues introduced by managing our own locking in this way which is why I would like NT to confirm the best approach to mitigating this risk.

[koganam]

For clarity, here is an example of what I mean when I say "introduce our own object locking." Please let me know if this is the approach NT advocates to avoid the issue described above.

This approach appears to sidestep the problem described in this thread (and "Broken" is no longer printed unlike the demonstration code below), but it is unclear if there are other threading issues introduced by managing our own locking in this way which is why I would like NT to confirm the best approach to mitigating this risk.

ref: http://ninjatrader.com/support/helpG...fecycle_of.htm

and a little context discussion: http://ninjatrader.com/support/forum...ad.php?t=83608

[QuantKey_Bruce]

ref: http://ninjatrader.com/support/helpG...fecycle_of.htm

and a little context discussion: http://ninjatrader.com/support/forum...ad.php?t=83608

Thanks. Yes, I read that. I don't see, though, how one can manage the situation better than doing locking as in my little reference sample there. Is there a better way?

[NinjaTrader_ChelseaB]

Hello Bruce DeVault,

Adding a sleep to a data event driven method (or any NS method) is not supported by NinjaTrader and will cause unexpected behavior in the script.

I would instead recommend creating a custom method and triggering this on a timer.

(Update March 3rd, 2020 - Added a return when ChartControl is null to prevent errors. This example uses the ChartControl and is intended to be applied directly a chart)

[QuantKey_Bruce]

Hello Bruce DeVault,

Adding a sleep to a data event driven method (or any NS method) is not supported by NinjaTrader and will cause unexpected behavior in the script.

I would instead recommend creating a custom method and triggering this on a timer.

Thanks. I do understand how to create a timer - that was just an example of any long running routine. It could as easily be a loop from 1 to a billion. The key thing is that State changes to Terminated while OnBarUpdate is still executing because they are running on different threads. What I was asking was what NT's recommended best practices are to avoid the issue that unmanaged resources are cleaned up in OnStateChange but they are still in use in OnBarUpdate in another thread. I was demonstrating how locking can be used to address that, but asking if this is the recommended approach or if there is a better reference example for how to keep the two threads from stomping each other resulting in issues.

[NinjaTrader_ChelseaB]

Hello Bruce DeVault,

Without using a sleep, can you create a simple script that shows OnBarUpdate will trigger after the last trigger of OnTermination? (OnTermination will trigger 3 times, we are only concerned with the last trigger).That I know of this should not be possible and I am not able to reproduce this behavior.

[QuantKey_Bruce]

Hello Bruce DeVault,

Without using a sleep, can you create a simple script that shows OnBarUpdate will trigger after the last trigger of OnTermination? (OnTermination will trigger 3 times, we are only concerned with the last trigger).That I know of this should not be possible and I am not able to reproduce this behavior.

To be clear, what happens is that OnBarUpdate is already running but has not yet exited when OnStateChange changes state to Terminated on another thread, cleaning up the unmanaged resources that OnBarUpdate is still using.

[NinjaTrader_ChelseaB]

Hi Bruce DeVault,

This would be expected.

OnStateChange is called from a different thread than the data event driven methods which are triggered from the Instrument thread.

When Termination is called, this can cause items to become null in OnBarUpdate if this is still processing. I would recommend you check for null on any objects being called to prevent errors. (I also suggest this for anything being cleaned up in State.Terminated)

If you would like we can submit a feature request to ensure OnBarUpdate has finished running before initiating OnStateChange(). Please note, it is up to the NinjaTrader Development to decide if and when a request will be implemented.

[QuantKey_Bruce]

It seems clear to me that OnBarUpdate should be finished processing before you set the state to Terminated and I think that feature request would be logical. Maybe you could explain the case where it would be better to go ahead and clean up the resources even though OnBarUpdate is still using them. What am I missing?

While, yes, we could do things like "if (a != null) a.method();" there is no guarantee of atomicity in that construct unless you use locking as I mentioned in the other post - however unlikely, "a" could become null or disposed after the if and before the method invocation because the other thread is busying breaking it down.

Under what sort of conditions would it be best to go ahead and destruct the resources of the other thread while it's still running the method?

[NinjaTrader_ChelseaB]

Hello Bruce,

I received a tracking ID for your feature request. For future reference, your request for OnStateChange to not Trigger Until OnBarUpdate has Finished Processing is being tracked with ID # SFT-1954.

Please note it is up to the ninjatrader development to decide if and when a future request will be implemented.

We highly appreciate your feedback. Please let me know of any other ideas or suggestions you have for the ninjatrader platform.

[ren37]

Hi

I thought this issue (but in OnRender() in my case) I have reported to NT support was fixed in 8.0.2.0
10758
Fixed
Core
Exceptions were being generated by terminated/finalized NinjaScripts

Before this fix has been released they recommend to me to use try-catch() operator in OnRender().

[QuantKey_Bruce]

Hi

I thought this issue (but in OnRender() in my case) I have reported to NT support was fixed in 8.0.2.0
10758
Fixed
Core
Exceptions were being generated by terminated/finalized NinjaScripts

Before this fix has been released they recommend to me to use try-catch() operator in OnRender().

Understood. I know the issue I was referring to is still true as of 8.0.4.0 but as you point out it did not involve OnRender in this case.

The NT8 multithreading model has lots of nuances that are going to gradually uncover different little behaviors like this. Some of them may not be "fixable" but others will get worked around as they're documented - either by NT making a fix in a later release, or because we figure out how to get around them through side steps.

Something like try-catch can certainly silence an exception but it doesn't address the problem itself cleanly - it's a major pain to try to use finally and using for every resource so all resources are properly cleaned up when an exception is thrown deep in a set of logic and caught way outside in a try catch block before the resources can be cleaned up in a way that's straight-forward. Far better would be the world if resources are not disposed on another thread while the first thread is still in the middle of a method.

Locking is one way to solve it on our end, but this is something that probably should not be.

[ren37]

I think that Mutex or AutoResetEvent objects might solve NT multi threading sync.
My question to staff: Is it possible that OnMarketDepth() been invoked simultaneously from different threads? What about OnMarketData()?

[QuantKey_Bruce]

Because the data threads are in a thread pool, if NT has not taken special precautions to avoid this, it might be that a data method like OnMarketData or OnMarketDepth is still running when the next data event arrives and starts into the method from a second worker thread in the same pool. NT is saying in the another similar thread on the forums that they do not want to recommend everyone do locking as a general solution but I do not follow how, without locking, and if they do not handle it for us, we can be sure there aren't reentrancy problems like the one you posit.