Error in my rollback system (ArrayLists)

Hitzk0pf · February 11, 2016, 7:40pm

Hey,
(Error: ConcurrentModificationException)
I know, that there is a problem with adding / removing items to / from ArrayLists while looping the same ArrayList.
Although I do not do that, I get this error again and again…

My Server (SpongeForge) crashed because of this error a few times…

What do I want to do?
So i have not so much experience in Java yet, so I will try to explain, what I want to do, so you can say me, if thats completely stupid / if there is a better way…

When a player breaks / places a block, I save its id and variant in my mysql database. There are 2 tables (***_break and ***_place).

When I type in a command, I will get all broken & placed blocks of a player
Next this data is going to be saved in 2 ArrayLists
Now I loop through my ArrayLists and restore all destroyed blocks / destroy all placed blocks (by replacing them with air).

For that, I use a new Thread, which will sleep for a few millisecs in every loop passage, to let it look, like the blocks are disappearing / appearing slowly.

This is the code, that I have described above (only the loop of destroyed blocks, the second loop is similar):

//replace removed blocks
for (RollbackBlock block : blocks_break) {            
        try {
            sleep(200);
        } catch (InterruptedException e) {
            e.printStackTrace();
        }

        Location location = new Location(world, block.getX(), block.getY(), block.getZ());

        Integer x = location.getBlockX();
        Integer y = location.getBlockY();
        Integer z = location.getBlockZ();

        if (!block.hasVariant()) {

            location.getExtent().setBlock(x, y, z, Sponge.getRegistry().getType(BlockType.class, block.getId()).get().getDefaultState(), false, cause);

        } else {

            Optional<BlockType> optBlock = Sponge.getRegistry().getType(BlockType.class, block.getId());

            if (optBlock.isPresent()) {

                BlockState defaultState = optBlock.get().getDefaultState();
                Optional<BlockTrait<?>> optTrait = defaultState.getTrait("variant");

                if (optTrait.isPresent()) {

                    Optional<BlockState> optState = defaultState.withTrait(optTrait.get(), block.getVariant());

                    if (optState.isPresent()) {

                        BlockState blockState = optState.get();
                        location.getExtent().setBlock(x, y, z, blockState, false, cause);
                    }
                }

            }

        }
    }

This is how I start the new thread:

Thread t1 = new Thread(new Runnable() {
                public void run() {
                    try {
                        new UserPlotRollbackTool(user, logger);

                    } catch (IOException e) {
                        e.printStackTrace();
                    }
                }
            });
            t1.start();
            t1.setName("Normalize Plot");

Forge crash error:
---- Minecraft Crash Report ----

WARNING: coremods are present:
  SpongeCoremod (spongeforge-1.8.9-1732-3.1.0-BETA-1139.jar)
Contact their authors BEFORE contacting forge

// Don't be sad. I'll do better next time, I promise!

Time: 11.02.16 20:11
Description: Ticking entity

java.util.ConcurrentModificationException
	at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:901)
	at java.util.ArrayList$Itr.remove(ArrayList.java:865)
	at net.minecraft.world.World.handleBlockCaptures(World.java:884)
	at net.minecraft.world.World.handlePostTickCaptures(World.java:776)
	at net.minecraft.world.World.onCallEntityUpdate(World.java:544)
	at net.minecraft.world.World.func_72866_a(World.java:1860)
	at net.minecraft.world.WorldServer.func_72866_a(WorldServer.java:699)
	at net.minecraft.world.World.func_72870_g(World.java:1829)
	at net.minecraft.world.World.func_72939_s(World.java:1661)
	at net.minecraft.world.WorldServer.func_72939_s(WorldServer.java:544)
	at net.minecraft.server.MinecraftServer.func_71190_q(MinecraftServer.java:701)
	at net.minecraft.server.dedicated.DedicatedServer.func_71190_q(DedicatedServer.java:344)
	at net.minecraft.server.MinecraftServer.func_71217_p(MinecraftServer.java:605)
	at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:481)
	at java.lang.Thread.run(Thread.java:745)


