From 4715257de1e0d49fca734698ca417c5e841248c5 Mon Sep 17 00:00:00 2001
From: finn0s <90374296+finn0s@users.noreply.github.com>
Date: Fri, 15 Sep 2023 09:40:45 +0200
Subject: [PATCH] Unload broken plugins fix (#545)

---
 .../java/org/pf4j/AbstractPluginManager.java  | 19 ++++++++----
 .../java/org/pf4j/JarPluginManagerTest.java   | 30 +++++++++++++++++++
 2 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
index f56a94f..a23fc6a 100644
--- a/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
+++ b/pf4j/src/main/java/org/pf4j/AbstractPluginManager.java
@@ -280,15 +280,22 @@ public abstract class AbstractPluginManager implements PluginManager {
                     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
index 0e16f75..8cbb42b 100644
--- a/pf4j/src/test/java/org/pf4j/JarPluginManagerTest.java
+++ b/pf4j/src/test/java/org/pf4j/JarPluginManagerTest.java
@@ -16,8 +16,11 @@
 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;
@@ -25,8 +28,11 @@ 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 class JarPluginManagerTest {
         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));
+        }
+    }
 }