Copy dispatcher root children before passing it into async tree building

Author: kennytv

patches/server/0273-Async-command-map-building.patch

@@ -9,15 +9,17 @@ commands if the server is restarting. Using the default async pool caused issues
 due to the shutdown logic generally being much later.
 
 diff --git a/src/main/java/net/minecraft/commands/Commands.java b/src/main/java/net/minecraft/commands/Commands.java
-index 2008fd542eaf1c2fac776ae1751c227a3b6dde4b..34564f4118eab9e95176cb4ff4bc93fbe97d9b6b 100644
+index 2008fd542eaf1c2fac776ae1751c227a3b6dde4b..938db2436e0f099432263aba593e684eb89a44f9 100644
 --- a/src/main/java/net/minecraft/commands/Commands.java
 +++ b/src/main/java/net/minecraft/commands/Commands.java
-@@ -455,6 +455,19 @@ public class Commands {
+@@ -455,6 +455,21 @@ public class Commands {
          if ( org.spigotmc.SpigotConfig.tabComplete < 0 ) return; // Spigot
          // CraftBukkit start
          // Register Vanilla commands into builtRoot as before
 +        // Paper start - Perf: Async command map building
-+        COMMAND_SENDING_POOL.execute(() -> this.sendAsync(player));
++        // Copy root children to avoid concurrent modification during building
++        final Collection<CommandNode<CommandSourceStack>> commandNodes = new java.util.ArrayList<>(this.dispatcher.getRoot().getChildren());
++        COMMAND_SENDING_POOL.execute(() -> this.sendAsync(player, commandNodes));
 +    }
 +
 +    public static final java.util.concurrent.ExecutorService COMMAND_SENDING_POOL = java.util.concurrent.Executors.newFixedThreadPool(2,
@@ -27,12 +29,19 @@ index 2008fd542eaf1c2fac776ae1751c227a3b6dde4b..34564f4118eab9e95176cb4ff4bc93fb
 +            .build()
 +    );
 +
-+    private void sendAsync(ServerPlayer player) {
++    private void sendAsync(ServerPlayer player, Collection<CommandNode<CommandSourceStack>> dispatcherRootChildren) {
 +        // Paper end - Perf: Async command map building
          Map<CommandNode<CommandSourceStack>, CommandNode<SharedSuggestionProvider>> map = Maps.newIdentityHashMap(); // Use identity to prevent aliasing issues
          RootCommandNode vanillaRoot = new RootCommandNode();
 
-@@ -472,7 +485,14 @@ public class Commands {
+@@ -466,13 +481,20 @@ public class Commands {
+         RootCommandNode<SharedSuggestionProvider> rootcommandnode = new RootCommandNode();
+ 
+         map.put(this.dispatcher.getRoot(), rootcommandnode);
+-        this.fillUsableCommands(this.dispatcher.getRoot(), rootcommandnode, player.createCommandSourceStack(), map);
++        this.fillUsableCommands(dispatcherRootChildren, rootcommandnode, player.createCommandSourceStack(), map); // Paper - Perf: Async command map building; pass copy of children
+ 
+         Collection<String> bukkit = new LinkedHashSet<>();
          for (CommandNode node : rootcommandnode.getChildren()) {
              bukkit.add(node.getName());
          }
@@ -47,6 +56,28 @@ index 2008fd542eaf1c2fac776ae1751c227a3b6dde4b..34564f4118eab9e95176cb4ff4bc93fb
          PlayerCommandSendEvent event = new PlayerCommandSendEvent(player.getBukkitEntity(), new LinkedHashSet<>(bukkit));
          event.getPlayer().getServer().getPluginManager().callEvent(event);
 
+@@ -486,8 +508,10 @@ public class Commands {
+         player.connection.send(new ClientboundCommandsPacket(rootcommandnode));
+     }
+ 
+-    private void fillUsableCommands(CommandNode<CommandSourceStack> tree, CommandNode<SharedSuggestionProvider> result, CommandSourceStack source, Map<CommandNode<CommandSourceStack>, CommandNode<SharedSuggestionProvider>> resultNodes) {
+-        Iterator iterator = tree.getChildren().iterator();
++    // Paper start - Perf: Async command map building; pass copy of children
++    private void fillUsableCommands(Collection<CommandNode<CommandSourceStack>> children, CommandNode<SharedSuggestionProvider> result, CommandSourceStack source, Map<CommandNode<CommandSourceStack>, CommandNode<SharedSuggestionProvider>> resultNodes) {
++        Iterator iterator = children.iterator();
++        // Paper end - Perf: Async command map building
+ 
+         while (iterator.hasNext()) {
+             CommandNode<CommandSourceStack> commandnode2 = (CommandNode) iterator.next();
+@@ -522,7 +546,7 @@ public class Commands {
+                 resultNodes.put(commandnode2, commandnode3);
+                 result.addChild(commandnode3);
+                 if (!commandnode2.getChildren().isEmpty()) {
+-                    this.fillUsableCommands(commandnode2, commandnode3, source, resultNodes);
++                    this.fillUsableCommands(commandnode2.getChildren(), commandnode3, source, resultNodes); // Paper - Perf: Async command map building; pass children directly
+                 }
+             }
+         }
 diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java
 index 9a9291282f7ec99be9badf8b32fab5f0ea0f1037..feb45369b41b597fd603c12f3da23759923b6a6d 100644
 --- a/src/main/java/net/minecraft/server/MinecraftServer.java

patches/server/0274-Brigadier-Mojang-API.patch

@@ -87,26 +87,26 @@ index 4d5f1dd1c3bd742b1bc5e3914101a699041caa7e..5316f148f3f9128690f019d544e462b0
      public boolean hasPermission(int level) {
          // CraftBukkit start
 diff --git a/src/main/java/net/minecraft/commands/Commands.java b/src/main/java/net/minecraft/commands/Commands.java
-index 34564f4118eab9e95176cb4ff4bc93fbe97d9b6b..c2c8193a3b3f7a224b05a09cd65108860aaae814 100644
+index 938db2436e0f099432263aba593e684eb89a44f9..71c8692487c9c1c5b956b4356501d523368c0507 100644
 --- a/src/main/java/net/minecraft/commands/Commands.java
 +++ b/src/main/java/net/minecraft/commands/Commands.java
-@@ -486,6 +486,7 @@ public class Commands {
+@@ -488,6 +488,7 @@ public class Commands {
              bukkit.add(node.getName());
          }
          // Paper start - Perf: Async command map building
 +        new com.destroystokyo.paper.event.brigadier.AsyncPlayerSendCommandsEvent<CommandSourceStack>(player.getBukkitEntity(), (RootCommandNode) rootcommandnode, false).callEvent(); // Paper - Brigadier API
          net.minecraft.server.MinecraftServer.getServer().execute(() -> {
             runSync(player, bukkit, rootcommandnode);
          });
-@@ -493,6 +494,7 @@ public class Commands {
+@@ -495,6 +496,7 @@ public class Commands {
 
      private void runSync(ServerPlayer player, Collection<String> bukkit, RootCommandNode<SharedSuggestionProvider> rootcommandnode) {
          // Paper end - Perf: Async command map building
 +        new com.destroystokyo.paper.event.brigadier.AsyncPlayerSendCommandsEvent<CommandSourceStack>(player.getBukkitEntity(), (RootCommandNode) rootcommandnode, true).callEvent(); // Paper - Brigadier API
          PlayerCommandSendEvent event = new PlayerCommandSendEvent(player.getBukkitEntity(), new LinkedHashSet<>(bukkit));
          event.getPlayer().getServer().getPluginManager().callEvent(event);
 
-@@ -511,6 +513,11 @@ public class Commands {
+@@ -515,6 +517,11 @@ public class Commands {
 
          while (iterator.hasNext()) {
              CommandNode<CommandSourceStack> commandnode2 = (CommandNode) iterator.next();

patches/server/0953-Brigadier-based-command-API.patch

@@ -2068,7 +2068,7 @@ index fc0c60b22844ed010aede2fa125b9fa440d3de80..3549ffea451b932602efb113844ba21a
      public org.bukkit.command.CommandSender getBukkitSender() {
          return this.source.getBukkitSender(this);
 diff --git a/src/main/java/net/minecraft/commands/Commands.java b/src/main/java/net/minecraft/commands/Commands.java
-index 26bc8d7075f8a9c411776ab91aeddcd776d21348..2fc5651d1546fd4bb10b0617540e906f483cb36c 100644
+index f190a65ee2e091d4d5a1e114178eac42740075d4..622677fb281242681bf8ba39de34187bcc898a5f 100644
 --- a/src/main/java/net/minecraft/commands/Commands.java
 +++ b/src/main/java/net/minecraft/commands/Commands.java
 @@ -159,7 +159,7 @@ public class Commands {
@@ -2135,8 +2135,8 @@ index 26bc8d7075f8a9c411776ab91aeddcd776d21348..2fc5651d1546fd4bb10b0617540e906f
                  StackTraceElement[] astacktraceelement = exception.getStackTrace();
 
                  for (int i = 0; i < Math.min(astacktraceelement.length, 3); ++i) {
-@@ -473,13 +492,7 @@ public class Commands {
-     private void sendAsync(ServerPlayer player) {
+@@ -475,13 +494,7 @@ public class Commands {
+     private void sendAsync(ServerPlayer player, Collection<CommandNode<CommandSourceStack>> dispatcherRootChildren) {
          // Paper end - Perf: Async command map building
          Map<CommandNode<CommandSourceStack>, CommandNode<SharedSuggestionProvider>> map = Maps.newIdentityHashMap(); // Use identity to prevent aliasing issues
 -        RootCommandNode vanillaRoot = new RootCommandNode();
@@ -2150,15 +2150,15 @@ index 26bc8d7075f8a9c411776ab91aeddcd776d21348..2fc5651d1546fd4bb10b0617540e906f
          RootCommandNode<SharedSuggestionProvider> rootcommandnode = new RootCommandNode();
 
          map.put(this.dispatcher.getRoot(), rootcommandnode);
-@@ -513,6 +526,7 @@ public class Commands {
-     }
+@@ -516,6 +529,7 @@ public class Commands {
 
-     private void fillUsableCommands(CommandNode<CommandSourceStack> tree, CommandNode<SharedSuggestionProvider> result, CommandSourceStack source, Map<CommandNode<CommandSourceStack>, CommandNode<SharedSuggestionProvider>> resultNodes) {
+     // Paper start - Perf: Async command map building; pass copy of children
+     private void fillUsableCommands(Collection<CommandNode<CommandSourceStack>> children, CommandNode<SharedSuggestionProvider> result, CommandSourceStack source, Map<CommandNode<CommandSourceStack>, CommandNode<SharedSuggestionProvider>> resultNodes) {
 +        resultNodes.keySet().removeIf((node) -> !org.spigotmc.SpigotConfig.sendNamespaced && node.getName().contains( ":" )); // Paper - Remove namedspaced from result nodes to prevent redirect trimming ~ see comment below
-         Iterator iterator = tree.getChildren().iterator();
+         Iterator iterator = children.iterator();
+         // Paper end - Perf: Async command map building
 
-         while (iterator.hasNext()) {
-@@ -526,6 +540,42 @@ public class Commands {
+@@ -530,6 +544,42 @@ public class Commands {
 
              if (commandnode2.canUse(source)) {
                  ArgumentBuilder argumentbuilder = commandnode2.createBuilder(); // CraftBukkit - decompile error

patches/server/0985-Fix-entity-type-tags-suggestions-in-selectors.patch

@@ -37,18 +37,18 @@ index 3549ffea451b932602efb113844ba21a7bc72371..13bd145b1e8006a53c22f5dc0c78f29b
 +    // Paper end - tell clients to ask server for suggestions for EntityArguments
  }
 diff --git a/src/main/java/net/minecraft/commands/Commands.java b/src/main/java/net/minecraft/commands/Commands.java
-index 2fc5651d1546fd4bb10b0617540e906f483cb36c..f58275101fd5671bab0aa66875c989e568384647 100644
+index 622677fb281242681bf8ba39de34187bcc898a5f..daca005854ae2bb68300b19bceedadcbbe1e06fd 100644
 --- a/src/main/java/net/minecraft/commands/Commands.java
 +++ b/src/main/java/net/minecraft/commands/Commands.java
-@@ -529,6 +529,7 @@ public class Commands {
-         resultNodes.keySet().removeIf((node) -> !org.spigotmc.SpigotConfig.sendNamespaced && node.getName().contains( ":" )); // Paper - Remove namedspaced from result nodes to prevent redirect trimming ~ see comment below
-         Iterator iterator = tree.getChildren().iterator();
+@@ -533,6 +533,7 @@ public class Commands {
+         Iterator iterator = children.iterator();
+         // Paper end - Perf: Async command map building
 
 +        boolean registeredAskServerSuggestionsForTree = false; // Paper - tell clients to ask server for suggestions for EntityArguments
          while (iterator.hasNext()) {
              CommandNode<CommandSourceStack> commandnode2 = (CommandNode) iterator.next();
              // Paper start - Brigadier API
-@@ -591,6 +592,12 @@ public class Commands {
+@@ -595,6 +596,12 @@ public class Commands {
 
                      if (requiredargumentbuilder.getSuggestionsProvider() != null) {
                          requiredargumentbuilder.suggests(SuggestionProviders.safelySwap(requiredargumentbuilder.getSuggestionsProvider()));