Synchronized Access Errors

Hello!

Recently, I installed NoLagg on my server, to examine my plugins efficiency with /nl examine. While that worked out fine, it also brought to my attention what appears to be a far more serious error.

My plugin throws two nasty synchronization errors, and only when NoLagg is running. A bit more searching found that this is one of the features of NoLagg: it makes plugin authors aware of synchronization mistakes.

Thing is, the errors themselves are quite useful in determining the source of the problem, but a way to fix them I have not been able to find yet. One thing I did do was I modified my custom error logger to completely ignore the NoLagg-induced errors; an action I feel extremely guilty about.

Hence me coming forth to the Bukkit community, in hope that someone might shed some light.

Both errors are IllegalAccessExceptions.

Error 1 (open) Error 1 (close)

java.lang.IllegalAccessError: Synchronized code got accessed from another thread: java.util.concurrent.ThreadPoolExecutor$Worker
at org.bukkit.event.NLTCListener.onChunkLoad(NLTCListener:0)
at sun.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:616)
at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:339)
at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:62)
at org.bukkit.plugin.SimplePluginManager.fireEvent(SimplePluginManager.java:477)
at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:462)
at net.minecraft.server.ChunkProviderServer.getChunkAt(ChunkProviderServer.java:113)
at net.timedminecraft.server.TimedChunkProviderServer.getChunkAt(TimedChunkProviderServer.java:127)
at org.bukkit.craftbukkit.CraftWorld.getChunkAt(CraftWorld.java:108)
at org.bukkit.craftbukkit.CraftWorld.getBlockAt(CraftWorld.java:72)
at org.bukkit.craftbukkit.CraftWorld.getBlockAt(CraftWorld.java:469)
at org.bukkit.Location.getBlock(Location.java:82)
at com.github.StormTeam.Storm.Volcano.VolcanoMaker.generateLayer(VolcanoMaker.java:176)
at com.github.StormTeam.Storm.Volcano.VolcanoMaker.generateVolcanoAboveGround(VolcanoMaker.java:164)
at com.github.StormTeam.Storm.Volcano.VolcanoMaker$1.run(VolcanoMaker.java:146)
at org.bukkit.craftbukkit.scheduler.CraftTask.run(CraftTask.java:53)
at org.bukkit.craftbukkit.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:53)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)

Error 2 (open) Error 2 (close)

java.lang.IllegalAccessError: Synchronized code got accessed from another thread: java.util.concurrent.ThreadPoolExecutor$Worker
at org.bukkit.event.NLTCListener.onChunkLoad(NLTCListener:0)
at sun.reflect.GeneratedMethodAccessor4.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:616)
at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.java:339)
at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.java:62)
at org.bukkit.plugin.SimplePluginManager.fireEvent(SimplePluginManager.java:477)
at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.java:462)
at net.minecraft.server.ChunkProviderServer.getChunkAt(ChunkProviderServer.java:113)
at net.timedminecraft.server.TimedChunkProviderServer.getChunkAt(TimedChunkProviderServer.java:127)
at org.bukkit.craftbukkit.CraftWorld.getChunkAt(CraftWorld.java:108)
at com.github.StormTeam.Storm.Cuboid.getChunks(Cuboid.java:138)
at com.github.StormTeam.Storm.Cuboid.sendClientChanges(Cuboid.java:176)
at com.github.StormTeam.Storm.Volcano.VolcanoMaker.generateLayer(VolcanoMaker.java:177)
at com.github.StormTeam.Storm.Volcano.VolcanoMaker.generateVolcanoAboveGround(VolcanoMaker.java:164)
at com.github.StormTeam.Storm.Volcano.VolcanoMaker$1.run(VolcanoMaker.java:146)
at org.bukkit.craftbukkit.scheduler.CraftTask.run(CraftTask.java:53)
at org.bukkit.craftbukkit.scheduler.CraftAsyncTask.run(CraftAsyncTask.java:53)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)

In essence, both errors arise from Location.getBlock() & World.getChunkAt calls. From what bergerkiller wrote a while ago (and yes, this is a veiled attempt to ask for his help), getBlock() loads the chunk the block is in if it is not already loaded, as does getChunk(). Given that this section of code runs in an Async thread, there are specific complications.

The two alternatives I have found are creating a SYNC Bukkit thread and calling the getBlock there, and wrapping the entire method call as a Future call. Both of which significantly affect both the readability of the code, and (I would assume) its performance. Since the method in question is called a couple hundred times a second, performance is also key.

Any ideas? Or am I stuck wrapping in sync calls?

 

Icyene what about the most simple (and probably wrong, but just brainstorming here) solution and making the Async thread a Sync thread

 

fireblast709

I did consider that. However, as you could probably tell by the stack trace, I am attempting to make volcanoes. Due to the fact that they have random events occurring (explosions, eruptions etc), a Sync task could prove VERY cumbersome, as things would require to be timed, and instead of just a Thread.sleep I'd have to kill the thread, and store its process in a list/map/set. And then read from that list at another time to restart the threads.

So starting from 2 Async threads as I have now (one for volcano growing, the other for eruptions), I would end up with dozens of Sync task, and a massive Async task governing all of them. Not only would it require me to rewrite large sections of my code, it would also bloat it and make it even more unreadable than it is now (and trust me, with bitmasks/shifts in inlined ifs everywhere, its already pretty unreadable).

 

If you need to time things, use the scheduler, it will fire of your sync task as close as possible to when it is scheduled to.

 

Sagacious_Zed someones not reading the OP ;3

 

That was completely in response to his last post...

 

I know. Though it is no excuse to read the OP at least ;3. Now, lets get back to topic

Icyene how about (yea I know, this is going to sound like a shitty idea probably) writing a class that implements Runnable, which would cancel the current task on run() and start a new one with different variables. This would at least give you some flexibility of async tasks without giving up the 'sync'-ness

EDIT by Moderator: merged posts, please use the edit button instead of double posting.

 

Sagacious_Zed
Yeah chunk loads on another thread are triple worse. They can:

• Crash the server by throwing a concurrent modification exception because of added (tile)entities
• Cause other plugins handling the chunk load event to malfunction
• Cause serious issues if the chunk is generated (infinite loop/freeze/you name it)

If you really need to access a block from another thread, make sure you add a chunk load scheduler that manages the chunks you work on. Before doing your async stuff, load the chunk you are going to work in, and ensure that this chunk is kept loaded at all times and is released once you are done. Also make sure you do not cross your chunk boundaries when altering blocks.

Even then, changing blocks on another thread can cause physics to occur, so I recommend you directly change the blocks in the chunk, and avoid block.setType/TypeId at all times. Physics on another thread is equal bad as the chunk load, as it can cause items to drop and this way crash the server more easily.

 

bergerkiller
This might be better served if directed at the OP

 

Sagacious_Zed
My bad, thought you were the OP