[10.0.15.441 @ 116.3] Crash: Inhalation Purification Unit encounters ConcurrentModificationException
andrewsf opened this issue ยท 6 comments
Issue description
The Inhalation Purification Unit iterates over player.getActivePotionEffects(), which is a values-only view of a HashMap, but other threads may modify the HashMap (e.g., remove effects), causing Mekanism code to sometimes encounter a ConcurrentModificationException.
This is probably exacerbated by Mekanism speeding up all the effects so that many may expire at once. But even if Mekanism did nothing, iterating could still hit the exception.
Steps to reproduce
- Enable Inhalation Purification Unit
- Add a ton of potion effects at once
In my case, the crash has occurred when many effects get added at once by:
- Gaia Guardian fight in Botania
- Boss mobs in Apotheosis
Versions
Forge: 34.1.25
Mekanism: 10.0.15.441
Other relevant version: All the Mods 6 mod pack @ version 1.2.1
Partial crash log
Pretty sure mods shouldn't be changing things from other threads, but could be caused during the forced effect ticking I suppose
Neither Mekanism nor the other mods own the collection. It is exposed by Forge in a manner that is not safe. But Mekanism and other mods can try to anticipate & recover.
I've hit this crash maybe 10 times now, twice tonight :(
I was wrong, the ConcurrentModificationException in this case is not around thread safety, but rather reusing the iterator past the lifespan of its validity.
Easiest approach is probably to invert the loops (0...9 goes outside, so player.getActivePotionEffects() is fetched fresh each time).
That sounds terrible for performance, it would be better to just copy the active potion effects or just add the ones we want to add extra ticks to to a list and then do the 0-9 on those.
getActivePotionEffects() itself returns a cached view of the map, and its ValueIterator is very cheap to instantiate, but point taken. I had assumed it's not safe to tick an effect 10 times without checking whether it got removed, but obviously it would have been very broken if that were true. Thanks for the fix.