From 0e864fbe6e90cd4f13076bd6a4944a16bd4a6154 Mon Sep 17 00:00:00 2001 From: Jonathing Date: Mon, 23 Dec 2024 15:09:22 -0500 Subject: [PATCH] Fix NightConfig FileWatcher causing dedicated server to hang when exiting (#10214) --- .../fml/config/ConfigFileTypeHandler.java | 77 +++++++++++++++++-- .../fml/config/ConfigTracker.java | 16 ++++ .../minecraftforge/fml/config/ModConfig.java | 2 +- .../net/minecraft/client/Minecraft.java.patch | 8 ++ .../dedicated/DedicatedServer.java.patch | 4 +- 5 files changed, 98 insertions(+), 9 deletions(-) diff --git a/fmlcore/src/main/java/net/minecraftforge/fml/config/ConfigFileTypeHandler.java b/fmlcore/src/main/java/net/minecraftforge/fml/config/ConfigFileTypeHandler.java index 64ecee8e71..ed0a59195e 100644 --- a/fmlcore/src/main/java/net/minecraftforge/fml/config/ConfigFileTypeHandler.java +++ b/fmlcore/src/main/java/net/minecraftforge/fml/config/ConfigFileTypeHandler.java @@ -14,6 +14,7 @@ import com.mojang.logging.LogUtils; import net.minecraftforge.fml.loading.FMLConfig; import net.minecraftforge.fml.loading.FMLPaths; import org.apache.commons.io.FilenameUtils; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import java.io.IOException; @@ -25,9 +26,66 @@ import static net.minecraftforge.fml.config.ConfigTracker.CONFIG; public class ConfigFileTypeHandler { private static final Logger LOGGER = LogUtils.getLogger(); - static final ConfigFileTypeHandler TOML = new ConfigFileTypeHandler(); private static final Path defaultConfigPath = FMLPaths.GAMEDIR.get().resolve(FMLConfig.getConfigValue(FMLConfig.ConfigValue.DEFAULT_CONFIG_PATH)); + private static final ConfigFileTypeHandler CLIENT = new ConfigFileTypeHandler(ModConfig.Type.CLIENT); + private static final ConfigFileTypeHandler COMMON = new ConfigFileTypeHandler(ModConfig.Type.COMMON); + private static final ConfigFileTypeHandler SERVER = new ConfigFileTypeHandler(ModConfig.Type.SERVER); + + private final @Nullable ModConfig.Type type; + private @Nullable FileWatcher watcher; + + // exists for bin compat + public ConfigFileTypeHandler() { + this(null); + } + + private ConfigFileTypeHandler(@Nullable ModConfig.Type type) { + this.type = type; + } + + /** + * Gets the handler for the given {@link ModConfig.Type}. + * + * @param type The type to get the handler for + * @return The handler + */ + static ConfigFileTypeHandler get(ModConfig.Type type) { + return switch (type) { + case CLIENT -> CLIENT; + case COMMON -> COMMON; + case SERVER -> SERVER; + }; + } + + /** + * Gets the {@link FileWatcher} for this handler, creating it if it doesn't exist and/or has been stopped. + * + * @return The watcher + * + * @apiNote This is package-private so modders can't just call {@link FileWatcher#stop()} and fuck up everything. + */ + FileWatcher getWatcher() { + if (this.watcher == null) { + LOGGER.debug(CONFIG, "Starting watcher for handler: {}", this); + this.watcher = new FileWatcher(); + } + + return this.watcher; + } + + /** + * Stops the {@link FileWatcher} for this handler, and sets it to null afterward. Use this instead of + * {@link FileWatcher#stop()}. + */ + void stopWatcher() { + if (this.watcher == null) return; + + LOGGER.debug(CONFIG, "Stopping watcher for hander: {}", this); + this.watcher.stop(); + this.watcher = null; + } + public Function reader(Path configBasePath) { return (c) -> { final Path configPath = configBasePath.resolve(c.getFileName()); @@ -37,7 +95,7 @@ public class ConfigFileTypeHandler { onFileNotFound((newfile, configFormat)-> setupConfigFile(c, newfile, configFormat)). writingMode(WritingMode.REPLACE). build(); - LOGGER.debug(CONFIG, "Built TOML config for {}", configPath.toString()); + LOGGER.debug(CONFIG, "Built TOML config for {}", configPath); try { configData.load(); @@ -46,9 +104,9 @@ public class ConfigFileTypeHandler { { throw new ConfigLoadingException(c, ex); } - LOGGER.debug(CONFIG, "Loaded TOML config file {}", configPath.toString()); - FileWatcher.defaultInstance().addWatch(configPath, new ConfigWatcher(c, configData, Thread.currentThread().getContextClassLoader())); - LOGGER.debug(CONFIG, "Watching TOML config file {} for changes", configPath.toString()); + LOGGER.debug(CONFIG, "Loaded TOML config file {}", configPath); + this.getWatcher().addWatch(configPath, new ConfigWatcher(c, configData, Thread.currentThread().getContextClassLoader())); + LOGGER.debug(CONFIG, "Watching TOML config file {} for changes", configPath); return configData; }; } @@ -56,9 +114,9 @@ public class ConfigFileTypeHandler { public void unload(Path configBasePath, ModConfig config) { Path configPath = configBasePath.resolve(config.getFileName()); try { - FileWatcher.defaultInstance().removeWatch(configBasePath.resolve(config.getFileName())); + this.getWatcher().removeWatch(configPath); } catch (RuntimeException e) { - LOGGER.error("Failed to remove config {} from tracker!", configPath.toString(), e); + LOGGER.error("Failed to remove config {} from tracker!", configPath, e); } } @@ -109,6 +167,11 @@ public class ConfigFileTypeHandler { } } + @Override + public String toString() { + return "ConfigFileTypeHandler[" + (type != null ? type : "UNKNOWN") + "]"; + } + private record ConfigWatcher( ModConfig modConfig, CommentedFileConfig commentedFileConfig, diff --git a/fmlcore/src/main/java/net/minecraftforge/fml/config/ConfigTracker.java b/fmlcore/src/main/java/net/minecraftforge/fml/config/ConfigTracker.java index f47d7da415..fd33c85d43 100644 --- a/fmlcore/src/main/java/net/minecraftforge/fml/config/ConfigTracker.java +++ b/fmlcore/src/main/java/net/minecraftforge/fml/config/ConfigTracker.java @@ -8,6 +8,9 @@ package net.minecraftforge.fml.config; import com.electronwill.nightconfig.core.CommentedConfig; import com.electronwill.nightconfig.core.file.CommentedFileConfig; import com.mojang.logging.LogUtils; +import net.minecraftforge.api.distmarker.Dist; +import net.minecraftforge.fml.DistExecutor; +import net.minecraftforge.fml.loading.FMLPaths; import org.slf4j.Logger; import org.slf4j.Marker; import org.slf4j.MarkerFactory; @@ -52,11 +55,24 @@ public class ConfigTracker { } } + // TODO: [FML] This is only called for the server (outside of forceUnload) + // rethink config implementation for eventual FML rewrite public void unloadConfigs(ModConfig.Type type, Path configBasePath) { LOGGER.debug(CONFIG, "Unloading configs type {}", type); for (ModConfig config : this.configSets.get(type)) { closeConfig(config, configBasePath); } + ConfigFileTypeHandler.get(type).stopWatcher(); + } + + // If there is a better way to do this, please tell me. Because this is FUCKED + public void forceUnload() { + // This is how ModStateProvider handles loading configs. So we're doing the same but with unloadConfigs instead + DistExecutor.unsafeRunWhenOn(Dist.CLIENT, () -> () -> this.unloadConfigs(ModConfig.Type.CLIENT, FMLPaths.CONFIGDIR.get())); + this.unloadConfigs(ModConfig.Type.COMMON, FMLPaths.CONFIGDIR.get()); + + // just in case server watcher is still alive somehow... + ConfigFileTypeHandler.get(ModConfig.Type.SERVER).stopWatcher(); } private static void openConfig(final ModConfig config, final Path configBasePath) { diff --git a/fmlcore/src/main/java/net/minecraftforge/fml/config/ModConfig.java b/fmlcore/src/main/java/net/minecraftforge/fml/config/ModConfig.java index 671866891b..7179e41120 100644 --- a/fmlcore/src/main/java/net/minecraftforge/fml/config/ModConfig.java +++ b/fmlcore/src/main/java/net/minecraftforge/fml/config/ModConfig.java @@ -30,7 +30,7 @@ public class ModConfig this.spec = spec; this.fileName = fileName; this.container = container; - this.configHandler = ConfigFileTypeHandler.TOML; + this.configHandler = ConfigFileTypeHandler.get(type); ConfigTracker.INSTANCE.trackConfig(this); } diff --git a/patches/minecraft/net/minecraft/client/Minecraft.java.patch b/patches/minecraft/net/minecraft/client/Minecraft.java.patch index 27e4dc5379..2022b56a2d 100644 --- a/patches/minecraft/net/minecraft/client/Minecraft.java.patch +++ b/patches/minecraft/net/minecraft/client/Minecraft.java.patch @@ -145,6 +145,14 @@ this.screen = p_91153_; if (this.screen != null) { this.screen.added(); +@@ -1146,6 +_,7 @@ + + FreeTypeUtil.destroy(); + Util.shutdownExecutors(); ++ net.minecraftforge.fml.config.ConfigTracker.INSTANCE.forceUnload(); + } catch (Throwable throwable) { + LOGGER.error("Shutdown failure!", throwable); + throw throwable; @@ -1215,9 +_,11 @@ this.mouseHandler.handleAccumulatedMovement(); profilerfiller.pop(); diff --git a/patches/minecraft/net/minecraft/server/dedicated/DedicatedServer.java.patch b/patches/minecraft/net/minecraft/server/dedicated/DedicatedServer.java.patch index cdc1aeba92..5bc247d9ea 100644 --- a/patches/minecraft/net/minecraft/server/dedicated/DedicatedServer.java.patch +++ b/patches/minecraft/net/minecraft/server/dedicated/DedicatedServer.java.patch @@ -45,7 +45,7 @@ } } -@@ -275,6 +_,11 @@ +@@ -275,6 +_,13 @@ if (this.queryThreadGs4 != null) { this.queryThreadGs4.stop(); } @@ -54,6 +54,8 @@ + this.dediLanPinger.interrupt(); + this.dediLanPinger = null; + } ++ ++ net.minecraftforge.fml.config.ConfigTracker.INSTANCE.forceUnload(); } @Override