Refactored listener semantics to also trigger ignore listeners for no…

…n-transformable or non-matched types upon retransformation / redefinition.
raphw · Oct 27, 2015 · 97bd570 · 97bd570
1 parent d334ba5
commit 97bd570
Show file tree

Hide file tree

Showing 2 changed files with 216 additions and 18 deletions.
diff --git a/byte-buddy-dep/src/main/java/net/bytebuddy/agent/builder/AgentBuilder.java b/byte-buddy-dep/src/main/java/net/bytebuddy/agent/builder/AgentBuilder.java
@@ -1062,8 +1062,17 @@ protected Collector makeCollector(Default.Transformation transformation) {
         },
 
         /**
+         * <p>
          * Applies a <b>redefinition</b> to all classes that are already loaded and that would have been transformed if
          * the built agent was registered before they were loaded.
+         * </p>
+         * <p>
+         * <b>Important</b>: If a redefined class was previously instrumented, this instrumentation information is lost
+         * during the instrumentation. The redefinition is applied upon the original byte code that is provided by a class
+         * loader and not upon the code in its currently transformed format. Use
+         * {@link net.bytebuddy.agent.builder.AgentBuilder.RedefinitionStrategy#RETRANSFORMATION} if this is a factual or
+         * potential limitation.
+         * </p>
          */
         REDEFINITION {
             @Override
@@ -1139,8 +1148,9 @@ protected interface Collector {
              * Considers a loaded class for modification.
              *
              * @param type The type that is to be considered.
+             * @return {@code true} if the class is considered to be redefined.
              */
-            void consider(Class<?> type);
+            boolean consider(Class<?> type);
 
             /**
              * Applies the represented type modification on all collected types.
@@ -1193,14 +1203,12 @@ protected ForRedefinition(Default.Transformation transformation) {
                 }
 
                 @Override
-                public void consider(Class<?> type) {
+                public boolean consider(Class<?> type) {
                     Default.Transformation.Resolution resolution = transformation.resolve(new TypeDescription.ForLoadedType(type),
                             type.getClassLoader(),
                             type,
                             type.getProtectionDomain());
-                    if (resolution.isResolved()) {
+                    return resolution.isResolved() && entries.add(new Entry(type, resolution));
-                        entries.add(new Entry(type, resolution));
-                    }
                 }
 
                 @Override
@@ -1358,10 +1366,9 @@ protected ForRetransformation(Default.Transformation transformation) {
                 }
 
                 @Override
-                public void consider(Class<?> type) {
+                public boolean consider(Class<?> type) {
-                    if (transformation.resolve(new TypeDescription.ForLoadedType(type), type.getClassLoader(), type, type.getProtectionDomain()).isResolved()) {
+                    return transformation.resolve(new TypeDescription.ForLoadedType(type),
-                        types.add(type);
+                            type.getClassLoader(), type, type.getProtectionDomain()).isResolved() && types.add(type);
-                    }
                 }
 
                 @Override
@@ -1720,16 +1727,27 @@ public ClassFileTransformer installOn(Instrumentation instrumentation) {
             if (redefinitionStrategy.isEnabled()) {
                 RedefinitionStrategy.Collector collector = redefinitionStrategy.makeCollector(transformation);
                 for (Class<?> type : instrumentation.getAllLoadedClasses()) {
-                    if (instrumentation.isModifiableClass(type)) {
+                    try {
-                        // To enforce identical listener semantics for a redefinition as for an agent instrumentation, this catching is required.
+                        if (!instrumentation.isModifiableClass(type) || !collector.consider(type)) {
+                            try {
+                                try {
+                                    listener.onIgnored(new TypeDescription.ForLoadedType(type));
+                                } finally {
+                                    listener.onComplete(type.getName());
+                                }
+                            } catch (Throwable ignored) {
+                                // Ignore exceptions that are thrown by listeners to mimic the behavior of a transformation.
+                            }
+                        }
+                    } catch (Throwable throwable) {
                         try {
-                            collector.consider(type);
-                        } catch (Throwable throwable) {
                             try {
                                 listener.onError(type.getName(), throwable);
                             } finally {
                                 listener.onComplete(type.getName());
                             }
+                        } catch (Throwable ignored) {
+                            // Ignore exceptions that are thrown by listeners to mimic the behavior of a transformation.
                         }
                     }
                 }

diff --git a/byte-buddy-dep/src/test/java/net/bytebuddy/agent/builder/AgentBuilderDefaultTest.java b/byte-buddy-dep/src/test/java/net/bytebuddy/agent/builder/AgentBuilderDefaultTest.java
@@ -62,7 +62,7 @@ public class AgentBuilderDefaultTest {
 
     @Mock
     private AgentBuilder.TypeStrategy typeStrategy;
-    
+
     @Mock
     private AgentBuilder.InitializationStrategy initializationStrategy;
 
@@ -164,7 +164,9 @@ public void testSkipRetransformationWithNonRedefinable() throws Exception {
                 .withoutNativeMethodPrefix()
                 .type(rawMatcher).transform(transformer)
                 .installOn(instrumentation);
-        verifyZeroInteractions(listener);
+        verify(listener).onIgnored(new TypeDescription.ForLoadedType(REDEFINED));
+        verify(listener).onComplete(REDEFINED.getName());
+        verifyNoMoreInteractions(listener);
         verify(instrumentation).addTransformer(classFileTransformer, true);
         verify(instrumentation).isModifiableClass(REDEFINED);
         verify(instrumentation).getAllLoadedClasses();
@@ -191,7 +193,71 @@ public void testSkipRetransformationWithNonMatched() throws Exception {
                 .withoutNativeMethodPrefix()
                 .type(rawMatcher).transform(transformer)
                 .installOn(instrumentation);
-        verifyZeroInteractions(listener);
+        verify(listener).onIgnored(new TypeDescription.ForLoadedType(REDEFINED));
+        verify(listener).onComplete(REDEFINED.getName());
+        verifyNoMoreInteractions(listener);
+        verify(instrumentation).addTransformer(classFileTransformer, true);
+        verify(instrumentation).isModifiableClass(REDEFINED);
+        verify(instrumentation).getAllLoadedClasses();
+        verify(instrumentation).isRetransformClassesSupported();
+        verifyNoMoreInteractions(instrumentation);
+        verify(rawMatcher).matches(new TypeDescription.ForLoadedType(REDEFINED), REDEFINED.getClassLoader(), REDEFINED, REDEFINED.getProtectionDomain());
+        verifyNoMoreInteractions(rawMatcher);
+        verifyZeroInteractions(initializationStrategy);
+    }
+
+    @Test
+    public void testSkipRetransformationWithNonMatchedListenerException() throws Exception {
+        when(unloaded.getBytes()).thenReturn(BAZ);
+        when(resolution.resolve()).thenReturn(new TypeDescription.ForLoadedType(REDEFINED));
+        when(rawMatcher.matches(new TypeDescription.ForLoadedType(REDEFINED), REDEFINED.getClassLoader(), REDEFINED, REDEFINED.getProtectionDomain()))
+                .thenReturn(false);
+        when(instrumentation.isModifiableClass(REDEFINED)).thenReturn(true);
+        when(instrumentation.isRetransformClassesSupported()).thenReturn(true);
+        doThrow(new RuntimeException()).when(listener).onIgnored(new TypeDescription.ForLoadedType(REDEFINED));
+        ClassFileTransformer classFileTransformer = new AgentBuilder.Default(byteBuddy)
+                .withInitializationStrategy(initializationStrategy)
+                .withRedefinitionStrategy(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
+                .withBinaryLocator(binaryLocator)
+                .withTypeStrategy(typeStrategy)
+                .withListener(listener)
+                .withoutNativeMethodPrefix()
+                .type(rawMatcher).transform(transformer)
+                .installOn(instrumentation);
+        verify(listener).onIgnored(new TypeDescription.ForLoadedType(REDEFINED));
+        verify(listener).onComplete(REDEFINED.getName());
+        verifyNoMoreInteractions(listener);
+        verify(instrumentation).addTransformer(classFileTransformer, true);
+        verify(instrumentation).isModifiableClass(REDEFINED);
+        verify(instrumentation).getAllLoadedClasses();
+        verify(instrumentation).isRetransformClassesSupported();
+        verifyNoMoreInteractions(instrumentation);
+        verify(rawMatcher).matches(new TypeDescription.ForLoadedType(REDEFINED), REDEFINED.getClassLoader(), REDEFINED, REDEFINED.getProtectionDomain());
+        verifyNoMoreInteractions(rawMatcher);
+        verifyZeroInteractions(initializationStrategy);
+    }
+
+    @Test
+    public void testSkipRetransformationWithNonMatchedListenerCompleteException() throws Exception {
+        when(unloaded.getBytes()).thenReturn(BAZ);
+        when(resolution.resolve()).thenReturn(new TypeDescription.ForLoadedType(REDEFINED));
+        when(rawMatcher.matches(new TypeDescription.ForLoadedType(REDEFINED), REDEFINED.getClassLoader(), REDEFINED, REDEFINED.getProtectionDomain()))
+                .thenReturn(false);
+        when(instrumentation.isModifiableClass(REDEFINED)).thenReturn(true);
+        when(instrumentation.isRetransformClassesSupported()).thenReturn(true);
+        doThrow(new RuntimeException()).when(listener).onComplete(REDEFINED.getName());
+        ClassFileTransformer classFileTransformer = new AgentBuilder.Default(byteBuddy)
+                .withInitializationStrategy(initializationStrategy)
+                .withRedefinitionStrategy(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
+                .withBinaryLocator(binaryLocator)
+                .withTypeStrategy(typeStrategy)
+                .withListener(listener)
+                .withoutNativeMethodPrefix()
+                .type(rawMatcher).transform(transformer)
+                .installOn(instrumentation);
+        verify(listener).onIgnored(new TypeDescription.ForLoadedType(REDEFINED));
+        verify(listener).onComplete(REDEFINED.getName());
+        verifyNoMoreInteractions(listener);
         verify(instrumentation).addTransformer(classFileTransformer, true);
         verify(instrumentation).isModifiableClass(REDEFINED);
         verify(instrumentation).getAllLoadedClasses();
@@ -258,7 +324,9 @@ public void testSkipRedefinitionWithNonRedefinable() throws Exception {
                 .withoutNativeMethodPrefix()
                 .type(rawMatcher).transform(transformer)
                 .installOn(instrumentation);
-        verifyZeroInteractions(listener);
+        verify(listener).onIgnored(new TypeDescription.ForLoadedType(REDEFINED));
+        verify(listener).onComplete(REDEFINED.getName());
+        verifyNoMoreInteractions(listener);
         verify(instrumentation).addTransformer(classFileTransformer, false);
         verify(instrumentation).isModifiableClass(REDEFINED);
         verify(instrumentation).getAllLoadedClasses();
@@ -285,7 +353,71 @@ public void testSkipRedefinitionWithNonMatched() throws Exception {
                 .withoutNativeMethodPrefix()
                 .type(rawMatcher).transform(transformer)
                 .installOn(instrumentation);
-        verifyZeroInteractions(listener);
+        verify(listener).onIgnored(new TypeDescription.ForLoadedType(REDEFINED));
+        verify(listener).onComplete(REDEFINED.getName());
+        verifyNoMoreInteractions(listener);
+        verify(instrumentation).addTransformer(classFileTransformer, false);
+        verify(instrumentation).isModifiableClass(REDEFINED);
+        verify(instrumentation).getAllLoadedClasses();
+        verify(instrumentation).isRedefineClassesSupported();
+        verifyNoMoreInteractions(instrumentation);
+        verify(rawMatcher).matches(new TypeDescription.ForLoadedType(REDEFINED), REDEFINED.getClassLoader(), REDEFINED, REDEFINED.getProtectionDomain());
+        verifyNoMoreInteractions(rawMatcher);
+        verifyZeroInteractions(initializationStrategy);
+    }
+
+    @Test
+    public void testSkipRedefinitionWithNonMatchedListenerException() throws Exception {
+        when(unloaded.getBytes()).thenReturn(BAZ);
+        when(resolution.resolve()).thenReturn(new TypeDescription.ForLoadedType(REDEFINED));
+        when(rawMatcher.matches(new TypeDescription.ForLoadedType(REDEFINED), REDEFINED.getClassLoader(), REDEFINED, REDEFINED.getProtectionDomain()))
+                .thenReturn(false);
+        when(instrumentation.isModifiableClass(REDEFINED)).thenReturn(true);
+        when(instrumentation.isRedefineClassesSupported()).thenReturn(true);
+        doThrow(new RuntimeException()).when(listener).onIgnored(new TypeDescription.ForLoadedType(REDEFINED));
+        ClassFileTransformer classFileTransformer = new AgentBuilder.Default(byteBuddy)
+                .withInitializationStrategy(initializationStrategy)
+                .withRedefinitionStrategy(AgentBuilder.RedefinitionStrategy.REDEFINITION)
+                .withBinaryLocator(binaryLocator)
+                .withTypeStrategy(typeStrategy)
+                .withListener(listener)
+                .withoutNativeMethodPrefix()
+                .type(rawMatcher).transform(transformer)
+                .installOn(instrumentation);
+        verify(listener).onIgnored(new TypeDescription.ForLoadedType(REDEFINED));
+        verify(listener).onComplete(REDEFINED.getName());
+        verifyNoMoreInteractions(listener);
+        verify(instrumentation).addTransformer(classFileTransformer, false);
+        verify(instrumentation).isModifiableClass(REDEFINED);
+        verify(instrumentation).getAllLoadedClasses();
+        verify(instrumentation).isRedefineClassesSupported();
+        verifyNoMoreInteractions(instrumentation);
+        verify(rawMatcher).matches(new TypeDescription.ForLoadedType(REDEFINED), REDEFINED.getClassLoader(), REDEFINED, REDEFINED.getProtectionDomain());
+        verifyNoMoreInteractions(rawMatcher);
+        verifyZeroInteractions(initializationStrategy);
+    }
+
+    @Test
+    public void testSkipRedefinitionWithNonMatchedListenerFinishedException() throws Exception {
+        when(unloaded.getBytes()).thenReturn(BAZ);
+        when(resolution.resolve()).thenReturn(new TypeDescription.ForLoadedType(REDEFINED));
+        when(rawMatcher.matches(new TypeDescription.ForLoadedType(REDEFINED), REDEFINED.getClassLoader(), REDEFINED, REDEFINED.getProtectionDomain()))
+                .thenReturn(false);
+        when(instrumentation.isModifiableClass(REDEFINED)).thenReturn(true);
+        when(instrumentation.isRedefineClassesSupported()).thenReturn(true);
+        doThrow(new RuntimeException()).when(listener).onComplete(REDEFINED.getName());
+        ClassFileTransformer classFileTransformer = new AgentBuilder.Default(byteBuddy)
+                .withInitializationStrategy(initializationStrategy)
+                .withRedefinitionStrategy(AgentBuilder.RedefinitionStrategy.REDEFINITION)
+                .withBinaryLocator(binaryLocator)
+                .withTypeStrategy(typeStrategy)
+                .withListener(listener)
+                .withoutNativeMethodPrefix()
+                .type(rawMatcher).transform(transformer)
+                .installOn(instrumentation);
+        verify(listener).onIgnored(new TypeDescription.ForLoadedType(REDEFINED));
+        verify(listener).onComplete(REDEFINED.getName());
+        verifyNoMoreInteractions(listener);
         verify(instrumentation).addTransformer(classFileTransformer, false);
         verify(instrumentation).isModifiableClass(REDEFINED);
         verify(instrumentation).getAllLoadedClasses();
@@ -472,6 +604,54 @@ public void testRetransformationConsiderationException() throws Exception {
         verifyNoMoreInteractions(listener);
     }
 
+    @Test
+    public void testRedefinitionConsiderationExceptionListenerException() throws Exception {
+        RuntimeException exception = new RuntimeException();
+        when(instrumentation.isRedefineClassesSupported()).thenReturn(true);
+        when(instrumentation.getAllLoadedClasses()).thenReturn(new Class<?>[]{REDEFINED});
+        when(instrumentation.isModifiableClass(REDEFINED)).thenReturn(true);
+        when(rawMatcher.matches(new TypeDescription.ForLoadedType(REDEFINED), REDEFINED.getClassLoader(), REDEFINED, REDEFINED.getProtectionDomain()))
+                .thenThrow(exception);
+        doThrow(new RuntimeException()).when(listener).onError(REDEFINED.getName(), exception);
+        ClassFileTransformer classFileTransformer = new AgentBuilder.Default(byteBuddy)
+                .withInitializationStrategy(initializationStrategy)
+                .withRedefinitionStrategy(AgentBuilder.RedefinitionStrategy.REDEFINITION)
+                .withBinaryLocator(binaryLocator)
+                .withTypeStrategy(typeStrategy)
+                .withListener(listener)
+                .withoutNativeMethodPrefix()
+                .type(rawMatcher).transform(transformer)
+                .installOn(instrumentation);
+        verify(instrumentation).addTransformer(classFileTransformer, false);
+        verify(listener).onError(REDEFINED.getName(), exception);
+        verify(listener).onComplete(REDEFINED.getName());
+        verifyNoMoreInteractions(listener);
+    }
+
+    @Test
+    public void testRetransformationConsiderationExceptionListenerException() throws Exception {
+        RuntimeException exception = new RuntimeException();
+        when(instrumentation.isRetransformClassesSupported()).thenReturn(true);
+        when(instrumentation.getAllLoadedClasses()).thenReturn(new Class<?>[]{REDEFINED});
+        when(instrumentation.isModifiableClass(REDEFINED)).thenReturn(true);
+        when(rawMatcher.matches(new TypeDescription.ForLoadedType(REDEFINED), REDEFINED.getClassLoader(), REDEFINED, REDEFINED.getProtectionDomain()))
+                .thenThrow(exception);
+        doThrow(new RuntimeException()).when(listener).onError(REDEFINED.getName(), exception);
+        ClassFileTransformer classFileTransformer = new AgentBuilder.Default(byteBuddy)
+                .withInitializationStrategy(initializationStrategy)
+                .withRedefinitionStrategy(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
+                .withBinaryLocator(binaryLocator)
+                .withTypeStrategy(typeStrategy)
+                .withListener(listener)
+                .withoutNativeMethodPrefix()
+                .type(rawMatcher).transform(transformer)
+                .installOn(instrumentation);
+        verify(instrumentation).addTransformer(classFileTransformer, true);
+        verify(listener).onError(REDEFINED.getName(), exception);
+        verify(listener).onComplete(REDEFINED.getName());
+        verifyNoMoreInteractions(listener);
+    }
+
     @Test
     public void testObjectProperties() throws Exception {
         ObjectPropertyAssertion.of(AgentBuilder.Default.class).create(new ObjectPropertyAssertion.Creator<AccessControlContext>() {