Collection was modified error while processing data from socket stream

Question

I'm trying to capture tick level data from a Binance Aggregated data stream. As soon as the date changes, a function processes the data from the previous day. I'm using Task.Run to create another thread as new data for the current day is still streaming in and needs to be captured. However, I'm getting a 'Collection was modified enumeration operation may not execute.' in the foreach loop of the function that processes the data. I'm not sure why tradeData would be modified after it is passed to the processing function? Not sure if it matters but I'm capturing about 40 different symbols on separate threads.

private static void Start()
{
    foreach (SymbolData symbol in symbolData)
    {
         Task.Run(() => SubscribeToSymbol(symbol.symbol));
    }
}

private static void SubscribeToSymbol(string symbol)
{
    Dictionary<decimal, decimal> tradeData = new Dictionary<decimal, decimal>();
    DateTime lastTrade = DateTime.UtcNow;
    try
    {
         var socketClient = new BinanceSocketClient();
         socketClient.FuturesUsdt.SubscribeToAggregatedTradeUpdates(symbol, data =>
         {
              if (data.TradeTime.Date > lastTrade.Date)
              {
                   Task.Run(() => ProcessData(symbol, lastTrade.Date, tradeData));
                   tradeData.Clear();
              }
              lastTrade = data.TradeTime;

              if (tradeData.ContainsKey(data.Price))
              {
                   tradeData[data.Price] = tradeData[data.Price] + data.Quantity;
              }
              else
              {
                   tradeData[data.Price] = data.Quantity;
              }
         });
     }
     catch { }
}


private static void ProcessData(string symbol, DateTime date, Dictionary<decimal, decimal> tradeData)
{
     foreach (var price in tradeData)
     {
     //Error: Collection was modified enumeration operation may not execute.
     }
}

`catch { }` Don't do this. At a *bare minimum*, log something. Eating the exception like this will only make it harder to diagnose issues with your code. — , Jan 12 '21 at 19:12
Well, you modify `tradeData` while you are processing it: `ProcessData` may run parallel to `tradeData[data.Price] = tradeData[data.Price] + data.Quantity;` or `tradeData[data.Price] = data.Quantity;`, because you started it with `Task.Run()`. — Xaver, Jan 12 '21 at 19:15
Expanding on Xaver--you Clear tradeData as soon as you kick it off to run asynchronously. So, if timed right, the foreach loop starts, then while the foreach loop is running, tradeData gets cleared, so the foreach fails because the elements it was working on disappear. You might better get what you are looking for by passing a copy of tradeData to ProcessData instead of passing the actual tradeData. — Russ, Jan 12 '21 at 19:21
Ok interesting. I thought that the Task.Run step would at least complete sending the current copy of tradeData in its state to the ProcessData function before continuing on with the rest of the code. That's a good suggestion though - i'll make a copy of the data before sending it to the ProcessData function. — sleepymatto, Jan 12 '21 at 19:28

score 1 · Answer 1 · answered Jan 12 '21 at 19:28

You're calling Task.Run repeatedly and as a result have more than one thread working with your tradeData object which is why you're seeing the collection modified exception.

I would recommend you rethink the code design so that you dont have multiple threads working with the same objects. If you absolutely must have threads working with the same objects then lock should be used:

private static object _objlock = new object();

private static void ProcessData(string symbol, DateTime date, Dictionary<decimal, decimal> tradeData)
{
    lock (_objlock)
    {
        foreach (var price in tradeData)
        {
     
        }
     }
}

Just be careful with deadlocks when using lock. I'd recommend reading up on locks and multi threading in C#.

Ok thank you for the suggestion I will look into that – sleepymatto Jan 12 '21 at 19:32 — sleepymatto, Jan 12 '21 at 19:32

score 0 · Answer 2 · answered Jan 12 '21 at 20:20

By the time you start the foreach loop over the dictionary, the outside is "clearing" it. You should make a copy and send to ProcessData.

Task.Run(() => ProcessData(symbol, lastTrade.Date, tradeData.ToArray()));

And Receive the copy:

private static void ProcessData(string symbol, DateTime date, KeyValuePair<decimal, decimal>[] tradeData)
{
    foreach (var price in tradeData)
    {
// ... do your thing here
    }
}

Or, even more simple, just recreate a new dictionary instead of clearing it:

//tradeData.Clear();
tradeData = new Dictionary<decimal, decimal>();

Collection was modified error while processing data from socket stream

2 Answers2