From aa9da2e0a0355a02f8e75795976f6bd86ed7f2db Mon Sep 17 00:00:00 2001 From: Worive <13164341+Worive@users.noreply.github.com> Date: Sun, 25 Oct 2026 16:10:38 +0100 Subject: [PATCH] Fix NEI initialization aborting while logging plugin errors (#1093) --- .../java/codechicken/nei/NEIClientConfig.java | 12 +- .../java/codechicken/nei/PluginLoader.java | 38 +++++ .../codechicken/nei/PluginLoaderTest.java | 134 ++++++++++++++++++ 3 files changed, 173 insertions(+), 11 deletions(-) create mode 100644 src/main/java/codechicken/nei/PluginLoader.java create mode 100644 src/test/java/codechicken/nei/PluginLoaderTest.java diff --git a/src/main/java/codechicken/nei/NEIClientConfig.java b/src/main/java/codechicken/nei/NEIClientConfig.java index 5218f534..549b746a 100644 --- a/src/main/java/codechicken/nei/NEIClientConfig.java +++ b/src/main/java/codechicken/nei/NEIClientConfig.java @@ -37,7 +37,6 @@ import codechicken.lib.config.ConfigTag; import codechicken.lib.config.ConfigTagParent; import codechicken.nei.api.API; import codechicken.nei.api.GuiInfo; -import codechicken.nei.api.IConfigureNEI; import codechicken.nei.api.ItemInfo; import codechicken.nei.api.NEIInfo; import codechicken.nei.config.ConfigSet; @@ -1030,16 +1029,7 @@ public class NEIClientConfig { ? NEIClientConfig.pluginsList.parallelStream() : NEIClientConfig.pluginsList.stream(); - stream.forEach(clazz -> { - try { - IConfigureNEI config = (IConfigureNEI) clazz.getConstructor().newInstance(); - config.loadConfig(); - NEIModContainer.plugins.add(config); - logger.debug("Loaded {}", clazz.getName()); - } catch (Throwable e) { - logger.error("Failed to Load {}", clazz.getName(), e); - } - }); + PluginLoader.load(stream, NEIModContainer.plugins, logger); KeyManager.requestOptionsReload(); RecipeCatalysts.loadCatalystInfo(); diff --git a/src/main/java/codechicken/nei/PluginLoader.java b/src/main/java/codechicken/nei/PluginLoader.java new file mode 100644 index 00000000..9a498458 --- /dev/null +++ b/src/main/java/codechicken/nei/PluginLoader.java @@ -0,0 +1,38 @@ +package codechicken.nei; + +import java.util.Collection; +import java.util.stream.Stream; + +import org.apache.logging.log4j.Logger; + +import codechicken.nei.api.IConfigureNEI; + +final class PluginLoader { + + private PluginLoader() {} + + static void load(Stream> plugins, Collection loadedPlugins, Logger logger) { + final ClassLoader classLoader = IConfigureNEI.class.getClassLoader(); + + 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); + } + } finally { + thread.setContextClassLoader(previousClassLoader); + } + }); + } +} diff --git a/src/test/java/codechicken/nei/PluginLoaderTest.java b/src/test/java/codechicken/nei/PluginLoaderTest.java new file mode 100644 index 00000000..e478c87f --- /dev/null +++ b/src/test/java/codechicken/nei/PluginLoaderTest.java @@ -0,0 +1,134 @@ +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; + +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +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 { + ClassLoader workerClassLoader = new ClassLoader(null) {}; + Set workers = ConcurrentHashMap.newKeySet(); + ForkJoinPool pool = new ForkJoinPool(2, owner -> { + ForkJoinWorkerThread worker = ForkJoinPool.defaultForkJoinWorkerThreadFactory.newThread(owner); + worker.setContextClassLoader(workerClassLoader); + workers.add(worker); + return worker; + }, null, false); + Logger logger = mock(Logger.class); + doAnswer(invocation -> { + assertSame(IConfigureNEI.class.getClassLoader(), Thread.currentThread().getContextClassLoader()); + return null; + }).when(logger).error(eq("Failed to Load {}"), anyString(), any(Throwable.class)); + + List> plugins = new ArrayList<>(); + for (int i = 0; i < 16; i++) { + plugins.add(HealthyPlugin.class); + plugins.add(ConstructorFailurePlugin.class); + plugins.add(ConfigurationFailurePlugin.class); + } + List loaded = new CopyOnWriteArrayList<>(); + + try { + pool.submit(() -> PluginLoader.load(parallel ? plugins.parallelStream() : plugins.stream(), loaded, logger)) + .get(10, TimeUnit.SECONDS); + + assertEquals(16, loaded.size()); + verify(logger, times(32)).error(eq("Failed to Load {}"), anyString(), any(Throwable.class)); + workers.forEach(worker -> assertSame(workerClassLoader, worker.getContextClassLoader())); + } finally { + pool.shutdownNow(); + } + } + + @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() { + checkClassLoader(); + } + + @Override + public void loadConfig() { + checkClassLoader(); + } + + private static void checkClassLoader() { + if (Thread.currentThread().getContextClassLoader() != IConfigureNEI.class.getClassLoader()) { + throw new IllegalStateException("Plugin cannot see the game classloader"); + } + } + + @Override + public String getName() { + return "Test plugin"; + } + + @Override + public String getVersion() { + return "1"; + } + } + + public static class ConstructorFailurePlugin extends HealthyPlugin { + + public ConstructorFailurePlugin() { + throw new IllegalStateException("Constructor failed"); + } + } + + public static class ConfigurationFailurePlugin extends HealthyPlugin { + + @Override + public void loadConfig() { + super.loadConfig(); + throw new IllegalStateException("Configuration failed"); + } + } +}