DynamicLights 1.9 crash on cme
spacerecycler opened this issue ยท 12 comments
I've been doing some testing to try to nail down a ConcurrentModificationException in a modpack being tested and I think DynamicLights is causing a CME when Minecraft tries to update lighting in the updateClouds (deobf) method or net.minecraft.client.renderer.RenderGlobal.func_72734_e. It looks like that code in Minecraft is using a HashSet to hold on to light updates and trying to perform these updates on the main thread and some other thread is changing the set contents when it is iterating over the collection. See http://imgur.com/9DSgx3o for the code snippet from RenderGlobal. Here's the crash log https://paste.ee/p/eNZ55.
My steps to repro are below:
- Launch a new world
- Create a barrier block coffin (might also be able to use creative)
- /time set 12000 (probably don't need to do this but the crash has occurred mostly at night)
- Wait for crash, since this is a timing bug it's hard to reproduce every time
Versions:
JDK 1.8.0_77
Forge 12.16.0.1804
DynamicLights 1.9 (sha1 - 782298c0f98819b5df9361699d21d3364e0edcd2)
My first thought upon reading this was that maybe IDEA had changed one of the Dynamic Lights internal loops to a streaming foreach, but no. Dynamic Lights should also run on the main thread only and block whatever the world does until it's done.
Also, i had a look at the mc code and the crashlog, the crashing part is the "damagedBlocks" values iterator (only hashmap in updateclouds) which has nothing to do with light updates. I'm going to presume this is a forge bug, for the time being.
HashSets are backed by HashMaps though and when I ran the remote debugger against the Minecraft instance and caught the CME it was trying to iterate over a Map of BlockPos, Object. I was using eclipse and I think I had to configure it to catch the CME in uncaught and caught situations since MC catches it and then gracefully shuts down. This was before I started to narrow down the mods in the modpack though so I could try it again with just Dynamic Lights if you want.
True, here's that stack with the local variables http://imgur.com/azhgdam. I'll try to run a test with just Minecraft and Forge and see if I can't get it to crash there as well.
If all else fails I can make dynamic lights replace that hashmap with something that can do concurrency
I found a different bug, I'll create a PR for that one. That bug only happens when trying to run the deobf Forge code. I'm going to try to run it in a local environment to try to see what is modifying that set.
@AtomicStryker not really related to the main topic but for the lighting calculations would it possible that if there are x number of pending updates for the lighting from the player held torch remove the old ones and only have minecraft do the newest one? That would probably help with the calculation lag that minecraft has though its more a remove old outdated work than a proper fix.
So I switched the breakpoint to suspend the entire VM when the CME was thrown and I think I found the stack that is causing the CME. Here's a snip of my eclipse debugging enviornment http://imgur.com/osVRMkn. Looks like entities are being updated on another thread but I'm not sure where this thread is being started.
See if ee9e703 fixes the issue for you. It should.
I've been testing for a while and I think that solved the crash.
I might need to do some more testing for something that might be related. The game seems a little laggy when it is trying to calculate lighting and it even seems to stop calculating it for a while. I tested this by holding a torch and running around a full speed for 30 seconds or so and the lighting calculations seem to work for a while but then start backing up in the set. On the F3 screen the L: number seems to keep increasing till you stop and then after a certain amount of time MC catches up and runs all the remaining calculations.