diff --git a/build.gradle b/build.gradle
index 81709df4d..40de891ef 100644
--- a/build.gradle
+++ b/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 {
diff --git a/src/main/java/org/spongepowered/asm/mixin/transformer/IMixinTransformer.java b/src/main/java/org/spongepowered/asm/mixin/transformer/IMixinTransformer.java
index 3098177e9..f6aaf153b 100644
--- a/src/main/java/org/spongepowered/asm/mixin/transformer/IMixinTransformer.java
+++ b/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) {
+ }
+
}
diff --git a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinTransformer.java b/src/main/java/org/spongepowered/asm/mixin/transformer/MixinTransformer.java
index 2c2df6ed1..78b6d0b88 100644
--- a/src/main/java/org/spongepowered/asm/mixin/transformer/MixinTransformer.java
+++ b/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 null 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 getSuperTypes(String className) {
+ List 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;
+ }
}
diff --git a/src/main/java/org/spongepowered/asm/mixin/transformer/TransformationLock.java b/src/main/java/org/spongepowered/asm/mixin/transformer/TransformationLock.java
new file mode 100644
index 000000000..02be45f6c
--- /dev/null
+++ b/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
+ * 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 waitTokens = new HashMap<>();
+ // Set of classes we can safely skip transformations for
+ private final Set skipTransformationFor = new HashSet<>();
+ // Used to determine the class loads that will follow while still holding the CL lock for each class
+ private final Function> getSuperTypes;
+ // Current lock holder
+ private Thread holder = null;
+ // Number of locks held (this lock is reentrant)
+ private int lockCount = 0;
+
+ public TransformationLock(Function> 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 withLock(String className, T fallback, Supplier 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 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;
+ }
+}
diff --git a/src/test/java/org/spongepowered/asm/mixin/transformer/TransformationLockTest.java b/src/test/java/org/spongepowered/asm/mixin/transformer/TransformationLockTest.java
new file mode 100644
index 000000000..361f4fbf4
--- /dev/null
+++ b/src/test/java/org/spongepowered/asm/mixin/transformer/TransformationLockTest.java
@@ -0,0 +1,126 @@
+/*
+ * This file is part of Mixin, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) SpongePowered
+ * 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 org.jetbrains.lincheck.Lincheck;
+import org.junit.jupiter.api.Test;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.function.Supplier;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class TransformationLockTest {
+ @Test
+ void testDeadlockFree() {
+ Lincheck.runConcurrentTest(() -> {
+ Scenario scenario = new Scenario();
+ Thread t1 = thread(() -> {
+ boolean transformed = scenario.withTransformationLock("Target 1", () -> {
+ scenario.simulateClassLoad("A1");
+ return true;
+ }, false);
+ assertTrue(transformed, "Target 1 not transformed");
+ });
+ Thread t2 = thread(() -> {
+ boolean transformed = scenario.withTransformationLock("Target 2", () -> {
+ scenario.simulateClassLoad("A2");
+ return true;
+ }, false);
+ assertTrue(transformed, "Target 2 not transformed");
+ });
+ Thread t3 = thread(() -> scenario.simulateClassLoad("A1"));
+ Thread t4 = thread(() -> scenario.simulateClassLoad("A2"));
+ try {
+ t1.join();
+ t2.join();
+ t3.join();
+ t4.join();
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ });
+ }
+
+ private Thread thread(Runnable runnable) {
+ Thread thread = new Thread(runnable);
+ thread.start();
+ return thread;
+ }
+
+ private static final class Scenario {
+ private final ConcurrentMap clLocks = new ConcurrentHashMap<>();
+ private final TransformationLock transformationLock = new TransformationLock(this::getSuperTypes);
+ private final Set loaded = Collections.newSetFromMap(new ConcurrentHashMap<>());
+
+ public void withClLock(String name, Runnable block) {
+ synchronized (clLocks.computeIfAbsent(name, k -> new Object())) {
+ block.run();
+ }
+ }
+
+ public T withTransformationLock(String name, Supplier block, T fallback) {
+ return transformationLock.withLock(name, fallback, block);
+ }
+
+ public void simulateClassLoad(String name) {
+ transformationLock.notifyClassLoadAttempt(name);
+ withClLock(name, () -> {
+ if (loaded.contains(name)) {
+ return;
+ }
+ withTransformationLock(name, () -> null, null);
+ loaded.add(name);
+ for (String superType : getSuperTypes(name)) {
+ simulateClassLoad(superType);
+ }
+ });
+ }
+
+ private List getSuperTypes(String className) {
+ switch (className) {
+ case "A1":
+ return Collections.singletonList("B");
+ case "A2":
+ return Arrays.asList("B", "D");
+ case "B":
+ return Collections.singletonList("C");
+ case "C":
+ return Collections.singletonList("D");
+ case "D":
+ return Collections.singletonList("java.lang.Object");
+ case "java.lang.Object":
+ return Collections.emptyList();
+ default:
+ throw new IllegalArgumentException("Unknown class: " + className);
+ }
+ }
+ }
+}
\ No newline at end of file