Deadlock using Account.AccountStatusUpdate

[ntbone]

I am running the latest version of NinjaTrader 8.1.4.x. I launched NinjaTrader and it was stuck at the screen with the animating logo. I attached the Visual Studio Debugger and Visual Studio detected a deadlock.

The first thread is one of NinjaTraders threads. The call stack was

NinjaTrader.Cbi.MasterInstrument.DbLoad()
NinjaTrader.Cbi.MasterInstrument.All.get()
NinjaTrader.Core.Globals.ApplicationStart(System.A ction<string, bool, bool>, System.Func<string, int>)
NinjaTrader.AppHelper.OnStartup.AnonymousMethod__0 (object)
System.Threading.ExecutionContext.RunInternal(Syst em.Threading.ExecutionContext, System.Threading.ContextCallback, object, bool)
System.Threading.ExecutionContext.Run(System.Threa ding.ExecutionContext, System.Threading.ContextCallback, object, bool)
System.Threading.QueueUserWorkItemCallback.System. Threading.IThreadPoolWorkItem.ExecuteWorkItem()
System.Threading.ThreadPoolWorkQueue.Dispatch()


The other thread was an addon I created.

    public class DemoAddon: NinjaTrader.NinjaScript.AddOnBase
    {
        protected override void OnStateChange()
        {
            switch(State)
            {
                case State.Configure:
                    Account.AccountStatusUpdate += Account_AccountStatusUpdate;  // dead locked here
                    break;
            }
        }

        private void Account_AccountStatusUpdate(object sender, AccountStatusEventArgs e)
        {            
        }
    }
​

Looking at the call stack for the AccountStatusUpdate += it too is accessing Account.All based on the call stack

System.Windows.Threading.DispatcherSynchronization Context.Wait(System.IntPtr[], bool, int)
[Native to Managed Transition]
[Managed to Native Transition]
NinjaTrader.Cbi.MasterInstrument.DbGet(string, NinjaTrader.Cbi.InstrumentType, bool)
NinjaTrader.Cbi.Commission.MasterInstrumentsSerial izable.set(string[])
Microsoft.Xml.Serialization.GeneratedAssembly.XmlS erializationReaderCommission.Read4_Commission(bool , bool)
Microsoft.Xml.Serialization.GeneratedAssembly.XmlS erializationReaderCommission.Read5_Commission()
[Native to Managed Transition]
[Managed to Native Transition]
System.Xml.Serialization.TempAssembly.InvokeReader (System.Xml.Serialization.XmlMapping, System.Xml.XmlReader, System.Xml.Serialization.XmlDeserializationEvents, string)
System.Xml.Serialization.XmlSerializer.Deserialize (System.Xml.XmlReader, string, System.Xml.Serialization.XmlDeserializationEvents)
NinjaTrader.Cbi.Deserializer.Deserialize()
NinjaTrader.Cbi.Commission.LoadAll()
NinjaTrader.Cbi.Commission.Get(string)
NinjaTrader.Cbi.Account.DbLoad()
NinjaTrader.Cbi.Account.All.get()
NinjaTrader.Cbi.Account.AccountStatusUpdate.add(Sy stem.EventHandler<NinjaTrader.Cbi.AccountStatusEve ntArgs>)

If I recall from sample code, when accessing Account.All one is supposed to lock(Account.All).
Am I supposed to do something similar when connecting to this event handler?
This is the first time having this happen since using NinjaTrader going back to 8.0.2x and this code is at least that old.

[NinjaTrader_ChelseaB]

Hello ntbone,

Does this addon create a window?

If so, I would recommend attaching the account event handler method in the window Loaded event handler.

This will give time for everything to populate as the platform starts up.

If not, you might try adding the account event handler method in the OnWindowCreated() override. I'm not certain this would be late enough but it would be worth a try.
Alternatively maybe use a timer to wait a few seconds?

[ntbone]

This particular addon does not use a Window. Wrapping lock(Account.All) around the += operator seems to have worked, but with deadlocks and threading it's not clear this is the correct way to handle this. The reason the event is in the addon is because I am trying to monitor all the events that come through for this event. Waiting runs the risk of missing some.

