Forge Config API Port

Forge Config API Port

29M Downloads

[Bug]: Race condition with unloading server config

SquidDev opened this issue ยท 4 comments

commented

Mod Loader (Required)

Fabric

Minecraft Version (Required)

1.19.3

Mod Version (Required)

4.2.6

Notes (Required)

When unloading a world, it's possible for the config file watcher to observe a file update at the same time as the config is cleared. This can cause a race condition where ForgeConfigSpec.isLoaded() returns true at the start of the method, but disappears out under you while reading config values!

I'm afraid I've not spent much time debugging this: my guess is that the config object being saved before unloading is triggering the file watcher. However, I've not observed the behaviour on Forge, so it's very odd!

Reproduction case

Consider the following class body, where onReload is subscribed to a mod's config reload bus.

public static final ForgeConfigSpec configSpec;
private static final ConfigValue<Integer> someConfigOption;

public static void onReload(ModConfig config) {
    LOG.info("Syncing config {}/{} with loaded={}", config.getModId(), config.getType(), spec.isLoaded());
    try {
        if (!configSpec.isLoaded()) return;

        for(int i = 0; i < 1000; i++) someConfigOption.get();
    } finally {
        LOG.info("Done syncing config loaded={}", serverSpec.isLoaded());
    }
}

When exiting a world, one may (it's not deterministic, maybe 1 in 5 times) see the following output:

[18:10:22] [Thread-1/INFO] (ConfigSpec) Syncing config computercraft/SERVER with loaded=true
[18:10:22] [Thread-1/INFO] (ConfigSpec) Done syncing loaded=false
[18:10:22] [Thread-1/INFO] (Minecraft) [STDERR]: java.lang.IllegalStateException: Cannot get config value before config is loaded.
This error is currently only thrown in the development environment, to avoid breaking published mods.
In a future version, this will also throw in the production environment.

[18:10:22] [Thread-1/INFO] (Minecraft) [STDERR]: 	at com.google.common.base.Preconditions.checkState(Preconditions.java:502)
[18:10:22] [Thread-1/INFO] (Minecraft) [STDERR]: 	at net.minecraftforge.common.ForgeConfigSpec$ConfigValue.get(ForgeConfigSpec.java:952)
[18:10:22] [Thread-1/INFO] (Minecraft) [STDERR]: 	at dan200.computercraft.shared.config.ConfigSpec.sync(ConfigSpec.java:420)
[18:10:22] [Thread-1/INFO] (Minecraft) [STDERR]: 	at net.minecraftforge.api.fml.event.config.ModConfigEvents$ModConfigEventsHolder.lambda$create$2(ModConfigEvents.java:113)
[18:10:22] [Thread-1/INFO] (Minecraft) [STDERR]: 	at net.minecraftforge.fml.config.ConfigFileTypeHandler$ConfigWatcher.run(ConfigFileTypeHandler.java:159)
[18:10:22] [Thread-1/INFO] (Minecraft) [STDERR]: 	at com.electronwill.nightconfig.core.file.FileWatcher$WatcherThread.run(FileWatcher.java:181)
[18:10:23] [Render thread/INFO] (ConfigSpec) Syncing config computercraft/SERVER with loaded=false
[18:10:23] [Render thread/INFO] (ConfigSpec) Done syncing config loaded=false

You can see that the config is reloaded on two separate threads, in such a way that ForgeConfigSpec.isLoaded changes from true to false during our method execution!

latest.log (Optional)

No response

commented

I did that in the latest version, hope all works fine now.

commented

Does this open Forge PR look like it would fix your issue?
MinecraftForge/MinecraftForge#9016

commented

That definitely looks like the same bug, yep! I think applying the patch from ConfigTracker would be "good enough" (so swapping save and unload), at least for my needs.

commented

Sorry, only just got round to updating. This appears to work perfectly, thank you!