Unload broken plugins fix (pf4j#545)

dre4merp · Sep 15, 2023 · 4715257 · 4715257
1 parent e4d7c7b
commit 4715257
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 6 deletions.
diff --git a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
@@ -280,15 +280,22 @@ protected boolean unloadPlugin(String pluginId, boolean unloadDependents) {
                     dependents.addAll(0, dependencyResolver.getDependents(dependent));
                 }
             }
+            PluginWrapper pluginWrapper = getPlugin(pluginId);
+            PluginState pluginState;
+            try {
+                pluginState = stopPlugin(pluginId, false);
+                if (PluginState.STARTED == pluginState) {
+                    return false;
+                }
 
-            PluginState pluginState = stopPlugin(pluginId, false);
-            if (PluginState.STARTED == pluginState) {
-                return false;
+                log.info("Unload plugin '{}'", getPluginLabel(pluginWrapper.getDescriptor()));
+            } catch (Exception e) {
+                if (pluginWrapper == null) {
+                    return false;
+                }
+                pluginState = PluginState.FAILED;
             }
 
-            PluginWrapper pluginWrapper = getPlugin(pluginId);
-            log.info("Unload plugin '{}'", getPluginLabel(pluginWrapper.getDescriptor()));
-
             // remove the plugin
             plugins.remove(pluginId);
             getResolvedPlugins().remove(pluginWrapper);

diff --git a/pf4j/src/test/java/org/pf4j/JarPluginManagerTest.java b/pf4j/src/test/java/org/pf4j/JarPluginManagerTest.java
@@ -16,17 +16,23 @@
 package org.pf4j;
 
 import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledOnOs;
+import org.junit.jupiter.api.condition.OS;
 import org.junit.jupiter.api.io.TempDir;
 import org.pf4j.test.PluginJar;
 import org.pf4j.test.TestExtension;
 import org.pf4j.test.TestExtensionPoint;
 import org.pf4j.test.TestPlugin;
 
 import java.io.IOException;
+import java.nio.file.FileSystemException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
+import java.util.concurrent.CompletableFuture;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -95,4 +101,28 @@ public void deletePlugin() throws Exception {
         assertFalse(pluginJar.file().exists());
     }
 
+    @Test
+    @EnabledOnOs(OS.WINDOWS)
+    public void releaseBrokenJarOnWindows() throws IOException {
+        PluginJar pluginZip = new PluginJar.Builder(pluginsPath.resolve("test.jar"), "test")
+            .pluginVersion("1.2.3")
+            .pluginClass("invalidClass")
+            .build();
+
+        pluginManager.loadPlugins();
+        Path pluginPath = pluginManager.getPlugin(pluginZip.pluginId()).getPluginPath();
+
+        try {
+            pluginManager.startPlugin(pluginZip.pluginId());
+        } catch (Exception exceptionStartPlugin) {
+            Assertions.assertThrows(FileSystemException.class, () -> Files.delete(pluginPath));
+
+            // Try to remove the plugin if it cannot be started
+            try {
+                pluginManager.unloadPlugin(pluginZip.pluginId());
+            } catch (Exception ignored2) {
+            }
+            Assertions.assertDoesNotThrow(() -> Files.delete(pluginPath));
+        }
+    }
 }