From b0f0f6d3902097693b3763c48375e566e6f8e5af Mon Sep 17 00:00:00 2001 From: LlamaLad7 Date: Wed, 11 Mar 2026 00:03:52 +0000 Subject: [PATCH 1/2] Build: Set up tests. --- build.gradle | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/build.gradle b/build.gradle index 81709df4d..6a1f744d9 100644 --- a/build.gradle +++ b/build.gradle @@ -236,6 +236,14 @@ 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" +} + +tasks.test { + useJUnitPlatform() + jvmArgs "-XX:+EnableDynamicAgentLoading" } javadoc { From 513f646a5abc767b30d71a777e28b167cb5942b8 Mon Sep 17 00:00:00 2001 From: LlamaLad7 Date: Wed, 11 Mar 2026 00:11:36 +0000 Subject: [PATCH 2/2] New: Custom TransformationLock with deadlock avoidance. --- build.gradle | 1 + .../mixin/transformer/IMixinTransformer.java | 8 + .../mixin/transformer/MixinTransformer.java | 85 ++++++--- .../mixin/transformer/TransformationLock.java | 170 ++++++++++++++++++ .../transformer/TransformationLockTest.java | 126 +++++++++++++ 5 files changed, 369 insertions(+), 21 deletions(-) create mode 100644 src/main/java/org/spongepowered/asm/mixin/transformer/TransformationLock.java create mode 100644 src/test/java/org/spongepowered/asm/mixin/transformer/TransformationLockTest.java diff --git a/build.gradle b/build.gradle index 6a1f744d9..40de891ef 100644 --- a/build.gradle +++ b/build.gradle @@ -239,6 +239,7 @@ dependencies { 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 { 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