InvokeAsync and Invoke for Dispatcher

[sultanofsuede]

I thought the documentation stating that InvokeAsync was needed in certain multi-threaded use cases involving UI updates was strange. There was an odd claim about "deadlocks" if InvokeAsync() wasn't used for certain UI operations. My use case is the strategy class, not any indicator, etc.

Note: As a best practice, you should always make sure to use Dispatcher.InvokeAsync() to ensure your action is done asynchronously to any internal NinjaTrader actions. Calling the synchronous Dispatcher.Invoke() method can potentially result in a deadlock scenarios as your script is loaded.

InvokeAsync is awaitable and nothing in the OnBarUpdate chain of calls is asynchronous out of the box (Task never appears as a return type in any of the strategy code I've been working with). Therefore, telling anyone that InvokeAsync() is the way to go is not going to work for the strategy specializations. You have to make the code complete its work or you will be left with half-baked oddities in the UI.

For this reason, MS provides Invoke(), which is intended for NT's synchronous OnBarUpdate calls. Invoke() is the correct use case for situations where the UI thread is needed to update a widget already drawn on a chart. ChatGPT and other LLMs give a good explanation for why this is the case.

Obviously, using InvokeAsync() is going to cause all sorts of issues if no await keyword is used. I ran into this while trying to paint the diamond of a previous bar with a specific color. Some diamonds got the color, some don't because obviously the thread is not necessarily running to completion on each call. It's not properly awaited. Reloading the strategy yields a random completion for the same exact diamonds/bars.

I've noticed the NT UI gets into freeze issues whenever Invoke() is used. Something smells off about its behavior in situations where a simple UI update is needed. I must be doing something wrong and have not found anything in the documentation on multi-threading.

Please provide a pointer. I've used the ChartControl's Dispatcher as well as the application current's version with no luck.

[NinjaTrader_ChelseaB]

Hello sultanofsuede,

Dispatcher.InvokeAsync() is necessary when accessing a different thread. For example accessing the UI thread from a NinjaScript thread to add buttons to a chart.

Invoke() can and will deadlock the UI thread and freeze the platform (which I have done on a few occasions until I got in the habit of using InvokeAsync()).
Trying to interrupt the UI thread as it is doing work to try and force other work will cause the deadlock.

Try testing the SampleWPFModifications reference sample. Note no await is used and the script does not experience issues.

https://ninjatrader.com/support/helpGuides/nt8/creating-chart-wpf-(ui)-modifi.htm

Note, setting BarBrush, BarBrushes, BackBrush, BackBrushes, calling Draw methods are all done from the NinjaScript thread and do not require an invoke at all.

If you are accessing Series<T> objects from outside of the data-driven-method (like OnBarUpdate()), a TriggerCustomEvent() call is required to synchronize the data first.

https://ninjatrader.com/support/helpGuides/nt8/triggercustomevent.htm

[sultanofsuede]

Hi Chelsea,

Thanks for the reply.

This is not the way Task handling in .NET works.

InvokeAsync is awaitable, then it must be properly awaited by the caller in order for the work to run to completion. Otherwise, as I mentioned you will end up with work that is not complete and will eventually leave the app in an unstable place.

await chartControl.Dispatcher.InvokeAsync(() => ... )

NOT

chartControl.Dispatcher.InvokeAsync(() => ...)

The latter can be used if you call dispatcherOperation.Wait(), but then, why not just use Invoke() since this is the synchronous version? There's no point in using InvokeAsync() since the Invoke is fine for sequential programming which is what NT strategy builder uses even though there are multiple threads running. From the programmer's perspective, strategy looks "sequential." Moreover, Invoke and InvokeAsync are not different in purpose and design; both use a thread-safe queue to track work items based on MS documentation. So it's simply not the case that one would cause a deadlock and the other would not. Imagine a core piece of the .NET Framework like the Dispatcher being prone to what you call "deadlocks." I suspect that NT has code that is the issue, not Microsoft's Dispatcher.

Using the await operator would require that methods either return a Task to the caller or that they handle the result synchronously (using Wait() for example).

OnBarUpdate and the other methods in strategy do not use the modern Task-based programming for parallel programming introduced by .NET over a decade ago. That's fine, but you should *not* be calling InvokeAsync in the call stack for strategy unless you are synchronizing correctly. I would definitely not be telling NT coders to be doing this either.

What you call "deadlocks" can be diagnosed with VS by taking a memory dump of the process and then analyzing in VS.

This to say that I don't think your multi-threaded advice is sound at all for strategy building. Can't speak to other stuff in NT.

Note, setting BarBrush, BarBrushes, BackBrush, BackBrushes, calling Draw methods are all done from the NinjaScript thread and do not require an invoke at all.

Not what I'm seeing at all. If you use the chartControl.Dispatcher.CheckAccess() method, it will indicate whether the current executing code in a method needs to be invoked on the Dispatcher or not. If True, then yes, then you are on the main UI thread and can update a bar brush color directly. In my case, the CheckAccess() returns false and indicates that I must use the Dispatcher to update an existing diamond, bar, etc. My class is created in the OnBarUpdate method and doesn't do anything sophisticated with apart from trying to set a color on a diamond or bar.

As a test, I've run the code without using the Dispatcher and gotten NT to hang. I've taken a crash dump and looked at in VS. It identifies the iteration over DrawObjects as the culprit. Using the Dispatcher has resolved the issue.

For now, I will continue to use Wait() on the InvokeAsync() calls but this is really the same as calling Invoke() directly.

[NinjaTrader_ChelseaB]

Hello sultanofsuede,

Just to confirm are you seeing an issue when testing SampleWPFModifications ?

[sultanofsuede]

No, I'm just running a strategy I wrote.

OnBarUpdate()

Create a class Xyz inside of OnBarUpdate.

Call Xyz.SetDiamondArrowBrush(SolidColorBrush diamondBrush)

        public void SetDiamondBrushColor(SolidColorBrush diamondBrushColor)
        {
            var prefix = $"{nameof(SetDiamondBrushColor)}";
            var checkAccess = chartControl.Dispatcher.CheckAccess();

            log(prefix, $"{nameof(checkAccess)}: {checkAccess}",
                LogLevel.Information);

            var dispatcherOperation = chartControl.Dispatcher.InvokeAsync(() =>
            {
                DiamondBrushColor = diamondBrushColor;

                foreach (var drawObject in drawObjects)
                {
                    if (!drawObject.Tag.Contains($".{CurrentBar}"))
                        continue;

                    var diamondToChange = drawObject switch
                    {
                        Square square => square,
                        _ => null
                    };

                    if (diamondToChange == null)
                        continue;

                    diamondToChange.AreaBrush = diamondBrushColor;

                    break;
                }
            });

            dispatcherOperation.Wait();
        }​

IRL, I would never use InvokeAsync() in this kind of context and would use Invoke() instead since it's the same method but it is meant to be run without a parallel context (Task-based). OnBarUpdate is NOT Task-based at all and knows nothing of the TPL. MS provides sync/async versions of some framework methods to support the different use cases.

In either case, the app freezes up after running for a bit. Works fine and updates icons until it eventually causes NT to hang.

Here is the correct way to use InvokeAsync():

        public async Task SetDiamondBrushColorX(SolidColorBrush diamondBrushColor)
        {
            var prefix = $"{nameof(SetDiamondBrushColorX)}";
            var checkAccess = chartControl.Dispatcher.CheckAccess();

            log(prefix, $"{nameof(checkAccess)}: {checkAccess}",
                LogLevel.Information);

            await chartControl.Dispatcher.InvokeAsync(() =>
            {
                DiamondBrushColor = diamondBrushColor;

                foreach (var drawObject in drawObjects)
                {
                    if (!drawObject.Tag.Contains($".{CurrentBar}"))
                        continue;

                    var diamondToChange = drawObject switch
                    {
                        Square square => square,
                        _ => null
                    };

                    if (diamondToChange == null)
                        continue;

                    diamondToChange.AreaBrush = diamondBrushColor;

                    break;
                }
            });
        }​
        }​