A detailed walkthrough of the error, its code path and all known details is as follows:
---------------------------------------------------------------------------------------

-- Head --
Stacktrace:
	at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:901)
	at java.util.ArrayList$Itr.remove(ArrayList.java:865)
	at net.minecraft.world.World.handleBlockCaptures(World.java:884)
	at net.minecraft.world.World.handlePostTickCaptures(World.java:776)
	at net.minecraft.world.World.onCallEntityUpdate(World.java:544)
	at net.minecraft.world.World.func_72866_a(World.java:1860)
	at net.minecraft.world.WorldServer.func_72866_a(WorldServer.java:699)
	at net.minecraft.world.World.func_72870_g(World.java:1829)

-- Entity being ticked --
Details:
	Entity Type: Skeleton (net.minecraft.entity.monster.EntitySkeleton)
	Entity ID: 32478
	Entity Name: Skeleton
	Entity's Exact location: -386,80, 29,00, 315,24
	Entity's Block location: -387,00,29,00,315,00 - World: (-387,29,315), Chunk: (at 13,1,11 in -25,19; contains blocks -400,0,304 to -385,255,319), Region: (-1,0; contains chunks -32,0 to -1,31, blocks -512,0,0 to -1,255,511)
	Entity's Momentum: 0,00, -0,08, 0,00
	Entity's Rider: ~~ERROR~~ NullPointerException: null
	Entity's Vehicle: ~~ERROR~~ NullPointerException: null
Stacktrace:
	at net.minecraft.world.World.func_72939_s(World.java:1661)
	at net.minecraft.world.WorldServer.func_72939_s(WorldServer.java:544)

-- Affected level --
Details:
	Level name: freebuild
	All players: 1 total; [EntityPlayerMP['Hitzk0pf'/1078, l='freebuild', x=-404,66, y=62,97, z=193,14]]
	Chunk stats: ServerChunkCache: 591 Drop: 0
	Level seed: 1370881112180462787
	Level generator: ID 00 - default, ver 1. Features enabled: false
	Level generator options: 
	Level spawn location: -402,00,69,00,86,00 - World: (-402,69,86), Chunk: (at 14,4,6 in -26,5; contains blocks -416,0,80 to -401,255,95), Region: (-1,0; contains chunks -32,0 to -1,31, blocks -512,0,0 to -1,255,511)
	Level time: 4692379 game time, 105761 day time
	Level dimension: 2
	Level storage version: 0x04ABD - Anvil
	Level weather: Rain time: 1 (now: false), thunder time: 1 (now: false)
	Level game mode: Game mode: survival (ID 0). Hardcore: false. Cheats: false
Stacktrace:
	at net.minecraft.server.MinecraftServer.func_71190_q(MinecraftServer.java:701)
	at net.minecraft.server.dedicated.DedicatedServer.func_71190_q(DedicatedServer.java:344)
	at net.minecraft.server.MinecraftServer.func_71217_p(MinecraftServer.java:605)
	at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:481)
	at java.lang.Thread.run(Thread.java:745)

