From edb3da4f745fb084264d0b1e63364b8509c22378 Mon Sep 17 00:00:00 2001
From: embeddedt <42941056+embeddedt@users.noreply.github.com>
Date: Fri, 21 Jul 2023 11:13:56 -0400
Subject: [PATCH] Fix deadlock with the config fixers by moving all locking to
 one object

---
 .../modernfix/forge/config/ConfigFixer.java   |  9 ++++-
 .../forge/config/NightConfigFixer.java        | 35 +++++++++++++------
 2 files changed, 32 insertions(+), 12 deletions(-)

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 fb7e0f71..2af232b4 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
@@ -43,7 +43,14 @@ public class ConfigFixer {
 
         @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);
             }
         }
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 f48b896d..5fb11574 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
@@ -38,7 +38,7 @@ 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");
 
-    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,6 +82,25 @@ public class NightConfigFixer {
         }
     }
 
+    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;
 
@@ -89,17 +108,16 @@ public class NightConfigFixer {
             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 @@ public class NightConfigFixer {
                         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();
             }
         }