Fix/deadlock

build.gradle

@@ -236,6 +236,15 @@ dependencies {
     legacyImplementation "org.ow2.asm:asm-tree:$asmVersion"
 
     modularityCompileOnly 'org.apache.logging.log4j:log4j-core:2.11.2'
+
+    testImplementation "org.junit.jupiter:junit-jupiter:5.7.1"
+    testRuntimeOnly "org.junit.platform:junit-platform-launcher"
+    testImplementation "org.jetbrains.lincheck:lincheck:3.4"
+}
+
+tasks.test {
+    useJUnitPlatform()
+    jvmArgs "-XX:+EnableDynamicAgentLoading"
 }
 
 javadoc {

src/main/java/org/spongepowered/asm/mixin/transformer/IMixinTransformer.java

@@ -133,4 +133,12 @@ public interface IMixinTransformer {
      */
     public abstract IExtensionRegistry getExtensions();
 
+    /**
+     * Notify Mixin that a class load was requested for a given class. Used for deadlock avoidance.
+     * Must be called *before* acquiring the class's class loading lock.
+     * @param name Class being loaded (. separated)
+     */
+    public default void notifyClassLoadAttempt(String name) {
+    }
+
 }

src/main/java/org/spongepowered/asm/mixin/transformer/MixinTransformer.java

@@ -24,9 +24,14 @@
  */
 package org.spongepowered.asm.mixin.transformer;
 
+import java.io.IOException;
 import java.lang.reflect.Constructor;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 
+import org.objectweb.asm.ClassReader;
 import org.objectweb.asm.tree.ClassNode;
 import org.spongepowered.asm.launch.MixinInitialisationError;
 import org.spongepowered.asm.mixin.MixinEnvironment;
@@ -35,6 +40,7 @@
 import org.spongepowered.asm.mixin.transformer.ext.Extensions;
 import org.spongepowered.asm.mixin.transformer.ext.IExtensionRegistry;
 import org.spongepowered.asm.mixin.transformer.ext.IHotSwap;
+import org.spongepowered.asm.service.MixinService;
 import org.spongepowered.asm.transformers.TreeTransformer;
 import org.spongepowered.asm.util.Constants;
 import org.spongepowered.asm.util.asm.ASM;
@@ -92,6 +98,8 @@ public IMixinTransformer createTransformer() throws MixinInitialisationError {
      * Class generator 
      */
     private final MixinClassGenerator generator;
+
+    private final TransformationLock transformationLock = new TransformationLock(MixinTransformer::getSuperTypes);
 
     MixinTransformer() {
         MixinEnvironment environment = MixinEnvironment.getCurrentEnvironment();
@@ -229,15 +237,17 @@ public boolean computeFramesForClass(MixinEnvironment environment, String name,
      * @return Transformed bytecode
      */
     @Override
-    public synchronized byte[] transformClass(MixinEnvironment environment, String name, byte[] classBytes) {
-        if (!couldTransformClass(environment, name)) {
+    public byte[] transformClass(MixinEnvironment environment, String name, byte[] classBytes) {
+        return this.transformationLock.withLock(name, classBytes, () -> {
+            if (!couldTransformClass(environment, name)) {
+                return classBytes;
+            }
+            ClassNode classNode = this.readClass(name, classBytes);
+            if (this.processor.applyMixins(environment, name, classNode)) {
+                return this.writeClass(classNode);
+            }
             return classBytes;
-        }
-        ClassNode classNode = this.readClass(name, classBytes);
-        if (this.processor.applyMixins(environment, name, classNode)) {
-            return this.writeClass(classNode);
-        }
-        return classBytes;
+        });
     }
 
     /**
@@ -249,8 +259,8 @@ public synchronized byte[] transformClass(MixinEnvironment environment, String n
      * @return true if the class was transformed
      */
     @Override
-    public synchronized boolean transformClass(MixinEnvironment environment, String name, ClassNode classNode) {
-        return this.processor.applyMixins(environment, name, classNode);
+    public boolean transformClass(MixinEnvironment environment, String name, ClassNode classNode) {
+        return this.transformationLock.withLock(name, false, () -> this.processor.applyMixins(environment, name, classNode));
     }
 
     /**
@@ -261,8 +271,8 @@ public synchronized boolean transformClass(MixinEnvironment environment, String
      * @return true if the class could be transformed
      */
     @Override
-    public synchronized boolean couldTransformClass(MixinEnvironment environment, String name) {
-        return this.processor.couldTransformClass(environment, name);
+    public boolean couldTransformClass(MixinEnvironment environment, String name) {
+        return this.transformationLock.withLock(name, false, () -> this.processor.couldTransformClass(environment, name));
     }
 
     /**
@@ -273,12 +283,14 @@ public synchronized boolean couldTransformClass(MixinEnvironment environment, St
      * @return Generated bytecode or <tt>null</tt> if no class was generated
      */
     @Override
-    public synchronized byte[] generateClass(MixinEnvironment environment, String name) {
-        ClassNode classNode = MixinTransformer.createEmptyClass(name);
-        if (this.generator.generateClass(environment, name, classNode)) {
-            return this.writeClass(classNode);
-        }
-        return null;
+    public byte[] generateClass(MixinEnvironment environment, String name) {
+        return this.transformationLock.withLock(name, null, () -> {
+            ClassNode classNode = MixinTransformer.createEmptyClass(name);
+            if (this.generator.generateClass(environment, name, classNode)) {
+                return this.writeClass(classNode);
+            }
+            return null;
+        });
     }
 
     /**
@@ -288,10 +300,15 @@ public synchronized byte[] generateClass(MixinEnvironment environment, String na
      * @return True if the class was generated successfully
      */
     @Override
-    public synchronized boolean generateClass(MixinEnvironment environment, String name, ClassNode classNode) {
-        return this.generator.generateClass(environment, name, classNode);
+    public boolean generateClass(MixinEnvironment environment, String name, ClassNode classNode) {
+        return this.transformationLock.withLock(name, false, () -> this.generator.generateClass(environment, name, classNode));
     }
-
+
+    @Override
+    public void notifyClassLoadAttempt(String name) {
+        this.transformationLock.notifyClassLoadAttempt(name);
+    }
+
     /**
      * You need to ask yourself why you're reading this comment  
      */
@@ -302,5 +319,31 @@ private static ClassNode createEmptyClass(String name) {
         classNode.superName = Constants.OBJECT;
         return classNode;
     }
+
+    private static List<String> getSuperTypes(String className) {
+        List<String> result = new ArrayList<>();
+        ClassInfo info = ClassInfo.fromCache(className);
+        if (info != null) {
+            // Fast path
+            result.add(info.getSuperName());
+            result.addAll(info.getInterfaces());
+        } else {
+            ClassNode node;
+            try {
+                node = MixinService.getService().getBytecodeProvider().getClassNode(
+                        className, true, 
+                        ClassReader.SKIP_CODE | ClassReader.SKIP_DEBUG | ClassReader.SKIP_FRAMES
+                );
+            } catch (ClassNotFoundException | IOException e) {
+                // Hopefully the class just doesn't exist
+                return Collections.emptyList();
+            }
+            result.add(node.superName);
+            result.addAll(node.interfaces);
+        }
+        result.removeIf(Objects::isNull);
+        result.replaceAll(it -> it.replace('/', '.'));
+        return result;
+    }
 
 }

src/main/java/org/spongepowered/asm/mixin/transformer/TransformationLock.java

@@ -0,0 +1,170 @@
+/*
+ * This file is part of Mixin, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) SpongePowered <https://www.spongepowered.org>
+ * Copyright (c) contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.spongepowered.asm.mixin.transformer;
+
+import java.util.*;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.function.Function;
+import java.util.function.Supplier;
+
+/**
+ * Custom lock to avoid deadlocks when re-entrantly loaded classes are also loaded on other threads.
+ * Stops waiting for the transformation lock if we realise the class we want to transform has been loaded re=entrantly
+ * on another thread.
+ */
+class TransformationLock {
+    // Bookkeeping lock, never held during external operations
+    private final ReentrantLock lock = new ReentrantLock();
+    // Populated when a thread holds the CL lock for a given class but has to wait for the transformation lock
+    private final Map<String, Condition> waitTokens = new HashMap<>();
+    // Set of classes we can safely skip transformations for
+    private final Set<String> skipTransformationFor = new HashSet<>();
+    // Used to determine the class loads that will follow while still holding the CL lock for each class
+    private final Function<String, List<String>> getSuperTypes;
+    // Current lock holder
+    private Thread holder = null;
+    // Number of locks held (this lock is reentrant)
+    private int lockCount = 0;
+
+    public TransformationLock(Function<String, List<String>> getSuperTypes) {
+        this.getSuperTypes = getSuperTypes;
+    }
+
+    /**
+     * Performs the given operation while holding the transformation lock.
+     * If the transformation is cancelled because the target is loaded re-entrantly, returns the fallback.
+     */
+    public <T> T withLock(String className, T fallback, Supplier<T> operation) {
+        boolean success = acquireTransformationLock(className);
+        if (!success) {
+            // Cancelled, no need to do any transformation
+            return fallback;
+        }
+        try {
+            return operation.get();
+        } finally {
+            releaseTransformationLock();
+        }
+    }
+
+    /**
+     * Notifies an attempt to load a class (must be called *before* acquiring that class's CL lock).
+     */
+    public void notifyClassLoadAttempt(String className) {
+        lock.lock();
+        try {
+            if (holder == Thread.currentThread()) {
+                // Re-entrant load, therefore the given class and its supertypes cannot have any mixins applied to them
+                skipTransformationRecursively(className);
+            }
+        } finally {
+            lock.unlock();
+        }
+    }
+
+    private boolean acquireTransformationLock(String className) {
+        lock.lock();
+        try {
+            if (holder == null || holder == Thread.currentThread()) {
+                // Uncontended fast path
+                holder = Thread.currentThread();
+                lockCount++;
+                return true;
+            }
+            Condition waitToken = lock.newCondition();
+            waitTokens.put(className, waitToken);
+            try {
+                while (true) {
+                    if (skipTransformationFor.contains(className)) {
+                        // Skip
+                        return false;
+                    }
+                    try {
+                        waitToken.await();
+                    } catch (InterruptedException e) {
+                        Thread.currentThread().interrupt();
+                        throw new RuntimeException("Interrupted while waiting for transformation lock", e);
+                    }
+                    if (holder == null) {
+                        // We can acquire the lock
+                        holder = Thread.currentThread();
+                        lockCount++;
+                        return true;
+                    }
+                    // Got raced, try again
+                }
+            } finally {
+                // Don't need this anymore
+                waitTokens.remove(className);
+            }
+        } finally {
+            lock.unlock();
+        }
+    }
+
+    private void releaseTransformationLock() {
+        lock.lock();
+        try {
+            if (--lockCount == 0) {
+                holder = null;
+                wakeUpWaiter();
+            }
+        } finally {
+            lock.unlock();
+        }
+    }
+
+    private void wakeUpWaiter() {
+        // Only need to wake up 1 waiter when releasing the lock, because the waiter we wake up will always finish
+        // its transformation and then wake up 1 more waiter, etc
+        for (Condition waitToken : waitTokens.values()) {
+            waitToken.signal();
+            return;
+        }
+    }
+
+    private void skipTransformationRecursively(String className) {
+        // If we've seen this before no need to calculate the supertypes again
+        if (skipTransformation(className)) {
+            List<String> superTypes = getSuperTypes.apply(className);
+            if (superTypes != null) {
+                superTypes.forEach(this::skipTransformationRecursively);
+            }
+        }
+    }
+
+    private boolean skipTransformation(String className) {
+        boolean isNew = skipTransformationFor.add(className);
+        if (isNew) {
+            // Cancel any thread waiting to apply mixins
+            Condition waitToken = waitTokens.get(className);
+            if (waitToken != null) {
+                waitToken.signal();
+            }
+        }
+        return isNew;
+    }
+}