fix: work-around ClassCircularityError:

#278 #234
alibaba · Dec 18, 2021 · c55acbd · c55acbd
1 parent 3db9753
commit c55acbd
Show file tree

Hide file tree

Showing 9 changed files with 85 additions and 8 deletions.
diff --git a/scripts/run-agent-test.sh b/scripts/run-agent-test.sh
@@ -24,9 +24,9 @@ logAndRun "${JAVA_CMD[@]}" -cp "$(getClasspathWithoutTtlJar)" \
 blueEcho 'Run agent check for Timer/TimerTask, default "ttl.agent.enable.timer.task"'
 logAndRun "${JAVA_CMD[@]}" -cp "$(getClasspathWithoutTtlJar)" \
     "-javaagent:$(getTtlJarPath)=ttl.agent.logger:STDOUT" \
-    com.alibaba.ttl.threadpool.agent.check.timer.TimerAgentCheck
+    com.alibaba.test.ttl.threadpool.agent.check.timer.TimerAgentCheck
 
 blueEcho 'Run agent check for Timer/TimerTask, explicit "ttl.agent.enable.timer.task"'
 logAndRun "${JAVA_CMD[@]}" -cp "$(getClasspathWithoutTtlJar)" \
     "-javaagent:$(getTtlJarPath)=ttl.agent.logger:STDOUT,ttl.agent.enable.timer.task:true" \
-    com.alibaba.ttl.threadpool.agent.check.timer.TimerAgentCheck
+    com.alibaba.test.ttl.threadpool.agent.check.timer.TimerAgentCheck
diff --git a/src/main/java/com/alibaba/ttl/threadpool/agent/TtlTransformer.java b/src/main/java/com/alibaba/ttl/threadpool/agent/TtlTransformer.java
@@ -13,6 +13,8 @@
 import java.util.List;
 import java.util.logging.Level;
 
