Mekanism Additions

Mekanism Additions

21M Downloads

Mekanism Mine Colonies Crash

Alexhupp opened this issue ยท 3 comments

commented

Issue description:

Game Crash on Block Update Of Rack From Minecolonies
1.16.3

Steps to reproduce:

1.Place Down A Mine Colonies Rack
2.Place A Mekanism Logistics pipe down next to it
3.Break The Rack

Version (make sure you are on the latest version before reporting):

Forge:31.1.42
Mekanism:10.0.15.441
Other relevant version: minecolonies-0.13.406-BETA

If a (crash)log is relevant for this issue, link it here: (It's almost always relevant)

https://pastebin.com/D4ZSrcH6

commented

Seems to be a minecolonies problem and how they implement getCapability on their rack... https://github.com/ldtteam/minecolonies/blob/version/1.16.3/src/api/java/com/minecolonies/api/tileentities/TileEntityRack.java#L444-L482 is invalidating the lazy optional inside of getCapability, thus causing mek's connectivity listener to try and recheck the side to see if it can connect, but calling getCapability on their rack then causes it to invalidate it again causing an endless loop until a stack overflow happens.

commented

That's weird though. It only invalidates the old one on a new call. We don't cache the lazy optionals and invalidate them early because we got large inventories spanning over many racks and tracking chunk loading etc around it is an absolute mess so we do want to rebuild it regularly.

commented

Ray the issue here is that if there is a capability already (from having listened to it initially), if then it gets called again what happens is more or less this:

if (lastCapability != null) {
lastCapability.invalidate();

listeners -> it got invalidated
my listener rechecks connection, calls getCapability again
lastCapability isn't null yet because it is mid invalidate call hasn't progressed yet so this entire thing repeats
}

While yes in my opinion it potentially is also a forge issue that calling invalidate multiple times on the same lazy optional fires for each of the listeners even though it was already invalidated, the entire getCapability implementation you have there in the rack currently is rather problematic. Personally in Mekanism before invalidating any capability I also am checking to make sure it is actually present (aka still valid), though I don't think we have any places where it would be as problematic if it didn't as your thing currently is, it is just an extra safety measure I put in place.