[NinjaTrader_ChelseaB]

Hello sultanofsuede,

Drawing objects are accessed from the NinjaScript thread not the UI thread.

Don't use a Dispatcher when looping the DrawObjects collection from OnBarUpdate. (If this from a different thread, invoke into the NinjaScript thread)

Custom brush objects need to be frozen.

SolidColorBrush coolGreen = new SolidColorBrush(Color.FromRgb(30, 255, 128));
// you must freeze these brushes after they are constructed!
coolGreen.Freeze();

See 'Using WPF brushes' in the help guide.

https://ninjatrader.com/support/helpGuides/nt8/index.html?ninjascript_best_practices.htm#Errorhandling

[sultanofsuede]

Don't use a Dispatcher when looping the DrawObjects collection from OnBarUpdate. (If this from a different thread, invoke into the NinjaScript thread)

Custom brush objects need to be frozen

There are exceptions thrown when iterating over the objects in the DrawObjects collection because other threads are modifying those objects within NT. Not using the Dispatcher or some read write slim lock or full blow lock to look at the collection induces these since your product has so many threads doing things. Locks and slim locks are not the correct choice here since the Dispatcher is the recommended way of doing work with objects hosted on the UI thread in Windows-based app.

Again, this is why I query the CheckAccess() method -- the Dispatcher will tell me whether I need to use it for accessing UI objects hosted on the UI main thread. In this case, it's telling me that I should be using the Dispatcher.

