Indicators, Threads and Dispatchers

[LeonK]

When accessing things from multiple threads, some sort of synchronization mechanism is required to prevent, say the OnStateChange from destroying or changing things used by OnBarUpdate/OnRender.

That's exactly my point: the suggestion in the help guide is insufficient. One need to guard a critical section with some semaphoring / locking.

I am looking forward to Ninja team to chime in.

[ntbone]

My solution, since I don't want to lock OnRendering/OnBarUpdate is to not clean up those resources and rely on garbage collection. There are things for which Dispose should be called when you are done with them, but since it was causing crashes due to OnBarUpdate/OnRender this was the what I ended up doing for now.

[QuantKey_Bruce]

I have run into this scenario and problem numerous times. I have some code that is using an object, often a render object that should be disposed. What happens, when the timing is just right, is that Terminate is called, the object is disposed while it is being used by OnRender.

I even did their recommended checks but because Terminate can be called while OnRender or OnBarUpdate is half way through execution, its always possible that the object will be assigned null/disposed while half way through execution of OnRender/OnBarUpdate.

When accessing things from multiple threads, some sort of synchronization mechanism is required to prevent, say the OnStateChange from destroying or changing things used by OnBarUpdate/OnRender.

Are you only disposing of your render-related resources in OnRenderTargetChanged when RenderTarget is null? Or, are you trying to dispose of your render-related resources in OnStateChange when State == State.Terminated?

It is recommended, I believe, to only dispose of render-related resources in OnRenderTargetChanged when RenderTarget is null.

So, there are render-related resources that get disposed in OnRenderTargetChanged when RenderTarget is null, and there are calculation-related resources that get disposed of in OnStateChanged when State is State.Terminated.

It can be, and sometimes is, that OnStateChanged hits State.Terminated while still running OnRender (on another thread). However, it cannot be that OnRenderTargetChanged is hit with RenderTarget null while OnRender is still running. That is the saving grace and key piece of understanding here.

[QuantKey_Bruce]

My solution, since I don't want to lock OnRendering/OnBarUpdate is to not clean up those resources and rely on garbage collection. There are things for which Dispose should be called when you are done with them, but since it was causing crashes due to OnBarUpdate/OnRender this was the what I ended up doing for now.

Yes, I agree - this is what you need to do to assure platform stability. Let GC handle when to dispose of your objects. It may not be immediately but it will do so when they are no longer referenced and it gets around to running the GC cycle. I have been through the same painful exercises - attempting to force-dispose things like this leads to low-probability events that occasionally bring down the platform because of the threading design. The best thing you can do is to chill and let the framework handle it and it will do so when it thinks GC is necessary or when it has idle cycles.

I've been through some long, winding development cycles in which I enforced all sorts of things for sort of memory hygiene only to find out that on some other computer with some other number of cores or with some other number of windows open, it crashes or does something completely different than it does in development/testing. The way to not find yourself in that predicament is to not do that stuff - just let GC wind down your resources when GC is sure they are done.

[QuantKey_Bruce]

The sort of worst case scenario for disposal is when running a long run on Strategy Analyzer. There can be cases there where GC does not run until you are actually out of physical memory because it's busy 100% of the time. That's because Strategy Analyzer is designed deliberately to use 100% of the available resources. But in normal charting usage, no.

[ntbone]

The idea from QuantKey_Bruce for cleaning up rendering resources in OnRenderTarget changed looks good. That should solve cleaning up rendering resources without causing crashing.

The fact that OnBarUpdate and OnRender run from different threads, indicates that it is possible for both to be running at the same time. This can mean other issues with data stability. For example, if a collection is being used and OnBarUpdate adds an item while OnRender iterates the collection can cause an exception to be thrown as a collection cannot be modified while it is being iterated over. These sort of bugs are hard to reproduce as the timing must be just right for them to occur.

[QuantKey_Bruce]

ntbone It is DEFINITELY possible for them to be running at the same time and that is the norm and 100% the expectation if the multithreading is working. Plan accordingly.

It's been covered above I believe, but for anyone else reading this, you should only be cleaning up UNMANAGED resources in State.Terminated - let GC handle managed resource disposal. And definitely DO NOT clean up render resources in State.Terminated. Use OnRenderTargetChanged when RenderTarget is null for that.


