MicroTrendsLtd / NinjaTrader8

NinjaTrader8 Components Strategies and Trading tools
MIT License
72 stars 17 forks source link

Fix to actually call TradeManagement and reset flag #84

Closed bdowling closed 1 year ago

bdowling commented 1 year ago

Unless I'm missing something, TradeManagement callback was never actually being called in the Hybrid Demo or other Algos.

It looks to me like bits of the locking logic were removed or incomplete, as the isInTradeManagementProcessInternal flag was never actually cleared either.

This small fix appears to rectify that, I'm guessing this was the intent of the locking.

The second test inside the lock seems dubious, the exclusive lock should be sufficient, but I left it for now.

   if (isInTradeManagementProcessInternal)
                    return;
MicroTrendsTom commented 1 year ago

ok

In theory the Virtual is called inside the Base Class and the override method is called.

The base class calls this method TradeManagementExecInternal(LastPrice); from multiple methods OnBarUpdate: OnMarketData:

so we apply some fine grain locking as follows and we only call it 1 time.

` private void TradeManagementExecInternal(double lastPrice) { //make sure something is not midflight suchj as order operations if (!IsTradeWorkFlowReady()) return;

        //use this to guard against multiple thread entry from OnMarket data and onBarUpdate
        if (isInTradeManagementProcessInternal)
            return;

        lock (lockObjectTradeManInternal)
        {
            if (isInTradeManagementProcessInternal)
                return;
            isInTradeManagementProcessInternal = true;
        }
        TradeManagement(lastPrice);
    }`

As we can see this is not correct it as it will lock and never unlock..

This should be: ` private void TradeManagementExecInternal(double lastPrice) { //make sure something is not midflight suchj as order operations if (!IsTradeWorkFlowReady()) return;

        //use this to guard against multiple thread entry from OnMarket data and onBarUpdate
        if (isInTradeManagementProcessInternal)
            return;

        lock (lockObjectTradeManInternal)
        {
            if (isInTradeManagementProcessInternal)
                return;
            isInTradeManagementProcessInternal = true;
        }
        TradeManagement(lastPrice);
        isInTradeManagementProcessInternal = false;
    }

`