Improved fix for PaperMC#1372

Realized that the precondition that gets violated happens way too early, in `Maps.uniqueIndex()`.
clrxbl · Jul 6, 2024 · 5154f02 · 5154f02
1 parent 6224adf
commit 5154f02
Showing 1 changed file with 17 additions and 15 deletions.
diff --git a/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java b/proxy/src/main/java/com/velocitypowered/proxy/plugin/VelocityPluginManager.java
@@ -86,43 +86,45 @@ public void loadPlugins(Path directory) throws IOException {
     checkNotNull(directory, "directory");
     checkArgument(directory.toFile().isDirectory(), "provided path isn't a directory");
 
-    List<PluginDescription> found = new ArrayList<>();
+    Map<String, PluginDescription> foundCandidates = new LinkedHashMap<>();
     JavaPluginLoader loader = new JavaPluginLoader(server, directory);
 
     try (DirectoryStream<Path> stream = Files.newDirectoryStream(directory,
         p -> p.toFile().isFile() && p.toString().endsWith(".jar"))) {
       for (Path path : stream) {
         try {
-          found.add(loader.loadCandidate(path));
+          PluginDescription candidate = loader.loadCandidate(path);
+
+          // If we found a duplicate candidate (with the same ID), don't load it.
+          PluginDescription maybeExistingCandidate = foundCandidates.putIfAbsent(
+              candidate.getId(), candidate);
+
+          if (maybeExistingCandidate != null) {
+            logger.error("Refusing to load plugin at path {} since we already "
+                    + "loaded a plugin with the same ID {} from {}",
+                candidate.getSource().map(Objects::toString).orElse("<UNKNOWN>"),
+                candidate.getId(),
+                maybeExistingCandidate.getSource().map(Objects::toString).orElse("<UNKNOWN>"));
+          }
         } catch (Throwable e) {
           logger.error("Unable to load plugin {}", path, e);
         }
       }
     }
 
-    if (found.isEmpty()) {
+    if (foundCandidates.isEmpty()) {
       // No plugins found
       return;
     }
 
-    List<PluginDescription> sortedPlugins = PluginDependencyUtils.sortCandidates(found);
+    List<PluginDescription> sortedPlugins = PluginDependencyUtils.sortCandidates(
+        new ArrayList<>(foundCandidates.values()));
 
     Map<String, PluginDescription> loadedCandidates = new HashMap<>();
     Map<PluginContainer, Module> pluginContainers = new LinkedHashMap<>();
     // Now load the plugins
     pluginLoad:
     for (PluginDescription candidate : sortedPlugins) {
-      // If we found a duplicate candidate (with the same ID), don't load it.
-      PluginDescription existingCandidate = loadedCandidates.get(candidate.getId());
-      if (existingCandidate != null) {
-        logger.error("Refusing to load plugin at path {} since we already "
-            + "loaded a plugin with the same ID {} from {}",
-            candidate.getSource().map(Objects::toString).orElse("<UNKNOWN>"),
-            candidate.getId(),
-            existingCandidate.getSource().map(Objects::toString).orElse("<UNKNOWN>"));
-        continue;
-      }
-
       // Verify dependencies
       for (PluginDependency dependency : candidate.getDependencies()) {
         if (!dependency.isOptional() && !loadedCandidates.containsKey(dependency.getId())) {