Let's say you have a List<Whatever> MyList that you're going to be iterating in OnRender to render your Whatevers. Here is what you should do.


At instantiation, to make sure there's always a list until the indicator instance is destructed by GC (after State.Terminated - don't do it there):

List<Whatever> MyList = new List<Whatever>(); // don't destruct this yourself - let GC do it

In OnBarUpdate, where you are going to be monkeying with the list:

if (MyList == null) return;
lock(MyList)
{
  // add some, remove some, change some, whatever you like
  // but keep this area tight so you don't lock anything else or wait on anything else
  // get your changes done and get out of this scope asap
}

In OnRender, where you need to safely work with the list from a different thread:

List<Whatever> MyListLocalCopy = null;
if (MyList == null) return;
lock(MyList)
{
  if (MyList == null) return;
  MyListLocalCopy = new List<Whatever>(MyList); // note that this operation is extremely fast, and the lock scope is tight
  // if Whatever is simple e.g. some primitive type fields like strings, DateTimes, ints, and doubles, this suffices
  // if Whatever has some lists and other things in it, you may find you want to make a deep copy instead here
  // but don't do this unless you need to because new List<Whatever>(MyList) is very fast and should be used unless
  // you have a specific reason you need a deeper copy
}

// now you can safely iterate MyListLocalCopy and do whatever you want, because you have made a safe, local clone
// while the list was locked.

// even if this instance reaches State.Terminated, you're only working with a local variable here at this point and no
// other thread can change or in any way touch your local list copy

foreach (Whatever MyWhatever in MyListLocalCopy)
{
  // render MyWhatever
}

// MyListLocalCopy will be disposed after you fall out of OnRender scope

May the force be with you.

[LeonK]

if (MyList == null) return;
lock(MyList)
{
// add some, remove some, change some, whatever you like
// but keep this area tight so you don't lock anything else or wait on anything else
// get your changes done and get out of this scope asap
}

With all due respect, how do you know that the thread running OnRender() in your example has not been preempted after

if (MyList == null) return;

Why instead not to do this:

if (State == State.Configure)
{

MyLock = new object();

OnRender() // OnBarUpdate() is same logic / approach
{
lock(MyLock) // this will never fail
{
if (MyList != null)
{
// Here you're guaranteed that MyList is not null since the other thread that could null the list has been "paused" for the time you hold the MyLock.

[QuantKey_Bruce]

LeonK Thanks for your comments. In my example, I had set MyList once and was never changing it, so it is not possible that another thread could null the list. I should probably have added comments to the effect that you should .Clear() the list inside the lock if you need to rather than nulling it. The null guard code is largely gratuitous but I added it out of habit - it could have been an assertion or we could just let it exception if I am wrong and the list did get nulled. If the code were preempted after the null guard code but before the lock (something that while unlikely is absolutely possible) the worst that could happen would be that it was nulled and we tried to lock null, something the framework would kick out with an exception, but since I am never allowing it to be nulled and explicitly stated that only the GC should dispose it, that is not something that actually happens. Further, the reason I put the null guard code outside of the lock statement is to assure that the list is not null before locking it so the framework wouldn't exception there - and I am never nulling it during the entire lifetime of the indicator instance so this is largely gratuitous.

You can separate the list from the lock, as you have done - that's a matter of style - but since it is the only list I am concerned with here I didn't think that added anything to the example. You absolutely can do that if you wish, and then you could potentially allow the list to be nulled if you need that. As a matter of personal habit, if I'm only concerned with controlling access to one object, I don't add a separate lock but lock the object itself because that makes the code more readable to me and I know exactly what is being controlled as I am reading the code. The framework does not care what you lock - it just compares the object references to see if they are the same.

I would also suggest, if you want to follow the approach you list above (which would be a way I would go if either I were controlling more than one object with a single lock or if I needed the list/object to be nullable yet still lockable), that you create object MyLock = new object; as a field declaration in the class rather than setting it in State.Configure, because you cannot guarantee OnRender will not run before it reaches that line the State.Configure handler, and here again, if you lock null, you will exception out. That is why in my example I created the list outside of State.Configure when the indicator was instantiated.