-- System Details --
Details:
	Minecraft Version: 1.8.9
	Operating System: Windows 10 (amd64) version 10.0
	Java Version: 1.8.0_72, Oracle Corporation
	Java VM Version: Java HotSpot(TM) 64-Bit Server VM (mixed mode), Oracle Corporation
	Memory: 232749864 bytes (221 MB) / 876085248 bytes (835 MB) up to 954728448 bytes (910 MB)
	JVM Flags: 2 total; -Xmx1024M -Xms512M
	IntCache: cache: 0, tcache: 0, allocated: 0, tallocated: 0
	FML: MCP 9.19 Powered by Forge 11.15.0.1715 10 mods loaded, 10 mods active
	States: 'U' = Unloaded 'L' = Loaded 'C' = Constructed 'H' = Pre-initialized 'I' = Initialized 'J' = Post-initialized 'A' = Available 'D' = Disabled 'E' = Errored
	UCHIJAAAA	mcp{9.18} [Minecraft Coder Pack] (minecraft.jar) 
	UCHIJAAAA	FML{8.0.99.99} [Forge Mod Loader] (forge.jar) 
	UCHIJAAAA	Forge{11.15.0.1715} [Minecraft Forge] (forge.jar) 
	UCHIJAAAA	sponge{1.8.9-1732-3.1.0-BETA-1139} [SpongeForge] (spongeforge-1.8.9-1732-3.1.0-BETA-1139.jar) 
	UCHIJAAAA	CaveCore{1.1} [CaveCore] (CaveCore.jar) 
	UCHIJAAAA	PJP{0.8.29} [Project Portals] (ProjectPortals-3.1.0-v0.8.29.jar) 
	UCHIJAAAA	PJW{0.6.40} [Project Worlds] (ProjectWorlds-3.1.0-v0.6.40.jar) 
	UCHIJAAAA	voxelsniperforge{7.1.0} [VoxelSniper-Forge] (VoxelSniper-7.1.0-SNAPSHOT-all.jar) 
	UCHIJAAAA	voxelsnipersponge{7.1.0} [VoxelSniper-Sponge] (VoxelSniper-7.1.0-SNAPSHOT-all.jar) 
	UCHIJAAAA	worldedit{6.1.1-SNAPSHOT} [WorldEdit] (worldedit-spongevanilla-mc1.8.9-6.1.1-SNAPSHOT-dist.jar) 
	Loaded coremods (and transformers): 
SpongeCoremod (spongeforge-1.8.9-1732-3.1.0-BETA-1139.jar)
  org.spongepowered.common.launch.transformer.SpongeSuperclassTransformer
	Profiler Position: N/A (disabled)
	Player Count: 1 / 20; [EntityPlayerMP['Hitzk0pf'/1078, l='freebuild', x=-404,66, y=62,97, z=193,14]]
	Is Modded: Definitely; Server brand changed to 'fml,forge,SpongeForge'
	Type: Dedicated Server (map_server.txt)

My Questions:

Why do I get this error?
Is there a better way to realize this function?

Thanks

simon816 · February 11, 2016, 7:56pm

And that is where it’s going wrong

Minecraft is single threaded; it is only safe to touch things (like world data) when you’re in the main thread. What you need to do is use the Scheduler in order to run tasks in line with the rest of the game.
Here’s a simple template.

Sponge.getScheduler().createTaskBuilder()
    .execute(task -> {
        // Stuff here
    })
    .delay() or .interval()
    .submit(MyPlugin.getInstance());

More information can be found on the docs, https://docs.spongepowered.org/en/plugin/scheduler.html
Also the javadoc for Task.Builder#async describes what an ‘asynchronous’ and a ‘synchronous’ task is
https://jd.spongepowered.org/3.0.0/org/spongepowered/api/scheduler/Task.Builder.html#async--

Hitzk0pf · February 11, 2016, 8:05pm

Yeah, I already have schedulers in my plugin
Is there a way to destroy a scheduler when he did his task?
Because I would have to use multiple schedulers (remove the blocks of multiple users at the same time)

simon816 · February 11, 2016, 8:12pm

Yes, when building the task, the execute method takes a Consumer<Task> which means the body that you want to run can take the task as a parameter.
You can then cancel the task at any time.

.execute(task -> {
    if (somethingHappened) {
        task.cancel();
    }
})

The task builder also returns the task object so you can save the task object for use in other parts of the plugin

Hitzk0pf · February 12, 2016, 9:07am

another question:
as I said, I’m not too good when it comes to develop with java…
Is there a better way to perform rollbacks, or do I have to store each block in my database?
Thanks

viveleroi · February 12, 2016, 5:22pm

You’ll need to store every block snapshot from a transaction. You can look at Prism’s source for some examples but it listens to block events, iterates the transactions, and stores the blocksnapshot data to a storage engine. On rollback, it reconstructs the block snapshot and re-applies it to the world.