I am not creating any Thread objects on my own in the code.

I'm not sure why I would need to freeze a brush when applying it to the shape and I think your docs are already a little questionable on their claims.

At any rate, your documentation telling people to use InvokeAsync() is simply not correct and will lead to all sort of issues with threads never completing and producing a materialized result. This claim reveals a lack of understanding about asynchronous programming in .NET. I have doubts about the NT code base at this point.

UPDATE:

The brush freezing has nothing to do with the issues related here in this thread since I'm using the pre-defined brushes.

Using WPF brushes


Try to use a static predefined Brush if possible. If you need to customize a new brush object, make sure to .Freeze() the brush before using it.

Why: The pre-defined brushes are thread safe and do not require any special handling. Custom defined brushes, on the other hand, are NOT thread-safe and must be frozen otherwise cross-thread exceptions can occur.

[NinjaTrader_ChelseaB]

Hello sultanofsuede,

A "Collection was modified; enumeration operation may not execute" error occurs when the collection is being modified as it is being looped through. Not a threading issue. The DrawObjects collection should not use a dispatcher when accessing from the NinjaScript thread.

Copy the collection with .ToList() and loop through the copy list.

See the example 'Looping through the collection to find specific draw objects' in the help guide.

https://ninjatrader.com/support/helpGuides/nt8/drawingtools_drawobjects.htm

If you are able to produce issues with SampleWPFModifications without modifying the script, please let me know the steps to reproduce so I can investigate and confirm.

[sultanofsuede]

Invoke() can and will deadlock the UI thread and freeze the platform (which I have done on a few occasions until I got in the habit of using InvokeAsync()).
Trying to interrupt the UI thread as it is doing work to try and force other work will cause the deadlock.

The reason that calling InvokeAsync() appears to fix your freeze issues is that you have simply passed the work onto another thread. Since you don't await, you've no idea if the work ever completes but things keep moving in the UI so it seems like it's all fixed. If you look closer, you will see over time that UI work is not being completed because the threads get pre-empted; some finish their work without the await while others don't. They are pre-empted by the scheduler and the work never gets done. This is not a solution to the problem and reveals a misunderstanding about asynchronous programming.

[sultanofsuede]

Copy the collection with .ToList() and loop through the copy list.

See the example 'Looping through the collection to find specific draw objects' in the help guide.

If you are running frequent updates, using ToList() on a large collection of objects can be very non-performant.

A "Collection was modified; enumeration operation may not execute" error occurs when the collection is being modified as it is being looped through.

In the code I posted, I'm iterating and READING, not writing. As I mentioned, NT has many threads and those threads are modifying the collection that I'm reading. This is the cause for the exception. ToList()-ing a large number of UI objects seems pretty crazy.

It's really odd that your product doesn't provide a way to read a collection of UI objects safely since these are pretty critical for certain use cases.

[sultanofsuede]

Your DrawObjects.ToList() also creates deadlocks. Seen frequent crashes with it. Apart from being not very performant when there are lots of objects on the chart...