Lumina icon indicating copy to clipboard operation
Lumina copied to clipboard

`EmptyLazyRow.GetFirstLazyRowOrEmpty` Concurrency Issue

Open WorkingRobot opened this issue 1 year ago • 1 comments

https://github.com/NotAdam/Lumina/blob/bb7d83631a3e878017f16ee84e42e9f794868b88/src/Lumina/Excel/LazyRow.cs#L32

This being a static non-concurrent dictionary can cause concurrency issues when parsing new rows. Here is a stacktrace I've seen regarding this issue:

System.Collections.Generic.KeyNotFoundException: The given key 'ChocoboTaxiStand' was not present in the dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at Lumina.Excel.EmptyLazyRow.GetFirstLazyRowOrEmpty(GameData gameData, UInt32 row, Language language, String[] sheetNames)
   at ExdSheets.Action.PopulateData(RowParser parser, GameData gameData, Language language)
   at Lumina.Excel.ExcelSheet`1.ReadRowObj(RowParser parser, UInt32 rowId)

Obviously in code like the following, a dictionary lookup like that (L63) should never fail unless there is a race condition of some sort. https://github.com/NotAdam/Lumina/blob/bb7d83631a3e878017f16ee84e42e9f794868b88/src/Lumina/Excel/LazyRow.cs#L53-L66

WorkingRobot avatar Jun 22 '24 05:06 WorkingRobot

ah yeah that was likely an oversight when initially changing this to work a bit nicer in async/threaded environments - happy for this to move to a concurrentdict though if you'd like to PR it, as there's not really a trivial lock that would make this work nicely i think

NotAdam avatar Jun 24 '24 01:06 NotAdam