Fix NEI plugin loading crash under a SecurityManager (#1099)

src/main/java/codechicken/nei/NEIClientConfig.java

@@ -9,7 +9,6 @@ import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.regex.Pattern;
-import java.util.stream.Stream;
 
 import net.minecraft.client.Minecraft;
 import net.minecraft.client.gui.GuiScreen;
@@ -1025,11 +1024,11 @@ public class NEIClientConfig {
 
                 @Override
                 public void run() {
-                    final Stream<Class<?>> stream = NEIClientConfig.getBooleanSetting("loadPluginsInParallel")
-                            ? NEIClientConfig.pluginsList.parallelStream()
-                            : NEIClientConfig.pluginsList.stream();
-
-                    PluginLoader.load(stream, NEIModContainer.plugins, logger);
+                    PluginLoader.load(
+                            NEIClientConfig.pluginsList,
+                            NEIClientConfig.getBooleanSetting("loadPluginsInParallel"),
+                            NEIModContainer.plugins,
+                            logger);
 
                     KeyManager.requestOptionsReload();
                     RecipeCatalysts.loadCatalystInfo();

src/main/java/codechicken/nei/PluginLoader.java

@@ -1,7 +1,8 @@
 package codechicken.nei;
 
 import java.util.Collection;
-import java.util.stream.Stream;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.ForkJoinWorkerThread;
 
 import org.apache.logging.log4j.Logger;
 
@@ -11,28 +12,55 @@ final class PluginLoader {
 
     private PluginLoader() {}
 
-    static void load(Stream<Class<?>> plugins, Collection<IConfigureNEI> loadedPlugins, Logger logger) {
-        final ClassLoader classLoader = IConfigureNEI.class.getClassLoader();
+    static void load(Collection<Class<?>> plugins, boolean parallel, Collection<IConfigureNEI> loadedPlugins,
+            Logger logger) {
+        if (parallel && canSetContextClassLoader()) {
+            // Common pool workers use the system loader, which cannot see FML-only classes (old Log4j then throws
+            // while logging a plugin's stack frames), and under a SecurityManager they forbid changing it.
+            final ClassLoader classLoader = IConfigureNEI.class.getClassLoader();
+            final ForkJoinPool pool = new ForkJoinPool(Runtime.getRuntime().availableProcessors(), owner -> {
+                final ForkJoinWorkerThread worker = ForkJoinPool.defaultForkJoinWorkerThreadFactory.newThread(owner);
+                worker.setContextClassLoader(classLoader);
+                return worker;
+            }, null, false);
 
-        plugins.forEach(clazz -> {
-            final Thread thread = Thread.currentThread();
-            final ClassLoader previousClassLoader = thread.getContextClassLoader();
-
-            // Common-pool workers may use the system loader, which cannot see FML-only classes. Keep FML's loader
-            // through error logging too: old Log4j can otherwise throw while resolving a plugin's stack frames.
             try {
-                thread.setContextClassLoader(classLoader);
-                try {
-                    IConfigureNEI config = (IConfigureNEI) clazz.getConstructor().newInstance();
-                    config.loadConfig();
-                    loadedPlugins.add(config);
-                    logger.debug("Loaded {}", clazz.getName());
-                } catch (Throwable e) {
-                    logger.error("Failed to Load {}", clazz.getName(), e);
-                }
+                pool.submit(() -> plugins.parallelStream().forEach(clazz -> load(clazz, loadedPlugins, logger))).join();
             } finally {
-                thread.setContextClassLoader(previousClassLoader);
+                pool.shutdown();
             }
-        });
+        } else {
+            if (parallel) {
+                logger.warn("Context class loader cannot be changed, loading plugins sequentially");
+            }
+
+            plugins.forEach(clazz -> load(clazz, loadedPlugins, logger));
+        }
+    }
+
+    private static void load(Class<?> clazz, Collection<IConfigureNEI> loadedPlugins, Logger logger) {
+        try {
+            IConfigureNEI config = (IConfigureNEI) clazz.getConstructor().newInstance();
+            config.loadConfig();
+            loadedPlugins.add(config);
+            logger.debug("Loaded {}", clazz.getName());
+        } catch (Throwable e) {
+            logger.error("Failed to Load {}", clazz.getName(), e);
+        }
+    }
+
+    private static boolean canSetContextClassLoader() {
+        final SecurityManager securityManager = System.getSecurityManager();
+
+        if (securityManager == null) {
+            return true;
+        }
+
+        try {
+            securityManager.checkPermission(new RuntimePermission("setContextClassLoader"));
+            return true;
+        } catch (SecurityException e) {
+            return false;
+        }
     }
 }

src/test/java/codechicken/nei/PluginLoaderTest.java

@@ -2,12 +2,10 @@ package codechicken.nei;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertSame;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
-import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -20,20 +18,16 @@ import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.ForkJoinPool;
 import java.util.concurrent.ForkJoinWorkerThread;
 import java.util.concurrent.TimeUnit;
-import java.util.stream.Stream;
 
 import org.apache.logging.log4j.Logger;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.ValueSource;
 
 import codechicken.nei.api.IConfigureNEI;
 
 class PluginLoaderTest {
 
-    @ParameterizedTest
-    @ValueSource(booleans = { false, true })
-    void loadsPluginsAndLogsFailuresWithTheGameClassLoader(boolean parallel) throws Exception {
+    @Test
+    void loadsPluginsInParallelAndLogsFailuresWithTheGameClassLoader() throws Exception {
         ClassLoader workerClassLoader = new ClassLoader(null) {};
         Set<Thread> workers = ConcurrentHashMap.newKeySet();
         ForkJoinPool pool = new ForkJoinPool(2, owner -> {
@@ -57,8 +51,7 @@ class PluginLoaderTest {
         List<IConfigureNEI> loaded = new CopyOnWriteArrayList<>();
 
         try {
-            pool.submit(() -> PluginLoader.load(parallel ? plugins.parallelStream() : plugins.stream(), loaded, logger))
-                    .get(10, TimeUnit.SECONDS);
+            pool.submit(() -> PluginLoader.load(plugins, true, loaded, logger)).get(10, TimeUnit.SECONDS);
 
             assertEquals(16, loaded.size());
             verify(logger, times(32)).error(eq("Failed to Load {}"), anyString(), any(Throwable.class));
@@ -68,26 +61,6 @@ class PluginLoaderTest {
         }
     }
 
-    @Test
-    void restoresTheClassLoaderEvenIfErrorLoggingThrows() {
-        Thread thread = Thread.currentThread();
-        ClassLoader previousClassLoader = thread.getContextClassLoader();
-        ClassLoader workerClassLoader = new ClassLoader(null) {};
-        Logger logger = mock(Logger.class);
-        doThrow(new LinkageError("Logging failed")).when(logger)
-                .error(eq("Failed to Load {}"), anyString(), any(Throwable.class));
-
-        try {
-            thread.setContextClassLoader(workerClassLoader);
-            assertThrows(
-                    LinkageError.class,
-                    () -> PluginLoader.load(Stream.of(ConfigurationFailurePlugin.class), new ArrayList<>(), logger));
-            assertSame(workerClassLoader, thread.getContextClassLoader());
-        } finally {
-            thread.setContextClassLoader(previousClassLoader);
-        }
-    }
-
     public static class HealthyPlugin implements IConfigureNEI {
 
         public HealthyPlugin() {