Avoid stack frame re-computation + prevent indirect classloading (#1758)

* remove hybrid to avoid classloading
* avoid stack frame re-computation with >= java7

Co-authored-by: eyalkoren <[email protected]>

Author: SylvainJuge

apm-agent-core/src/main/java/co/elastic/apm/agent/bci/ElasticApmAgent.java

@@ -720,7 +720,7 @@ public static void ensureInstrumented(Class<?> classToInstrument, Collection<Cla
                         .with(TypeValidation.of(logger.isDebugEnabled()))
                         .with(FailSafeDeclaredMethodsCompiler.INSTANCE);
                     AgentBuilder agentBuilder = getAgentBuilder(
-                        byteBuddy, config, logger, AgentBuilder.DescriptionStrategy.Default.HYBRID, false, false
+                        byteBuddy, config, logger, AgentBuilder.DescriptionStrategy.Default.POOL_ONLY, false, false
                     );
                     for (Class<? extends ElasticApmInstrumentation> instrumentationClass : instrumentationClasses) {
                         ElasticApmInstrumentation apmInstrumentation = instantiate(instrumentationClass);

apm-agent-core/src/main/java/co/elastic/apm/agent/bci/bytebuddy/PatchBytecodeVersionTo51Transformer.java

@@ -25,6 +25,7 @@
 package co.elastic.apm.agent.bci.bytebuddy;
 
 import co.elastic.apm.agent.bci.TracerAwareInstrumentation;
+import net.bytebuddy.ClassFileVersion;
 import net.bytebuddy.agent.builder.AgentBuilder;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.asm.AsmVisitorWrapper;
@@ -48,12 +49,25 @@
  * {@link Advice.WithCustomMapping#bootstrap} which is important for {@linkplain TracerAwareInstrumentation#indyPlugin() indy plugins}.
  */
 public class PatchBytecodeVersionTo51Transformer implements AgentBuilder.Transformer {
+
+    private static boolean isAtLeastJava7(TypeDescription typeDescription) {
+        ClassFileVersion classFileVersion = typeDescription.getClassFileVersion();
+        return classFileVersion != null && classFileVersion.getJavaVersion() >= 7;
+    }
+
     @Override
     public DynamicType.Builder<?> transform(DynamicType.Builder<?> builder, TypeDescription typeDescription, ClassLoader classLoader, JavaModule javaModule) {
+        if (isAtLeastJava7(typeDescription)) {
+            // we can avoid the expensive (and somewhat dangerous) stack frame re-computation if stack frames are already
+            // present in the bytecode, which also allows eagerly loading types that might be present in the method
+            // body, but not yet loaded by the JVM.
+            return builder;
+        }
         return builder.visit(new AsmVisitorWrapper.AbstractBase() {
             @Override
             public ClassVisitor wrap(TypeDescription typeDescription, ClassVisitor classVisitor, Implementation.Context context,
                                      TypePool typePool, FieldList<FieldDescription.InDefinedShape> fieldList, MethodList<?> methodList, int writerFlags, int readerFlags) {
+
                 return new ClassVisitor(Opcodes.ASM7, classVisitor) {
                     private boolean patchVersion;
 

apm-agent-core/src/main/java/co/elastic/apm/agent/util/ExecutorUtils.java

@@ -197,15 +197,19 @@ private static void logException(Runnable r, @Nullable Throwable t) {
      * Implementation adapted form the {@link ExecutorService} Javadoc
      */
     public static void shutdownAndWaitTermination(ExecutorService executor) {
+        shutdownAndWaitTermination(executor, 1, TimeUnit.SECONDS);
+    }
+
+    public static void shutdownAndWaitTermination(ExecutorService executor, long timeout, TimeUnit unit){
         // Disable new tasks from being submitted
         executor.shutdown();
         try {
             // Wait a while for existing tasks to terminate
-            if (!executor.awaitTermination(1, TimeUnit.SECONDS)) {
+            if (!executor.awaitTermination(timeout, unit)) {
                 // Cancel currently executing tasks
                 executor.shutdownNow();
                 // Wait a while for tasks to respond to being cancelled
-                if (!executor.awaitTermination(1, TimeUnit.SECONDS)) {
+                if (!executor.awaitTermination(timeout, unit)) {
                     logger.warn("Thread pool did not terminate in time " + executor);
                 }
             }