As a side note, I see an inconsistency in the NinjaTrader code for accessing Account.All. There are two places that use the lock(Account.All) and two places that do not.

1. Ruler.cs and RiskReward.cs access Account.All without the lock.
2. PositionSize.cs and PositionAvgPrice.cs use the lock.

[NinjaTrader_ChelseaB]

Hello ntbone,

I would still recommend waiting a moment for the Account collection and accounts to be connected and ready before attempting to assign account handlers.

Perhaps add the code to the Connection.ConnectionStatusUpdate event handler.

https://developer.ninjatrader.com/docs/desktop/connectionstatusupdate

The code you suggested in post # 1 is the complete code of the test script correct?

The Ruler and RiskReward DrawingTools are not enumerating through the Account.All collection and are accessing the first account for a property.
Account.All[0].ForexLotSize

The PositionAvgPrice and PositionSize MarketAnalyzerColumns are enumerating through the Account.All collection to search for a specific account
Cbi.Account.All.FirstOrDefault(o => o.DisplayName == AccountName)

The lock is to prevent modifications to the collection while the collection is enumerated over.
'Collection was modified; enumeration operation may not execute'

I used to advise using a lock before enumerating over a collection that can be modified from another thread. However, I generally suggest making a copy of the collection with .ToList() and enumerating over the copy. This allows the original collection to remain lock free and not at risk of missing updates from other threads while locked.


Your suggested code is not showing any enumerating on the Account class.

However, I don't expect that code you've suggested can compile or run. The Account class itself does not have these event handlers.
You would need to select a specific Account object from the Account.All collection and save this to a variable to add an event handler.

[ntbone]

Regarding the lock, I assume one needs to still lock the collection before creating the copy? This is currently how I handle enumeration as well. I lock the collection, then I enumerate through the copy.

With regards to the code above, I have omitted the rest of the code but the structure I sent would have compiled. Account.AccountStatusUpdate is available on the Account class as per the documentation here

https://ninjatrader.com/support/helpGuides/nt8/NT%20HelpGuide%20English.html?accountstatusupdate. htm
or here with the new documetnation

https://developer.ninjatrader.com/docs/desktop/accountstatusupdate

If I use the connection status update per your recommendation, I may run into the same issue? It also operations on a static event handler for the Connection class. Its not clear what its doing under the hood.

No, my code is not accessing the Accounts.All collection but examining the call stack I saw that the code lower down is. Both threads were calling NinjaTrader.Cbi.Account.All.get(). I figured locking on Account.All prevent the code from potentially continuing but that also assumes there's a lock elsewhere on Account.All to prevent it from proceeding.

[NinjaTrader_ChelseaB]

Hello ntbone,

A lock is not necessary when creating a copy with .ToList(). Making a copy of the list is instead of using the lock keyword.

Posting code that different from the script it becomes harder to provide direction.

Creating a copy of the Account.All collection in the OnConnectionStatusUpdate() override method should prevent the script from halting.

Making a copy of the Account.All collection without locking and enumerating over the copy should prevent the collection modified error.

[ntbone]

If you don't lock the list while making a copy it is still possible for something to be modifying the list while the copy is being made. To make a copy, the code needs to enumerate the list.

I am not accessing or using Account.All in the start up script. As I said earlier the only call I am making in Configure is Account.AccountStatusUpdate += Account_AccountStatusUpdate; // dead locked here. When I looked at the call stacks I see that under the hood NinjaTrader is accessing Account.All which is why I made a guess about using the lock.

[NinjaTrader_ChelseaB]

Hello ntbone,

Please let me know if you get the enumeration error when using .ToList(). The scripts I've made or modified are not having that issue.

[ntbone]

Reproducing multi-threaded errors are difficult because timing is everything. To reproduce the error, you need code that is modifying the list at the same time as the call to ToList() is happening. This will be dependent on what other threads and code in the application are doing, as well as what hardware it is being run on and the load on the operating system. Multi-threaded code leads the possibility of an error, and difficulty reproducing the bug. Since .ToList is not an atomic operations it is always possible under the right conditions that a call to .ToList() will occur at the same time some other code is modifying said list resulting in a modification of the list. This is a fundamental concept to mulit-threaded software development.