R3944Realms/ModernFix
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove locking system for Night Config files

Browse Source 
This can cause deadlocks if mods themselves are also using their
own internal locks (Sophisticated Backpacks does this on 1.16 by
using a CHM)

This system will be replaced by a command/keybind to manually reload
configs
This commit is contained in:
embeddedt 2023-08-03 17:52:50 -04:00
parent 5853f9b034
commit 1989f122c6
No known key found for this signature in database
GPG Key ID: A69433EC199B5613
2 changed files with 26 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
forge/src/main/java/org/embeddedt/modernfix/forge/config/ConfigFixer.java
View File

@ -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<ModConfig.ModConfigEvent> {
private final Consumer<ModConfig.ModConfigEvent> actualHandler;
private final String modId;
private final Lock lock = new ReentrantLock();
LockingConfigHandler(String id, Consumer<ModConfig.ModConfigEvent> actualHandler) {
this.modId = id;
@ -43,14 +47,17 @@ public class ConfigFixer {
@Override
public void accept(ModConfig.ModConfigEvent 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();
}
}

103
forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java
View File

@ -1,29 +1,22 @@
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.common.ObfuscationReflectionHelper;
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.LinkedHashSet;
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<Runnable> 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");
@ -38,30 +31,6 @@ public class NightConfigFixer {
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<Class<?>, 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<Path, Object> {
public MonitoringMap(ConcurrentHashMap<Path, ?> oldMap) {
super(oldMap);
@ -82,27 +51,6 @@ public class NightConfigFixer {
}
}
private static final Set<Class<?>> 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 +58,18 @@ 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
*/
@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);
synchronized(configsToReload) {
int oldSize = configsToReload.size();
configsToReload.add(configTracker);
if(oldSize == 0) {
ModernFixMixinPlugin.instance.logger.info("Config file change detected on disk. The Forge feature to watch config files for changes is currently disabled due to random corruption issues.");
ModernFixMixinPlugin.instance.logger.info("This functionality will be restored in a future ModernFix update.");
}
} catch(ReflectiveOperationException e) {
e.printStackTrace();
}
}
}