Fix deadlock with the config fixers by moving all locking to one object

Author: embeddedt

forge/src/main/java/org/embeddedt/modernfix/forge/config/ConfigFixer.java

@@ -43,7 +43,14 @@ private static class LockingConfigHandler implements Consumer<ModConfig.ModConfi
 
         @Override
         public void accept(ModConfig.ModConfigEvent modConfigEvent) {
-            synchronized (this) {
+            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 {
+                ModernFix.LOGGER.warn("Unable to sync on a {} config object", modConfigEvent.getConfig().getConfigData().getClass().getName());
                 this.actualHandler.accept(modConfigEvent);
             }
         }

forge/src/main/java/org/embeddedt/modernfix/forge/config/NightConfigFixer.java

@@ -38,7 +38,7 @@ public static void monitorFileWatcher() {
     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");
 
-    private static final Class<?> WRITE_SYNC_CONFIG = LamdbaExceptionUtils.uncheck(() -> Class.forName("com.electronwill.nightconfig.core.file.WriteSyncFileConfig"));
+    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");
 
@@ -82,24 +82,42 @@ public Object computeIfAbsent(Path key, Function<? super Path, ?> mappingFunctio
         }
     }
 
+    private static final Set<Class<?>> UNKNOWN_FILE_CONFIG_CLASSES = Collections.synchronizedSet(new ReferenceOpenHashSet<>());
+
+    public static Object toWriteSyncConfig(Object config) {
+        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;
 
         MonitoringConfigTracker(Runnable r) {
             this.configTracker = r;
         }
 
-        private static final Set<Class<?>> UNKNOWN_FILE_CONFIG_CLASSES = Collections.synchronizedSet(new ReferenceOpenHashSet<>());
-
         private void protectFromSaving(FileConfig config, Runnable runnable) throws ReflectiveOperationException {
-            if(WRITE_SYNC_CONFIG.isAssignableFrom(config.getClass())) {
+            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 (config) {
-                        if(CURRENTLY_WRITING.getBoolean(config)) {
+                    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;
@@ -110,12 +128,7 @@ private void protectFromSaving(FileConfig config, Runnable runnable) throws Refl
                         break;
                     }
                 }
-            } else if(AUTOSAVE_CONFIG.isAssignableFrom(config.getClass())) {
-                FileConfig fc = (FileConfig)AUTOSAVE_FILECONFIG.get(config);
-                protectFromSaving(fc, runnable);
             } else {
-                if(UNKNOWN_FILE_CONFIG_CLASSES.add(config.getClass()))
-                    ModernFixMixinPlugin.instance.logger.warn("Unexpected FileConfig class: {}", config.getClass().getName());
                 runnable.run();
             }
         }