[SPIGOT-7089] Concurrent error crashes server when command block runs a command that creates or unloads a world Created: 29/Jun/22 Updated: 25/Dec/24 Resolved: 14/Aug/22 |
|
| Status: | Resolved |
| Project: | Spigot |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Minor |
| Reporter: | Will Kroboth | Assignee: | Unassigned |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Attachments: |
|
| Version: | This server is running CraftBukkit version 3540-Spigot-56be6a8-0231a37 (MC: 1.19) (Implementing API version 1.19-R0.1-SNAPSHOT) |
| Guidelines Read: | Yes |
| Description |
|
This report is based on Paper#6072. The attached plugin adds the command /testloadworld, which runs the following code:
@Override
public boolean onCommand(@NotNull CommandSender sender, @NotNull Command command, @NotNull String label, @NotNull String[] args) {
World world = WorldCreator.name("testworld")
.environment(World.Environment.NORMAL)
.type(WorldType.NORMAL)
.createWorld();
getLogger().info(String.valueOf(world));
return true;
}
When a player or the console runs this command, a new world is created without issue. However, when a command block runs the command, the server promptly crashes after the world is created with the following error: [11:05:37] [Server thread/ERROR]: Encountered an unexpected exception java.util.ConcurrentModificationException: null at java.util.LinkedHashMap$LinkedHashIterator.nextNode(LinkedHashMap.java:756) ~[?:?] at java.util.LinkedHashMap$LinkedValueIterator.next(LinkedHashMap.java:783) ~[?:?] at net.minecraft.server.MinecraftServer.b(MinecraftServer.java:1273) ~[spigot-1.19-R0.1-SNAPSHOT.jar:3540-Spigot-56be6a8-0231a37] at net.minecraft.server.dedicated.DedicatedServer.b(DedicatedServer.java:394) ~[spigot-1.19-R0.1-SNAPSHOT.jar:3540-Spigot-56be6a8-0231a37] at net.minecraft.server.MinecraftServer.a(MinecraftServer.java:1197) ~[spigot-1.19-R0.1-SNAPSHOT.jar:3540-Spigot-56be6a8-0231a37] at net.minecraft.server.MinecraftServer.v(MinecraftServer.java:1010) ~[spigot-1.19-R0.1-SNAPSHOT.jar:3540-Spigot-56be6a8-0231a37] at net.minecraft.server.MinecraftServer.lambda$0(MinecraftServer.java:291) ~[spigot-1.19-R0.1-SNAPSHOT.jar:3540-Spigot-56be6a8-0231a37] at java.lang.Thread.run(Thread.java:833) [?:?] The paper issue mentioned above was resolved by this pull request, which adds a check to make sure a world is not created while the worlds are being ticked. This explains the issue to developers more clearly with the following exception:
java.lang.IllegalStateException: Cannot create a world while worlds are being ticked
Even though this issue was resolved by Paper, a similar fix should probably exist for Spigot. Throwing an error that explains why the crash happens would be helpful. I don't know if this already exsists, but it would also be helpful if there was a method plugin developers could use to tell if the worlds are being ticked so they can avoid triggering this crash in the first place |
| Comments |
| Comment by md_5 [ 14/Aug/22 ] |
|
Command blocks can now load worlds |
| Comment by Will Kroboth [ 17/Jul/22 ] |
|
Looking at solution #2 from my previous comment, I think the ConcurrentHashMap could replace LinkedHashMap as the class used for MinecraftServer.levels. Using the ConcurrentHashMap resolves this issue because items can be added or removed from it while it is being iterated through without throwing a ConcurrentModificationException. As far as I know, the only benefit of using LinkedHashMap is that the iteration order is guaranteed to be the same, which doesn't seem very important to maintain, so there shouldn't be a problem with switching to the ConcurrentHashMap. |
| Comment by Will Kroboth [ 15/Jul/22 ] |
|
I don't know if anyone is working on this, but here's why I think this problem happens: Looking at the stack trace, the code that throws the ConcurrentModificationException is in LinkedHashMap#nextNode: final LinkedHashMap.Entry<K,V> nextNode() { LinkedHashMap.Entry<K,V> e = next; if (modCount != expectedModCount) throw new ConcurrentModificationException(); // right here if (e == null) throw new NoSuchElementException(); current = e; next = e.after; return e; } If the size of the linkedHashMap changes while it is being iterated through, it throws the ConcurrentModificationException, crashing the server. The LinkedHashMap being edited in this case is MinecraftServer.levels, which is iterated through in MinecraftServer#tickChildren: public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTask> implements CommandSource, AutoCloseable { public final Map<ResourceKey<Level>, ServerLevel> levels; public MinecraftServer(...){ this.levels = Maps.newLinkedHashMap(); } public void tickChildren(BooleanSupplier booleansupplier) { ... Iterator iterator = this.getAllLevels().iterator(); ... while(iterator.hasNext()) { ... Activate command blocks ... } } } WorldCreator#createWorld() and Server#unloadWorld() eventually add or remove an entry from this map in the methods CraftServer#createWorld() and CraftServer#unloadWorld respectively: public final class CraftServer implements Server { public World createWorld(WorldCreator creator) { // Lots of code... this.console.initWorld(internal, worlddata, worlddata, worlddata.worldGenSettings()); internal.setSpawnSettings(true, true); this.console.levels.put(internal.dimension(), internal); // right here this.getServer().prepareLevels(internal.getChunkSource().chunkMap.progressListener, internal); // Some more code... } public boolean unloadWorld(World world, boolean save) { // Lots of code... this.worlds.remove(world.getName().toLowerCase(Locale.ENGLISH)); this.console.levels.remove(handle.dimension()); // right here return true; } } I've thought of three ways to fix this problem. This would get the job done quickly, avoiding the crash. A method could be added for plugin developers to call to check if the worlds are being ticked so they can avoid trying to create or unload a world in the first place if it is a bad time. 2. Change the class used by MinecraftServer.levels to one that allows ConcurrentModification I haven't tried looking for a class that works here, but this would clearly allow creating or unloading worlds while the worlds are being ticked, though it might have some weird side effects. 3. Let CraftServer#createWorld and CraftServer#unloadWorld do their thing but delay them from modifying MinecraftServer.levels until the worlds are no longer being ticked. This one would be the most complicated, and might // do everything else if(!worldsAreBeingTicked){ Modify MinecraftServer.levels now because it is safe } else { Add world to list so it can be loaded/unload later } This would allow createWorld and unloadWorld to be called while the worlds are being ticked, but not actually modify the Map of levels until it is safe to do so. |
| Comment by Will Kroboth [ 12/Jul/22 ] |
|
I've never seen a command not found error from running a non-vanilla command in a command block. I'm pretty sure I haven't done anything to the server to make it run the commands. I used the spigot-1.19.jar file generated by BuildTools, and only had the TestPlugin when replicating the issue. Here's an example of a server log where deleting the world using a command block crashed the server: https://paste.gg/p/anonymous/580108841c274aaca3d1a407af227032. You'd know better than me, but it doesn't look like anything special is happening (Other than the crash). |
| Comment by Doc [ 12/Jul/22 ] |
|
i follow the steps in Spigot and the crash happen, not sure about the non-vanilla commands in command block i use command blocks for custom commands in the past.
But following the steps i get the ConcurrentModificationException when the world i finishing of build by running in the command block. |
| Comment by md_5 [ 12/Jul/22 ] |
|
> Does that mean that -when a command block (whose text input -is a command -added by a plugin) -receives redstone power -nothing should happen? All non-vanilla commands give a command not found error. How are you testing this on Spigot? |
| Comment by Will Kroboth [ 12/Jul/22 ] |
|
What do you mean that Spigot doesn't support plugin commands in command blocks? Does that mean that -when a command block (whose text input -is a command -added by a plugin) -receives redstone power -nothing should happen? or Does that mean any issues stemming from putting plugin commands in command blocks are irrelevant because players should never consider putting plugin commands in a command block? To be more specific, here is the IntelliJ project I used to make the plugin jar file: TestPlugin.zip There is one java file: package dev.benergy10.testplugin; import org.bukkit.World; import org.bukkit.WorldCreator; import org.bukkit.WorldType; import org.bukkit.command.Command; import org.bukkit.command.CommandSender; import org.bukkit.plugin.java.JavaPlugin; public final class TestPlugin extends JavaPlugin { @Override public void onEnable() { // Plugin startup logic getCommand("testloadworld").setExecutor(this); } @Override public void onDisable() { // Plugin shutdown logic } @Override public boolean onCommand(CommandSender sender, Command command, String label, String[] args) { if (args.length != 1) return false; String choice = args[0]; switch (choice) { case "create" -> { World world = WorldCreator.name("testworld") .environment(World.Environment.NORMAL) .type(WorldType.NORMAL) .createWorld(); getLogger().info(String.valueOf(world)); return true; } case "delete" -> { boolean result = getServer().unloadWorld("testworld", true); getLogger().info(result ? "Deleted successfully" : "Deleting failed"); return result; } default -> { return false; } } } } The plugin.yml looks like this: name: TestPlugin version: ${project.version} main: dev.benergy10.testplugin.TestPlugin api-version: 1.19 commands: testloadworld: description: Testing world load It is a maven project whose pom.xml looks like this: <?xml version="1.0" encoding="UTF-8"?> <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> <modelVersion>4.0.0</modelVersion> <groupId>dev.benergy10</groupId> <artifactId>TestPlugin</artifactId> <version>1.0-SNAPSHOT</version> <packaging>jar</packaging> <name>TestPlugin</name> <properties> <java.version>1.8</java.version> <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> </properties> <build> <plugins> <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-compiler-plugin</artifactId> <version>3.8.1</version> <configuration> <source>14</source> <target>14</target> </configuration> </plugin> <plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-shade-plugin</artifactId> <version>3.2.4</version> <executions> <execution> <phase>package</phase> <goals> <goal>shade</goal> </goals> <configuration> <createDependencyReducedPom>false</createDependencyReducedPom> </configuration> </execution> </executions> </plugin> </plugins> <resources> <resource> <directory>src/main/resources</directory> <filtering>true</filtering> </resource> </resources> </build> <repositories> <repository> <id>spigot-repo</id> <url>https://hub.spigotmc.org/nexus/content/repositories/snapshots/</url> </repository> </repositories> <dependencies> <dependency> <groupId>org.spigotmc</groupId> <artifactId>spigot-api</artifactId> <version>1.19-R0.1-SNAPSHOT</version> <scope>provided</scope> </dependency> </dependencies> </project> If you're looking for something else, feel free to ask directly |
| Comment by md_5 [ 12/Jul/22 ] |
|
> Second of all, I can't reproduce this because Spigot doesn't support plugin commands in command blocks? Can you please be more specific with reproduction steps |
| Comment by Will Kroboth [ 10/Jul/22 ] |
|
Sure, if a solution exists that would be better.
Steps to reproduce (Updated to show the issue with Server#unloadWorld() as well): 1. Have a command that runs WorldCreator#create() or Server#unloadWorld(). This plugin jar (TestPlugin-1.0-SNAPSHOT.jar
@Override
public boolean onCommand(@NotNull CommandSender sender, @NotNull Command command, @NotNull String label, @NotNull String[] args) {
if (args.length != 1)
return false;
String choice = args[0];
switch (choice) {
case "create" -> {
World world = WorldCreator.name("testworld")
.environment(World.Environment.NORMAL)
.type(WorldType.NORMAL)
.createWorld();
getLogger().info(String.valueOf(world));
return true;
}
case "delete" -> {
boolean result = getServer().unloadWorld("testworld", true);
getLogger().info(result ? "Deleted successfully" : "Deleting failed");
return result;
}
default -> {
return false;
}
}
}
Running /testloadworld create will create a world called "testworld", and running /testloadworld delete will unload the world called "testworld". 2. Put the command /testloadworld create in a command block. 3. Activate the command block with a button. The server crashes as mentioned before. 4. Start the server again. Run /testloadworld create as the console or player. 5. Put the command /testloadworld delete in a command block. 6. Activate the command block with a button. The server crashes as mentioned before. |
| Comment by md_5 [ 10/Jul/22 ] |
|
First of all, it would be better to fix the issue than throw an exception. Second of all, I can't reproduce this because Spigot doesn't support plugin commands in command blocks? Can you please be more specific with reproduction steps |
| Comment by Will Kroboth [ 01/Jul/22 ] |
|
A similar error occurs when Server#unloadWorld() is called while the worlds are being ticked, and this was fixed in Paper by Paper#8081. |