diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/config/ConfigFixer.java b/forge/src/main/java/org/embeddedt/modernfix/forge/config/ConfigFixer.java index b08e75f1..96db11ec 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/config/ConfigFixer.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/config/ConfigFixer.java @@ -8,6 +8,9 @@ import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.core.ModernFixMixinPlugin; import java.util.Optional; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; public class ConfigFixer { @@ -35,6 +38,7 @@ public class ConfigFixer { private static class LockingConfigHandler implements Consumer { private final Consumer actualHandler; private final String modId; + private final Lock lock = new ReentrantLock(); LockingConfigHandler(String id, Consumer actualHandler) { this.modId = id; @@ -43,14 +47,17 @@ public class ConfigFixer { @Override public void accept(IConfigEvent modConfigEvent) { - Object cfgObj = NightConfigFixer.toWriteSyncConfig(modConfigEvent.getConfig().getConfigData()); - if(cfgObj != null) { - // don't synchronize on 'this' as it produces a deadlock when used alongside NightConfigFixer - synchronized (cfgObj) { - this.actualHandler.accept(modConfigEvent); - } - } else { - this.actualHandler.accept(modConfigEvent); + try { + if(lock.tryLock(2, TimeUnit.SECONDS)) { + try { + this.actualHandler.accept(modConfigEvent); + } finally { + lock.unlock(); + } + } else + ModernFix.LOGGER.error("Failed to post config event for {}, someone else is holding the lock", modId); + } catch(InterruptedException e) { + Thread.currentThread().interrupt(); } } diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java index b0879cdd..d26df346 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java @@ -1,29 +1,26 @@ package org.embeddedt.modernfix.forge.config; -import com.electronwill.nightconfig.core.file.CommentedFileConfig; -import com.electronwill.nightconfig.core.file.FileConfig; import com.electronwill.nightconfig.core.file.FileWatcher; import cpw.mods.modlauncher.api.LamdbaExceptionUtils; -import it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet; +import net.minecraftforge.fml.loading.FMLLoader; import net.minecraftforge.fml.util.ObfuscationReflectionHelper; +import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.core.ModernFixMixinPlugin; import org.embeddedt.modernfix.util.CommonModUtil; import java.lang.reflect.Field; import java.nio.file.Path; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; -/** - * Relatively simple patch to wait for config saving to finish, made complex by Night Config classes being package-private, - * and Forge not allowing mixins into libraries. - */ public class NightConfigFixer { + public static final LinkedHashSet configsToReload = new LinkedHashSet<>(); public static void monitorFileWatcher() { + if(!ModernFixMixinPlugin.instance.isOptionEnabled("bugfix.fix_config_crashes.NightConfigFixerMixin")) + return; CommonModUtil.runWithoutCrash(() -> { FileWatcher watcher = FileWatcher.defaultInstance(); Field field = FileWatcher.class.getDeclaredField("watchedFiles"); @@ -35,33 +32,25 @@ public class NightConfigFixer { }, "replacing Night Config watchedFiles map"); } + public static void runReloads() { + List runnablesToRun; + synchronized (configsToReload) { + runnablesToRun = new ArrayList<>(configsToReload); + configsToReload.clear(); + } + for(Runnable r : runnablesToRun) { + try { + r.run(); + } catch(RuntimeException e) { + e.printStackTrace(); + } + } + ModernFix.LOGGER.info("Processed {} config reloads", runnablesToRun.size()); + } + private static final Class WATCHED_FILE = LamdbaExceptionUtils.uncheck(() -> Class.forName("com.electronwill.nightconfig.core.file.FileWatcher$WatchedFile")); private static final Field CHANGE_HANDLER = ObfuscationReflectionHelper.findField(WATCHED_FILE, "changeHandler"); - public static final Class WRITE_SYNC_CONFIG = LamdbaExceptionUtils.uncheck(() -> Class.forName("com.electronwill.nightconfig.core.file.WriteSyncFileConfig")); - private static final Class AUTOSAVE_CONFIG = LamdbaExceptionUtils.uncheck(() -> Class.forName("com.electronwill.nightconfig.core.file.AutosaveCommentedFileConfig")); - private static final Field AUTOSAVE_FILECONFIG = ObfuscationReflectionHelper.findField(AUTOSAVE_CONFIG, "fileConfig"); - - private static final Field CURRENTLY_WRITING = ObfuscationReflectionHelper.findField(WRITE_SYNC_CONFIG, "currentlyWriting"); - - private static final Map, Field> CONFIG_WATCHER_TO_CONFIG_FIELD = Collections.synchronizedMap(new HashMap<>()); - - private static Field getCurrentlyWritingFieldOnWatcher(Object watcher) { - return CONFIG_WATCHER_TO_CONFIG_FIELD.computeIfAbsent(watcher.getClass(), clz -> { - while(clz != null && clz != Object.class) { - for(Field f : clz.getDeclaredFields()) { - if(CommentedFileConfig.class.isAssignableFrom(f.getType())) { - f.setAccessible(true); - ModernFixMixinPlugin.instance.logger.debug("Found CommentedFileConfig: field '{}' on {}", f.getName(), clz.getName()); - return f; - } - } - clz = clz.getSuperclass(); - } - return null; - }); - } - static class MonitoringMap extends ConcurrentHashMap { public MonitoringMap(ConcurrentHashMap oldMap) { super(oldMap); @@ -82,27 +71,6 @@ public class NightConfigFixer { } } - private static final Set> UNKNOWN_FILE_CONFIG_CLASSES = Collections.synchronizedSet(new ReferenceOpenHashSet<>()); - - public static Object toWriteSyncConfig(Object config) { - if(config == null) - return null; - try { - if(WRITE_SYNC_CONFIG.isAssignableFrom(config.getClass())) { - return config; - } else if(AUTOSAVE_CONFIG.isAssignableFrom(config.getClass())) { - FileConfig fc = (FileConfig)AUTOSAVE_FILECONFIG.get(config); - return toWriteSyncConfig(fc); - } else { - if (UNKNOWN_FILE_CONFIG_CLASSES.add(config.getClass())) - ModernFixMixinPlugin.instance.logger.warn("Unexpected FileConfig class: {}", config.getClass().getName()); - return null; - } - } catch(ReflectiveOperationException e) { - return null; - } - } - static class MonitoringConfigTracker implements Runnable { private final Runnable configTracker; @@ -110,47 +78,15 @@ public class NightConfigFixer { this.configTracker = r; } - private void protectFromSaving(FileConfig config, Runnable runnable) throws ReflectiveOperationException { - Object writeSyncConfig = toWriteSyncConfig(config); - if(writeSyncConfig != null) { - // keep trying to write, releasing the config lock each time in case something else needs to lock it - // for any reason - while(true) { - // acquiring synchronized block here should in theory prevent any other concurrent loads/saves, based - // off WriteSyncFileConfig implementation - synchronized (writeSyncConfig) { - if(CURRENTLY_WRITING.getBoolean(writeSyncConfig)) { - ModernFixMixinPlugin.instance.logger.fatal("Config being written during load!!!"); - try { Thread.sleep(500); } catch(InterruptedException e) { Thread.currentThread().interrupt(); } - continue; - } - // at this point, currentlyWriting is false, and we acquired synchronized lock, should be good to - // go - runnable.run(); - break; - } - } - } else { - runnable.run(); - } - } - /** - * This entrypoint runs when the file watcher has detected a change on the config file. Before passing - * this through to Forge, use reflection hacks to confirm the config system is not still writing to the file. - * If it is, spin until writing finishes. Immediately returning might result in the event never being observed. + * Add the config runnable to the list to be processed by the main thread. */ @Override public void run() { - try { - Field theField = getCurrentlyWritingFieldOnWatcher(this.configTracker); - if(theField != null) { - CommentedFileConfig cfg = (CommentedFileConfig)theField.get(this.configTracker); - // will synchronize and check saving flag - protectFromSaving(cfg, configTracker); - } - } catch(ReflectiveOperationException e) { - e.printStackTrace(); + synchronized(configsToReload) { + if(configsToReload.size() == 0) + ModernFixMixinPlugin.instance.logger.info("Please use /{} to reload any changed mod config files", FMLLoader.getDist().isDedicatedServer() ? "mfsrc" : "mfrc"); + configsToReload.add(configTracker); } } } diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java index 9b7ae6dd..b64c6c0d 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixClientForge.java @@ -6,6 +6,7 @@ import net.minecraft.client.Minecraft; import net.minecraft.client.gui.components.DebugScreenOverlay; import net.minecraftforge.client.ClientRegistry; import net.minecraftforge.client.ConfigGuiHandler; +import net.minecraftforge.client.event.ClientChatEvent; import net.minecraftforge.client.event.RecipesUpdatedEvent; import net.minecraftforge.client.event.RenderGameOverlayEvent; import net.minecraftforge.client.gui.ForgeIngameGui; @@ -21,6 +22,7 @@ import net.minecraftforge.fml.event.lifecycle.FMLClientSetupEvent; import net.minecraftforge.fml.javafmlmod.FMLJavaModLoadingContext; import net.minecraftforge.fml.util.ObfuscationReflectionHelper; import org.embeddedt.modernfix.ModernFixClient; +import org.embeddedt.modernfix.forge.config.NightConfigFixer; import org.embeddedt.modernfix.screen.ModernFixConfigScreen; import java.util.ArrayList; @@ -52,6 +54,16 @@ public class ModernFixClientForge { } } + @SubscribeEvent(priority = EventPriority.LOW) + public void onClientChat(ClientChatEvent event) { + if(event.getMessage() != null && event.getMessage().trim().equals("/mfrc")) { + NightConfigFixer.runReloads(); + event.setCanceled(true); + // add it to chat history + Minecraft.getInstance().gui.getChat().addRecentChat(event.getMessage()); + } + } + private static final List brandingList = new ArrayList<>(); @SubscribeEvent(priority = EventPriority.HIGHEST) diff --git a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java index fad3d815..f5aa6c11 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java +++ b/forge/src/main/java/org/embeddedt/modernfix/forge/init/ModernFixForge.java @@ -1,10 +1,13 @@ package org.embeddedt.modernfix.forge.init; import com.google.common.collect.ImmutableList; +import com.mojang.brigadier.builder.LiteralArgumentBuilder; +import net.minecraft.commands.CommandSourceStack; import net.minecraft.world.item.Item; import net.minecraftforge.api.distmarker.Dist; import net.minecraftforge.common.MinecraftForge; import net.minecraftforge.event.OnDatapackSyncEvent; +import net.minecraftforge.event.RegisterCommandsEvent; import net.minecraftforge.event.RegistryEvent; import net.minecraftforge.event.server.ServerStartedEvent; import net.minecraftforge.event.server.ServerStoppedEvent; @@ -22,11 +25,12 @@ import net.minecraftforge.server.ServerLifecycleHooks; import org.apache.commons.lang3.tuple.Pair; import org.embeddedt.modernfix.ModernFix; import org.embeddedt.modernfix.core.ModernFixMixinPlugin; +import org.embeddedt.modernfix.entity.EntityDataIDSyncHandler; +import org.embeddedt.modernfix.forge.ModernFixConfig; import org.embeddedt.modernfix.forge.classloading.ClassLoadHack; import org.embeddedt.modernfix.forge.classloading.ModFileScanDataDeduplicator; -import org.embeddedt.modernfix.forge.ModernFixConfig; -import org.embeddedt.modernfix.entity.EntityDataIDSyncHandler; import org.embeddedt.modernfix.forge.config.ConfigFixer; +import org.embeddedt.modernfix.forge.config.NightConfigFixer; import org.embeddedt.modernfix.forge.packet.PacketHandler; import org.embeddedt.modernfix.forge.registry.ObjectHolderClearer; @@ -35,6 +39,7 @@ import java.util.List; @Mod(ModernFix.MODID) public class ModernFixForge { private static ModernFix commonMod; + public static boolean launchDone = false; public ModernFixForge() { commonMod = new ModernFix(); @@ -51,6 +56,19 @@ public class ModernFixForge { ConfigFixer.replaceConfigHandlers(); } + @SubscribeEvent + public void onCommandRegister(RegisterCommandsEvent event) { + // Register separate commands since redirecting doesn't work without arguments + for(String name : new String[] { "mfrc", "mfsrc"}) { + event.getDispatcher().register(LiteralArgumentBuilder.literal(name) + .requires(source -> source.hasPermission(3)) + .executes(context -> { + NightConfigFixer.runReloads(); + return 1; + })); + } + } + @SubscribeEvent public void onDatapackSync(OnDatapackSyncEvent event) { if(event.getPlayer() != null) { diff --git a/forge/src/main/java/org/embeddedt/modernfix/platform/forge/ModernFixPlatformHooksImpl.java b/forge/src/main/java/org/embeddedt/modernfix/platform/forge/ModernFixPlatformHooksImpl.java index 6dde752b..5c12a514 100644 --- a/forge/src/main/java/org/embeddedt/modernfix/platform/forge/ModernFixPlatformHooksImpl.java +++ b/forge/src/main/java/org/embeddedt/modernfix/platform/forge/ModernFixPlatformHooksImpl.java @@ -27,6 +27,7 @@ import org.embeddedt.modernfix.api.constants.IntegrationConstants; import org.embeddedt.modernfix.forge.classloading.ATInjector; import org.embeddedt.modernfix.forge.classloading.FastAccessTransformerList; import org.embeddedt.modernfix.forge.config.NightConfigFixer; +import org.embeddedt.modernfix.forge.init.ModernFixForge; import org.embeddedt.modernfix.forge.packet.PacketHandler; import org.embeddedt.modernfix.platform.ModernFixPlatformHooks; import org.embeddedt.modernfix.spark.SparkLaunchProfiler; @@ -197,6 +198,7 @@ public class ModernFixPlatformHooksImpl implements ModernFixPlatformHooks { if(ModernFixMixinPlugin.instance.isOptionEnabled("feature.spark_profile_launch.OnForge")) { CommonModUtil.runWithoutCrash(() -> SparkLaunchProfiler.stop("launch"), "Failed to stop profiler"); } + ModernFixForge.launchDone = true; } public String getPlatformName() {