#1322 Multiple transforms support

bootstrap-core/src/main/java/com/navercorp/pinpoint/bootstrap/instrument/DynamicTransformRequestListener.java

@@ -23,5 +23,8 @@
  */
 public interface DynamicTransformRequestListener {
     void onRetransformRequest(Class<?> target, ClassFileTransformer transformer);
+
+    ClassFileTransformer onRetransformFail(Class<?> target);
+
     void onTransformRequest(ClassLoader classLoader, String targetClassName, ClassFileTransformer transformer);
 }

profiler/src/main/java/com/navercorp/pinpoint/profiler/ClassFileTransformerDispatcher.java

@@ -133,6 +133,11 @@ public void onRetransformRequest(Class<?> target, final ClassFileTransformer tra
         this.dynamicTransformerRegistry.onRetransformRequest(target, transformer);
     }
 
+    @Override
+    public ClassFileTransformer onRetransformFail(Class<?> target) {
+        return this.dynamicTransformerRegistry.onRetransformFail(target);
+    }
+
     @Override
     public void onTransformRequest(ClassLoader classLoader, String targetClassName, ClassFileTransformer transformer) {
         this.dynamicTransformerRegistry.onTransformRequest(classLoader, targetClassName, transformer);

profiler/src/main/java/com/navercorp/pinpoint/profiler/DefaultDynamicTransformerRegistry.java

@@ -33,37 +33,55 @@ public class DefaultDynamicTransformerRegistry implements DynamicTransformerRegi
 
     @Override
     public void onRetransformRequest(Class<?> target, final ClassFileTransformer transformer) {
-        add(target.getClassLoader(), target.getName(), transformer);
+        final TransformerKey key = createTransformKey(target);
+        add(key, transformer);
         if (logger.isInfoEnabled()) {
             logger.info("added retransformer classLoader: {}, class: {}, registry size: {}", target.getClassLoader(), target.getName(), transformerMap.size());
         }
     }
-
+
+    @Override
+    public ClassFileTransformer onRetransformFail(Class<?> target) {
+        final TransformerKey key = createTransformKey(target);
+        return transformerMap.remove(key);
+    }
+
     @Override
     public void onTransformRequest(ClassLoader classLoader, String targetClassName, ClassFileTransformer transformer) {
         // TODO fix classLoader null case
 //        if (classLoader== null) {
 //            boot? ext? system?
 //            classLoader = ClassLoader.getSystemClassLoader();
 //        }
-        add(classLoader, targetClassName, transformer);
+        final TransformerKey transformKey = createTransformKey(classLoader, targetClassName);
+        add(transformKey, transformer);
 
         if (logger.isInfoEnabled()) {
             logger.info("added dynamic transformer classLoader: {}, className: {}, registry size: {}", classLoader, targetClassName, transformerMap.size());
         }
     }
 
-    private void add(ClassLoader classLoader, String targetClassName, ClassFileTransformer transformer) {
-        final String jvmName = JavaAssistUtils.javaNameToJvmName(targetClassName);
-
-        final TransformerKey key = new TransformerKey(classLoader, jvmName);
+    private void add(TransformerKey key, ClassFileTransformer transformer) {
         final ClassFileTransformer prev = transformerMap.putIfAbsent(key, transformer);
 
         if (prev != null) {
-            throw new ProfilerException("Transformer already exists. classLoader: " + classLoader + ", target: " + targetClassName + ", transformer: " + prev);
+            throw new ProfilerException("Transformer already exists. TransformKey: " + key + ", transformer: " + prev);
         }
     }
-
+
+    private TransformerKey createTransformKey(ClassLoader classLoader, String targetClassName) {
+        final String jvmName = JavaAssistUtils.javaNameToJvmName(targetClassName);
+        return new TransformerKey(classLoader, jvmName);
+    }
+
+    private TransformerKey createTransformKey(Class<?> targetClass) {
+
+        final ClassLoader classLoader = targetClass.getClassLoader();
+        final String targetClassName = targetClass.getName();
+
+        return createTransformKey(classLoader, targetClassName);
+    }
+
     @Override
     public ClassFileTransformer getTransformer(ClassLoader classLoader, String targetClassName) {
         // TODO fix classLoader null case
@@ -80,6 +98,10 @@ public ClassFileTransformer getTransformer(ClassLoader classLoader, String targe
 
         return transformer;
     }
+
+    int size() {
+        return transformerMap.size();
+    }
 
     private static final class TransformerKey {
         // TODO depends classLoader memory leak
@@ -112,5 +134,13 @@ public int hashCode() {
             result = 31 * result + targetClassName.hashCode();
             return result;
         }
+
+        @Override
+        public String toString() {
+            return "TransformerKey{" +
+                    "classLoader=" + classLoader +
+                    ", targetClassName='" + targetClassName + '\'' +
+                    '}';
+        }
     }
 }

profiler/src/main/java/com/navercorp/pinpoint/profiler/DynamicTransformService.java

@@ -54,8 +54,15 @@ public void retransform(Class<?> target, ClassFileTransformer transformer) {
         assertClass(target);
 
         this.dynamicTransformRequestListener.onRetransformRequest(target, transformer);
-
-        triggerRetransform(target);
+        boolean success = false;
+        try {
+            triggerRetransform(target);
+            success = true;
+        } finally {
+            if (!success) {
+                this.dynamicTransformRequestListener.onRetransformFail(target);
+            }
+        }
     }
 
     @Override

profiler/src/main/java/com/navercorp/pinpoint/profiler/instrument/JavassistClassPool.java

@@ -20,10 +20,8 @@
 import java.net.URLClassLoader;
 
 import com.navercorp.pinpoint.bootstrap.instrument.*;
-import javassist.ClassClassPath;
-import javassist.ClassPool;
-import javassist.CtClass;
-import javassist.NotFoundException;
+import com.navercorp.pinpoint.profiler.util.JavaAssistUtils;
+import javassist.*;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -96,25 +94,59 @@ public void handleClassPool(NamedClassPool systemClassPool) {
     }
 
     public InstrumentClass getClass(ClassLoader classLoader, String jvmInternalClassName, byte[] classFileBuffer) throws NotFoundInstrumentException {
-        CtClass cc = getClass(classLoader, jvmInternalClassName);
+        final CtClass cc = getCtClass(classLoader, jvmInternalClassName, classFileBuffer);
         return new JavassistClass(null, interceptorRegistryBinder, classLoader, cc);
     }
-    
+
     @Override
     public InstrumentClass getClass(InstrumentContext instrumentContext, ClassLoader classLoader, String jvmInternalClassName, byte[] classFileBuffer) throws NotFoundInstrumentException {
-        CtClass cc = getClass(classLoader, jvmInternalClassName);
+        if (jvmInternalClassName == null) {
+            throw new NullPointerException("jvmInternalClassName must not be null");
+        }
+        final CtClass cc = getCtClass(classLoader, jvmInternalClassName, classFileBuffer);
         return new JavassistClass(instrumentContext, interceptorRegistryBinder, classLoader, cc);
     }
-    
-    public CtClass getClass(ClassLoader classLoader, String className) throws NotFoundInstrumentException {
+
+    private CtClass getCtClass(ClassLoader classLoader, String className, byte[] classfileBuffer) throws NotFoundInstrumentException {
         final NamedClassPool classPool = getClassPool(classLoader);
         try {
-            return classPool.get(className);
+            if (classfileBuffer == null) {
+                // compatibility code
+                logger.info("classFileBuffer is null className:{}", className);
+                return classPool.get(className);
+            } else {
+                final ClassPool contextCassPool = getContextClassPool(classPool, className, classfileBuffer);
+                return contextCassPool.get(className);
+            }
         } catch (NotFoundException e) {
             throw new NotFoundInstrumentException(className + " class not found. Cause:" + e.getMessage(), e);
         }
     }
 
+    private ClassPool getContextClassPool(NamedClassPool parent, String jvmInternalClassName, byte[] classfileBuffer) {
+        final ClassPool contextCassPool = new ClassPool(parent);
+        contextCassPool.childFirstLookup = true;
+
+        final String javaName = JavaAssistUtils.jvmNameToJavaName(jvmInternalClassName);
+        if (isDebug) {
+            logger.debug("getContextClassPool() className={}", javaName);
+        }
+        final ClassPath byteArrayClassPath = new ByteArrayClassPath(javaName, classfileBuffer);
+        contextCassPool.insertClassPath(byteArrayClassPath);
+        return contextCassPool;
+    }
+
+
+    public CtClass getClass(ClassLoader classLoader, String jvmInternalClassName) throws NotFoundInstrumentException {
+        final NamedClassPool classPool = getClassPool(classLoader);
+        try {
+            return classPool.get(jvmInternalClassName);
+        } catch (NotFoundException e) {
+            throw new NotFoundInstrumentException(jvmInternalClassName + " class not found. Cause:" + e.getMessage(), e);
+        }
+    }
+
+
     public NamedClassPool getClassPool(ClassLoader classLoader) {
         return childClassPool.getClassPool(classLoader);
     }

profiler/src/main/java/com/navercorp/pinpoint/test/PluginTestAgent.java

@@ -684,7 +684,7 @@ private int findApiId(Member method) throws AssertionError {
 
         InstrumentClass ic;
         try {
-            ic = getClassPool().getClass(clazz.getClassLoader(), clazz.getName(), null);
+            ic = getClassPool().getClass(null, clazz.getClassLoader(), clazz.getName(), null);
         } catch (InstrumentException e) {
             throw new RuntimeException("Cannot get instrumentClass " + clazz.getName(), e);
         }

profiler/src/test/java/com/navercorp/pinpoint/profiler/DynamicTransformServiceTest.java

@@ -0,0 +1,66 @@
+/*
+ * *
+ *  * Copyright 2014 NAVER Corp.
+ *  * Licensed under the Apache License, Version 2.0 (the "License");
+ *  * you may not use this file except in compliance with the License.
+ *  * You may obtain a copy of the License at
+ *  *
+ *  *     http://www.apache.org/licenses/LICENSE-2.0
+ *  *
+ *  * Unless required by applicable law or agreed to in writing, software
+ *  * distributed under the License is distributed on an "AS IS" BASIS,
+ *  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  * See the License for the specific language governing permissions and
+ *  * limitations under the License.
+ *
+ */
+
+package com.navercorp.pinpoint.profiler;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+import java.lang.instrument.ClassFileTransformer;
+import java.lang.instrument.Instrumentation;
+import java.lang.instrument.UnmodifiableClassException;
+
+import static org.mockito.Mockito.*;
+
+/**
+ * @author Woonduk Kang(emeroad)
+ */
+public class DynamicTransformServiceTest {
+
+    @Test()
+    public void testRetransform_Fail_memoryleak_prevent() throws Exception {
+        final Instrumentation instrumentation = mock(Instrumentation.class);
+        when(instrumentation.isModifiableClass(any(Class.class))).thenReturn(true);
+        doAnswer(new Answer<Void>() {
+            @Override
+            public Void answer(InvocationOnMock invocation) throws Throwable {
+                throw new UnmodifiableClassException();
+            }
+        }).when(instrumentation).retransformClasses(any(Class.class));
+
+
+        DefaultDynamicTransformerRegistry listener = new DefaultDynamicTransformerRegistry();
+
+        final ClassFileTransformer classFileTransformer = mock(ClassFileTransformer.class);
+
+        DynamicTransformService dynamicTransformService = new DynamicTransformService(instrumentation, listener);
+
+        try {
+            dynamicTransformService.retransform(String.class, classFileTransformer);
+            Assert.fail("expected retransform fail");
+        } catch (Exception e) {
+        }
+        Assert.assertEquals(listener.size(), 0);
+    }
+
+//    @Test
+//    public void testAddClassFileTransformer() throws Exception {
+//
+//    }
+}

profiler/src/test/java/com/navercorp/pinpoint/profiler/instrument/JavassistClassPoolTest.java

@@ -0,0 +1,87 @@
+/*
+ * *
+ *  * Copyright 2014 NAVER Corp.
+ *  * Licensed under the Apache License, Version 2.0 (the "License");
+ *  * you may not use this file except in compliance with the License.
+ *  * You may obtain a copy of the License at
+ *  *
+ *  *     http://www.apache.org/licenses/LICENSE-2.0
+ *  *
+ *  * Unless required by applicable law or agreed to in writing, software
+ *  * distributed under the License is distributed on an "AS IS" BASIS,
+ *  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  * See the License for the specific language governing permissions and
+ *  * limitations under the License.
+ *
+ */
+
+package com.navercorp.pinpoint.profiler.instrument;
+
+import com.navercorp.pinpoint.bootstrap.instrument.InstrumentClass;
+import com.navercorp.pinpoint.profiler.interceptor.registry.InterceptorRegistryBinder;
+import com.navercorp.pinpoint.test.TestInterceptorRegistryBinder;
+import com.navercorp.pinpoint.test.util.BytecodeUtils;
+import javassist.ClassPool;
+import javassist.CtClass;
+import javassist.CtMethod;
+import javassist.bytecode.ConstPool;
+import javassist.bytecode.MethodInfo;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+/**
+ * @author Woonduk Kang(emeroad)
+ */
+public class JavassistClassPoolTest {
+
+    private final Logger logger = LoggerFactory.getLogger(this.getClass());
+
+    private final String mock = "com.navercorp.pinpoint.profiler.instrument.JavassistClassPoolTestMock";
+
+    @Test
+    public void testGetClass_original() throws Exception {
+        InterceptorRegistryBinder binder = new TestInterceptorRegistryBinder();
+        JavassistClassPool pool = new JavassistClassPool(binder, null);
+
+
+        final byte[] originalByteCode = BytecodeUtils.getClassFile(null, mock);
+        final InstrumentClass transformClass = pool.getClass(null, mock, originalByteCode);
+
+        Assert.assertNotNull(transformClass.getDeclaredMethod("test"));
+        Assert.assertNull("transform method", transformClass.getDeclaredMethod("transformMethod"));
+
+    }
+
+    @Test
+    public void testGetClass_transform() throws Exception {
+        InterceptorRegistryBinder binder = new TestInterceptorRegistryBinder();
+        JavassistClassPool pool = new JavassistClassPool(binder, null);
+
+
+        final byte[] transformByteCode = getTransformByteCode();
+        final InstrumentClass transformClass = pool.getClass(null, mock, transformByteCode);
+
+        Assert.assertNotNull(transformClass.getDeclaredMethod("test"));
+        Assert.assertNotNull("transform method", transformClass.getDeclaredMethod("transformMethod"));
+    }
+
+    public byte[] getTransformByteCode()  {
+        try {
+            final ClassPool pool = new ClassPool(true);
+
+            final CtClass ctClass = pool.get(mock);
+
+            final ConstPool constPool = ctClass.getClassFile2().getConstPool();
+
+            MethodInfo info = new MethodInfo(constPool, "transformMethod", "()V");
+            final CtMethod newMethod = CtMethod.make(info, ctClass);
+            ctClass.addMethod(newMethod);
+            return ctClass.toBytecode();
+        } catch (Exception ex) {
+            throw new RuntimeException(ex.getMessage(), ex);
+        }
+    }
+
+}
+

profiler/src/test/java/com/navercorp/pinpoint/profiler/instrument/JavassistClassPoolTestMock.java

@@ -0,0 +1,26 @@
+/*
+ * *
+ *  * Copyright 2014 NAVER Corp.
+ *  * Licensed under the Apache License, Version 2.0 (the "License");
+ *  * you may not use this file except in compliance with the License.
+ *  * You may obtain a copy of the License at
+ *  *
+ *  *     http://www.apache.org/licenses/LICENSE-2.0
+ *  *
+ *  * Unless required by applicable law or agreed to in writing, software
+ *  * distributed under the License is distributed on an "AS IS" BASIS,
+ *  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  * See the License for the specific language governing permissions and
+ *  * limitations under the License.
+ *
+ */
+
+package com.navercorp.pinpoint.profiler.instrument;
+
+/**
+ * @author Woonduk Kang(emeroad)
+ */
+public class JavassistClassPoolTestMock {
+    public void test() {
+    }
+}