+import static com.alibaba.ttl.threadpool.agent.internal.transformlet.impl.Utils.isClassUnderPackage;
+
 /**
  * TTL {@link ClassFileTransformer} of Java Agent
  *
@@ -51,8 +53,10 @@ public final byte[] transform(@Nullable final ClassLoader loader, @Nullable fina
             if (classFile == null) return NO_TRANSFORM;
 
             final String className = toClassName(classFile);
+            if (isClassUnderPackage(className, "com.alibaba.ttl")) return NO_TRANSFORM;
+            if (isClassUnderPackage(className, "java.lang")) return NO_TRANSFORM;
 
-            ClassInfo classInfo = new ClassInfo(className, classFileBuffer, loader);
+            final ClassInfo classInfo = new ClassInfo(className, classFileBuffer, loader);
 
             for (JavassistTransformlet transformlet : transformletList) {
                 transformlet.doTransform(classInfo);

diff --git a/src/main/java/com/alibaba/ttl/threadpool/agent/internal/logging/Logger.java b/src/main/java/com/alibaba/ttl/threadpool/agent/internal/logging/Logger.java
@@ -27,6 +27,13 @@ public static void setLoggerImplType(String type) {
         else loggerImplType = 0;
     }
 
+    /**
+     * Only for test code
+     */
+    public static void setLoggerImplTypeIfNotSetYet(String type) {
+        if (loggerImplType == -1) setLoggerImplType(type);
+    }
+
     public static Logger getLogger(Class<?> clazz) {
         if (loggerImplType == -1) throw new IllegalStateException("TTL logger implementation type is NOT set!");
 

diff --git a/.../com/alibaba/ttl/threadpool/agent/internal/transformlet/impl/TtlExecutorTransformlet.java b/.../com/alibaba/ttl/threadpool/agent/internal/transformlet/impl/TtlExecutorTransformlet.java
@@ -16,6 +16,7 @@
 import java.util.Set;
 import java.util.concurrent.Callable;
 
+import static com.alibaba.ttl.threadpool.agent.internal.transformlet.impl.Utils.isClassAtPackageJavaUtil;
 import static com.alibaba.ttl.threadpool.agent.internal.transformlet.impl.Utils.signatureOfMethod;
 
 /**
@@ -58,6 +59,11 @@ public TtlExecutorTransformlet(boolean disableInheritableForThreadPool) {
 
     @Override
     public void doTransform(@NonNull final ClassInfo classInfo) throws IOException, NotFoundException, CannotCompileException {
+        // work-around ClassCircularityError:
+        //      https://github.com/alibaba/transmittable-thread-local/issues/278
+        //      https://github.com/alibaba/transmittable-thread-local/issues/234
+        if (isClassAtPackageJavaUtil(classInfo.getClassName())) return;
+
         final CtClass clazz = classInfo.getCtClass();
         if (EXECUTOR_CLASS_NAMES.contains(classInfo.getClassName())) {
             for (CtMethod method : clazz.getDeclaredMethods()) {

diff --git a/...com/alibaba/ttl/threadpool/agent/internal/transformlet/impl/TtlTimerTaskTransformlet.java b/...com/alibaba/ttl/threadpool/agent/internal/transformlet/impl/TtlTimerTaskTransformlet.java
@@ -9,6 +9,7 @@
 import java.io.IOException;
 
 import static com.alibaba.ttl.threadpool.agent.internal.transformlet.impl.Utils.doTryFinallyForMethod;
+import static com.alibaba.ttl.threadpool.agent.internal.transformlet.impl.Utils.isClassAtPackageJavaUtil;
 
 /**
  * TTL {@link JavassistTransformlet} for {@link java.util.TimerTask}.
@@ -27,7 +28,12 @@ public class TtlTimerTaskTransformlet implements JavassistTransformlet {
 
     @Override
     public void doTransform(@NonNull final ClassInfo classInfo) throws IOException, NotFoundException, CannotCompileException {
-        if (TIMER_TASK_CLASS_NAME.equals(classInfo.getClassName())) return; // No need transform TimerTask class
+        // work-around ClassCircularityError:
+        if (isClassAtPackageJavaUtil(classInfo.getClassName())) return;
+
+        // TimerTask class is checked by above logic.
+        //
+        // if (TIMER_TASK_CLASS_NAME.equals(classInfo.getClassName())) return; // No need transform TimerTask class
 
         final CtClass clazz = classInfo.getCtClass();
 

diff --git a/src/main/java/com/alibaba/ttl/threadpool/agent/internal/transformlet/impl/Utils.java b/src/main/java/com/alibaba/ttl/threadpool/agent/internal/transformlet/impl/Utils.java
@@ -116,4 +116,25 @@ private static boolean isAutoWrapper(@Nullable Runnable ttlAttachments) {
 
         return value;
     }
+
+    @NonNull
+    public static String getPackageName(@NonNull String className) {
+        final int idx = className.lastIndexOf('.');
+        if (-1 == idx) return "";
+
+        return className.substring(0, idx);
+    }
+
+    public static boolean isClassAtPackage(@NonNull String className, @NonNull String packageName) {
+        return packageName.equals(getPackageName(className));
+    }
+
+    public static boolean isClassUnderPackage(@NonNull String className, @NonNull String packageName) {
+        String packageOfClass = getPackageName(className);
+        return packageOfClass.equals(packageName) || packageOfClass.startsWith(packageName + ".");
+    }
+
+    public static boolean isClassAtPackageJavaUtil(@NonNull String className) {
+        return isClassAtPackage(className, "java.util");
+    }
 }
diff --git a/...test/java/com/alibaba/ttl/threadpool/BeforeAndAfterExecuteMethodOfExecutorSubclassTest.kt b/...test/java/com/alibaba/ttl/threadpool/BeforeAndAfterExecuteMethodOfExecutorSubclassTest.kt
@@ -1,10 +1,15 @@
-package com.alibaba.ttl.threadpool
+@file:Suppress("PackageDirectoryMismatch")
+
+// Change the package out of com.alibaba.ttl
+// so agent will transform MyThreadPoolExecutor
+package com.alibaba.test.ttl.threadpool
 
 import com.alibaba.support.junit.conditional.ConditionalIgnoreRule
 import com.alibaba.support.junit.conditional.ConditionalIgnoreRule.ConditionalIgnore
 import com.alibaba.support.junit.conditional.IsAgentRun
 import com.alibaba.support.junit.conditional.NoAgentRun
 import com.alibaba.ttl.TtlRunnable
+import com.alibaba.ttl.threadpool.TtlExecutors
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
 import org.junit.Rule

diff --git a/src/test/java/com/alibaba/ttl/threadpool/agent/check/timer/TimerAgentCheck.kt b/src/test/java/com/alibaba/ttl/threadpool/agent/check/timer/TimerAgentCheck.kt
@@ -1,6 +1,7 @@
 @file:JvmName("TimerAgentCheck")
+@file:Suppress("PackageDirectoryMismatch")
 
-package com.alibaba.ttl.threadpool.agent.check.timer
+package com.alibaba.test.ttl.threadpool.agent.check.timer
 
 import com.alibaba.*
 import com.alibaba.ttl.testmodel.Task
@@ -39,7 +40,7 @@ fun main() {
 
     // child Inheritable
     assertChildTtlValues(tag, task.copied)
-    // child do not effect parent
+    // child do not affect parent
     assertParentTtlValues(copyTtlValues(ttlInstances))
 
     printHead("TimerAgentCheck OK!")

diff --git a/src/test/java/com/alibaba/ttl/threadpool/agent/internal/transformlet/impl/UtilsTest.java b/src/test/java/com/alibaba/ttl/threadpool/agent/internal/transformlet/impl/UtilsTest.java
@@ -1,13 +1,21 @@
 package com.alibaba.ttl.threadpool.agent.internal.transformlet.impl;
 
+import com.alibaba.ttl.threadpool.agent.internal.logging.Logger;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
 import java.util.HashMap;
 import java.util.Map;
 
-import static org.junit.Assert.fail;
+import static com.alibaba.ttl.threadpool.agent.internal.transformlet.impl.Utils.*;
+import static org.junit.Assert.*;
 
 public class UtilsTest {
+    @BeforeClass
+    public static void beforeClass() {
+        Logger.setLoggerImplTypeIfNotSetYet("stderr");
+    }
+
     @Test
     public void test_get_unboxing_boolean_fromMap() {
         Map<String, Object> map = new HashMap<String, Object>();
@@ -23,4 +31,23 @@ public void test_get_unboxing_boolean_fromMap() {
     private static boolean getUnboxingBoolean(Map<String, Object> map, String key) {
         return (Boolean) map.get(key);
     }
+
+    @Test
+    public void test_className_package() {
+        assertEquals("", getPackageName("Hello"));
+        assertEquals("com.foo", getPackageName("com.foo.Hello"));
+
+        assertTrue(isClassAtPackage("java.util.TimerTask", "java.util"));
+        assertFalse(isClassAtPackage("java.util.TimerTask", "java.utils"));
+        assertFalse(isClassAtPackage("java.util.TimerTask", "java"));
+        assertFalse(isClassAtPackage("java.util.TimerTask", "java.util.zip"));
+
+        assertTrue(isClassUnderPackage("java.util.TimerTask", "java.util"));
+        assertFalse(isClassUnderPackage("java.util.TimerTask", "java.utils"));
+        assertTrue(isClassUnderPackage("java.util.TimerTask", "java"));
+        assertFalse(isClassUnderPackage("java.util.TimerTask", "javax"));
+
+        assertTrue(isClassAtPackageJavaUtil("java.util.PriorityQueue"));
+        assertFalse(isClassAtPackageJavaUtil("java.util.zip.ZipInputStream"));
+    }
 }