New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementing a profiler agen with Byte Buddy which automatically attaches #110

Closed
felixbarny opened this Issue Apr 1, 2016 · 139 comments

Comments

Projects
None yet
3 participants
@felixbarny
Contributor

felixbarny commented Apr 1, 2016

Hi,

I'm the developer of stagemonitor and I'm considering replacing the Javassist-based profiler with Byte Buddy. It would be great if you could help me out as I'm stuck.

I've created an example project for a Byte Buddy proof of concept which touches the most important areas: https://github.com/felixbarny/byte-buddy-test/blob/master/src/test/java/net/bytebuddy/test/TestByteBuddyProfiler.java

  • Runtime attachment
  • Capture signature in custom format
  • Capture method arguments
  • Insert profiling code on enter and on exit

I also am not sure whether the AgentBuilder or the @Advice API is preferable and if those two approaches can be combined.

Thx in advance.

@raphw raphw self-assigned this Apr 2, 2016

@raphw raphw added this to the 1.3.6 milestone Apr 2, 2016

@raphw raphw added the question label Apr 2, 2016

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 2, 2016

Owner

Hi Felix,

the interception API requires the modification of the original class format which is not permitted by most JVMs. (Actually, there is a JEP for allow such transformations, one beautiful day in the future.)

For runtime instrumentation, you need to use the advice API. For timing a method, you would use code like the following:

class Advice {
  @Advice.OnMethodEnter
  static long enter() {
    return System.nanoTime();
  }
  @Advice.OnMethodExit
  static exit(@Advice.Return long value) {
    System.out.println(System.nanoTime() - value);
  }
}

This was the easy part. Unfortunately, the Advice API does not currently allow to intercept methods with arbitrary arity what I assume you require. Otherwise, an explicit argument of an intercepted method can be accessed using @Advice.Argument(x). Adding a possibility to extract a signature would be trivial but expanding the latter mechanism to random arities would be a tougher extension.

Owner

raphw commented Apr 2, 2016

Hi Felix,

the interception API requires the modification of the original class format which is not permitted by most JVMs. (Actually, there is a JEP for allow such transformations, one beautiful day in the future.)

For runtime instrumentation, you need to use the advice API. For timing a method, you would use code like the following:

class Advice {
  @Advice.OnMethodEnter
  static long enter() {
    return System.nanoTime();
  }
  @Advice.OnMethodExit
  static exit(@Advice.Return long value) {
    System.out.println(System.nanoTime() - value);
  }
}

This was the easy part. Unfortunately, the Advice API does not currently allow to intercept methods with arbitrary arity what I assume you require. Otherwise, an explicit argument of an intercepted method can be accessed using @Advice.Argument(x). Adding a possibility to extract a signature would be trivial but expanding the latter mechanism to random arities would be a tougher extension.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 2, 2016

Contributor

I just scanned through my StagemonitorJavassistInstrumenters and I actually don't need access to the method arguments. But I have come across some more requirements:

Is it possible to use the Advice API with the AgentBuilder to for example profile all classes that are in a certain package or is it only possible to instrument individual classes?

These are some of the pain points I currently have with javassist:

  • Especially my ProfilingInstrumenter is quite slow, because it is applied to many methods and Javassist has to compile the source every time. This increases startup times a bit
  • Instrumenting classes that are loaded by a different classloader than the ApplicationClassLoader is very tricky in application server environments. For example the javax.sql.DataSource is often loaded by a ModuleClassLoader of the application server and the class is loaded only once, even if you have deployed multiple applications. This makes it very hard to only apply the instrumentation to one of the deployed applications and requires some fancy hacks like in ConnectionMonitoringInstrumenter where I (ab)use System.getProperties().put(Object, Object) to share objects between classloaders. This leads to a lot of trouble because many libraries only expect Strings in the system properties: stagemonitor/stagemonitor#99.
  • Javassist loads the CtClasses classes directly from the classpath and thus erases any previous bytecode manipulations: stagemonitor/stagemonitor#130
Contributor

felixbarny commented Apr 2, 2016

I just scanned through my StagemonitorJavassistInstrumenters and I actually don't need access to the method arguments. But I have come across some more requirements:

Is it possible to use the Advice API with the AgentBuilder to for example profile all classes that are in a certain package or is it only possible to instrument individual classes?

These are some of the pain points I currently have with javassist:

  • Especially my ProfilingInstrumenter is quite slow, because it is applied to many methods and Javassist has to compile the source every time. This increases startup times a bit
  • Instrumenting classes that are loaded by a different classloader than the ApplicationClassLoader is very tricky in application server environments. For example the javax.sql.DataSource is often loaded by a ModuleClassLoader of the application server and the class is loaded only once, even if you have deployed multiple applications. This makes it very hard to only apply the instrumentation to one of the deployed applications and requires some fancy hacks like in ConnectionMonitoringInstrumenter where I (ab)use System.getProperties().put(Object, Object) to share objects between classloaders. This leads to a lot of trouble because many libraries only expect Strings in the system properties: stagemonitor/stagemonitor#99.
  • Javassist loads the CtClasses classes directly from the classpath and thus erases any previous bytecode manipulations: stagemonitor/stagemonitor#130

@raphw raphw added the enhancement label Apr 3, 2016

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 3, 2016

Owner

Hi Felix, all of this is possible using Byte Buddy. Byte Buddy agents are created by specifying ElementMatchers. This way, you can specify generic type / method pairs to instrument. Doing so, you can:

  • Adding local variables: In the code example above, I already showed how you can transport a local variable on the stack from the before to the after method. For the future, I also plan to allow adding further local variables by a designated annotation such that becomes possible to transport several values via the stack.
  • Accessing the return value: Add an annotation@Advice.Return in the @Advice.OnMethodExit method.
  • Adding catch clauses: Add an annotation @Advice.Thrown in the @Advice.OnMethodExit method. If no exception was thrown from the intercepted method, this parameter will be null.

As for the other points:

  • Performance: Byte Buddy should be much faster here. It uses the advice methods as byte code templates and simply remaps parameter access to the corresponding values of the instrumented method. All of this is single pass-through. One current performance downside is that Byte Buddy still computes the frames from scratch, so it could be even faster but this feature is on the bucket list.
  • Class loaders: The type matching is applied by matching a type / class loader pair. To share objects between classes loaded by different loaders, I would suggest you to inject a dispatcher class into the bootstrap class loader which is visible to all class loaders. That is common practice for communicating without being constraint by custom class loader implementations. Such a class would be quite trivial. For example, a class like:
public class Dispatcher {
  public static Map<Object, Object> VALUES = new HashMap<>();
}

can be injected into the bootstrap class loader using the Instrumentation API. Byte Buddy can help you to write a class file into a jar but this is not too difficult to do, even without the library.

  • Chaining agents: Byte Buddy respects chained agents by default.
Owner

raphw commented Apr 3, 2016

Hi Felix, all of this is possible using Byte Buddy. Byte Buddy agents are created by specifying ElementMatchers. This way, you can specify generic type / method pairs to instrument. Doing so, you can:

  • Adding local variables: In the code example above, I already showed how you can transport a local variable on the stack from the before to the after method. For the future, I also plan to allow adding further local variables by a designated annotation such that becomes possible to transport several values via the stack.
  • Accessing the return value: Add an annotation@Advice.Return in the @Advice.OnMethodExit method.
  • Adding catch clauses: Add an annotation @Advice.Thrown in the @Advice.OnMethodExit method. If no exception was thrown from the intercepted method, this parameter will be null.

As for the other points:

  • Performance: Byte Buddy should be much faster here. It uses the advice methods as byte code templates and simply remaps parameter access to the corresponding values of the instrumented method. All of this is single pass-through. One current performance downside is that Byte Buddy still computes the frames from scratch, so it could be even faster but this feature is on the bucket list.
  • Class loaders: The type matching is applied by matching a type / class loader pair. To share objects between classes loaded by different loaders, I would suggest you to inject a dispatcher class into the bootstrap class loader which is visible to all class loaders. That is common practice for communicating without being constraint by custom class loader implementations. Such a class would be quite trivial. For example, a class like:
public class Dispatcher {
  public static Map<Object, Object> VALUES = new HashMap<>();
}

can be injected into the bootstrap class loader using the Instrumentation API. Byte Buddy can help you to write a class file into a jar but this is not too difficult to do, even without the library.

  • Chaining agents: Byte Buddy respects chained agents by default.
@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 3, 2016

Contributor

Hi Raphael,
that's awesome news!

Could you give me a quick example of how to use the Advice API in combination with the AgentBuilder API?

For example, a class like: [...] can be injected into the bootstrap class loader using the Instrumentation API

I guess you mean the instrumentation.appendToBootstrapClassLoaderSearch(JarFile) method? How can Byte Buddy assist here? The Dispatcher pattern is exactly what I need, thanks a lot! Works just like the system property hack but without breaking things for others. Do you know of any blog which describes that in more detail?

Contributor

felixbarny commented Apr 3, 2016

Hi Raphael,
that's awesome news!

Could you give me a quick example of how to use the Advice API in combination with the AgentBuilder API?

For example, a class like: [...] can be injected into the bootstrap class loader using the Instrumentation API

I guess you mean the instrumentation.appendToBootstrapClassLoaderSearch(JarFile) method? How can Byte Buddy assist here? The Dispatcher pattern is exactly what I need, thanks a lot! Works just like the system property hack but without breaking things for others. Do you know of any blog which describes that in more detail?

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 3, 2016

Owner

Hi Felix,

the unit tests show several examples of how to use the Advice API. With the agent builder, there does not change much. You simply integrate it into the builder as in the unit tests. The interface is identical:

new AgentBuilder.Default()
                .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
                .with(AgentBuilder.InitializationStrategy.NoOp.INSTANCE)
                .with(AgentBuilder.TypeStrategy.Default.REDEFINE)
                .type(ElementMatchers.named(ProfiledClass.class.getName()))
                .transform((builder, typeDescription, classLoader) -> builder
                        .visit(Advice.to(MyAdvice.class).on(ElementMatchers.any()))
                .installOnByteBuddyAgent();

As for the appending: Yes, this is what I mean. You would need to create a seperate module that only contains this single class. The accessing code would only reference this module in provided scope whereas the agent would contain the entire jar file, store it on disk and append it to the bootstrap class path before applying the instrumentation. This way, the class becomes available universally on the JVM. Byte Buddy has an API for extracting a class file for a given class but ideally you would simply bundle the full jar and never load it implciitly via the agent. If the Dispatcher class ends up being loaded twice, you might end up accessing two different maps from different places.

Please not that you should use the latest snapshot using a solution like Jitpack. I have extended the Advice API a little bit last night and you might require the additional annotations. If you find any problems, please let me know and I will look into it and see how I can improve things.

I am closing the issue. But please feel free to reopen it if there are more questions.

Owner

raphw commented Apr 3, 2016

Hi Felix,

the unit tests show several examples of how to use the Advice API. With the agent builder, there does not change much. You simply integrate it into the builder as in the unit tests. The interface is identical:

new AgentBuilder.Default()
                .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION)
                .with(AgentBuilder.InitializationStrategy.NoOp.INSTANCE)
                .with(AgentBuilder.TypeStrategy.Default.REDEFINE)
                .type(ElementMatchers.named(ProfiledClass.class.getName()))
                .transform((builder, typeDescription, classLoader) -> builder
                        .visit(Advice.to(MyAdvice.class).on(ElementMatchers.any()))
                .installOnByteBuddyAgent();

As for the appending: Yes, this is what I mean. You would need to create a seperate module that only contains this single class. The accessing code would only reference this module in provided scope whereas the agent would contain the entire jar file, store it on disk and append it to the bootstrap class path before applying the instrumentation. This way, the class becomes available universally on the JVM. Byte Buddy has an API for extracting a class file for a given class but ideally you would simply bundle the full jar and never load it implciitly via the agent. If the Dispatcher class ends up being loaded twice, you might end up accessing two different maps from different places.

Please not that you should use the latest snapshot using a solution like Jitpack. I have extended the Advice API a little bit last night and you might require the additional annotations. If you find any problems, please let me know and I will look into it and see how I can improve things.

I am closing the issue. But please feel free to reopen it if there are more questions.

@raphw raphw closed this Apr 3, 2016

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 3, 2016

Contributor

Ah, nice. I was missing the builder.visit(Advice.to(MyAdvice.class).on(ElementMatchers.any())) part.

Would the agent contain the jar as a resource? This is my current understanding of it:

agent
+- src/main/java
|   \- org.example.Agent.java // instrumentation.appendToBootstrapClassLoaderSearch(dispatcherJar)
+- src/main/resources
|   \- dispatcher.jar
|       \- org.example.Dispatcher.class
\- pom.xml
    provided dispatcher
    some magic to add the dispatcher jar into the agent jar

dispatcher
\- src/main/java
    \- org.example.Dispatcher.java

accessing-code
+- src/main/java
|   \- ... // Dispatcher.VALUES.get(String)
\- pom.xml
    provided dispatcher
Contributor

felixbarny commented Apr 3, 2016

Ah, nice. I was missing the builder.visit(Advice.to(MyAdvice.class).on(ElementMatchers.any())) part.

Would the agent contain the jar as a resource? This is my current understanding of it:

agent
+- src/main/java
|   \- org.example.Agent.java // instrumentation.appendToBootstrapClassLoaderSearch(dispatcherJar)
+- src/main/resources
|   \- dispatcher.jar
|       \- org.example.Dispatcher.class
\- pom.xml
    provided dispatcher
    some magic to add the dispatcher jar into the agent jar

dispatcher
\- src/main/java
    \- org.example.Dispatcher.java

accessing-code
+- src/main/java
|   \- ... // Dispatcher.VALUES.get(String)
\- pom.xml
    provided dispatcher
@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 3, 2016

Owner

Yes, this is exactly how it would be done.

Owner

raphw commented Apr 3, 2016

Yes, this is exactly how it would be done.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 4, 2016

Contributor

Have you already added the possibility to extract a signature in a custom format?

nevermind, just found this commit: 7823ff8

Contributor

felixbarny commented Apr 4, 2016

Have you already added the possibility to extract a signature in a custom format?

nevermind, just found this commit: 7823ff8

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 4, 2016

Contributor

I tried using the Advice API but I thinks there is something wrong with my matchers. The advice does not get applied: https://github.com/felixbarny/byte-buddy-test/blob/686953fd20b15036f4e01abbc87194a425edbdd8/src/test/java/net/bytebuddy/test/TestByteBuddyProfiler.java

Also, how do I capture the return value when value is the added local variable?

Contributor

felixbarny commented Apr 4, 2016

I tried using the Advice API but I thinks there is something wrong with my matchers. The advice does not get applied: https://github.com/felixbarny/byte-buddy-test/blob/686953fd20b15036f4e01abbc87194a425edbdd8/src/test/java/net/bytebuddy/test/TestByteBuddyProfiler.java

Also, how do I capture the return value when value is the added local variable?

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 4, 2016

Owner

There are several problems with your code. First of all, I recommend you to register a listener to your agent for debugging purposes. Byte Buddy prevents the instrumenting procedure from throwing exceptions what is not supposed to happen within the VM's internal byte code transformation stack. Therefore, illegal transformations are simply suppressed. You can for example set:

new Agent.Builder().with(AgentBuilder.Listener.StreamWriting.toSystemOut())

within your test. Doing so, you can see that the advice adapter complains about incompatible types in your exit-mapping. You are using @Advice.Return where you want to use @Advice.Enter. But as a matter of fact you want to do both:

@Advice.OnMethodExit
public static void exit(@Advice.Enter long value, @Advice.Return int value2) {    
  returnValue = value2;
  executionTime = System.nanoTime() - value;
}

Finally, the Advice adapter will still complain. It will say that the makeSureClassIsLoaded and the class's constructor do not return int. If you change your matcher to ElementMatchers.returns(int.class), everything works as you expect it.

Note that the current release adds some performance improvements to the Advice which do however break for advice with "too comples" branching instructions. To avoid this, set

Advice.to(ProfilingAdvice.class)
  .on(ElementMatchers.returns(int.class))
  .writerFlags(ClassWriter.COMPUTE_FRAMES))

for now. I hopefully find time to fix this some time this week.

Owner

raphw commented Apr 4, 2016

There are several problems with your code. First of all, I recommend you to register a listener to your agent for debugging purposes. Byte Buddy prevents the instrumenting procedure from throwing exceptions what is not supposed to happen within the VM's internal byte code transformation stack. Therefore, illegal transformations are simply suppressed. You can for example set:

new Agent.Builder().with(AgentBuilder.Listener.StreamWriting.toSystemOut())

within your test. Doing so, you can see that the advice adapter complains about incompatible types in your exit-mapping. You are using @Advice.Return where you want to use @Advice.Enter. But as a matter of fact you want to do both:

@Advice.OnMethodExit
public static void exit(@Advice.Enter long value, @Advice.Return int value2) {    
  returnValue = value2;
  executionTime = System.nanoTime() - value;
}

Finally, the Advice adapter will still complain. It will say that the makeSureClassIsLoaded and the class's constructor do not return int. If you change your matcher to ElementMatchers.returns(int.class), everything works as you expect it.

Note that the current release adds some performance improvements to the Advice which do however break for advice with "too comples" branching instructions. To avoid this, set

Advice.to(ProfilingAdvice.class)
  .on(ElementMatchers.returns(int.class))
  .writerFlags(ClassWriter.COMPUTE_FRAMES))

for now. I hopefully find time to fix this some time this week.

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 7, 2016

Owner

Just a quick note: With Byte Buddy 1.3.9 (released just now) everything should work as expected. The component is still quite young but I think it has now matured to something of full production quality.

I also squeezed every last bit of performance out of the Advice component by translating stack map frames compared to computing them from scratch for each interception. I also made sure that the computed frames are optimal, i.e. that they do not add any overhead compared to a scenario where the Java compiler generates them. In the process I found out that computation of stack map frames based on byte code compared to Java source code does not allow for arriving at optimal frames but results in bigger frames which contributes to the method's size. Computation also requires a lot of IO for reading many class files as the full type hierarchy for every type needs to be considered. All this goes away with the newly introduced frame translation.

Let me know how it goes. I appreciate any feedback if you find mistakes or API limitations.

Finally, for purposes of a runtime agent, I added a convenience method. Simply start your agent with:

new AgentBuilder.Default()
  .disableClassFormatChanges()
  .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION);

and you are good to go. The overall setup for redefining agents was a bit too verbose for my taste.

Owner

raphw commented Apr 7, 2016

Just a quick note: With Byte Buddy 1.3.9 (released just now) everything should work as expected. The component is still quite young but I think it has now matured to something of full production quality.

I also squeezed every last bit of performance out of the Advice component by translating stack map frames compared to computing them from scratch for each interception. I also made sure that the computed frames are optimal, i.e. that they do not add any overhead compared to a scenario where the Java compiler generates them. In the process I found out that computation of stack map frames based on byte code compared to Java source code does not allow for arriving at optimal frames but results in bigger frames which contributes to the method's size. Computation also requires a lot of IO for reading many class files as the full type hierarchy for every type needs to be considered. All this goes away with the newly introduced frame translation.

Let me know how it goes. I appreciate any feedback if you find mistakes or API limitations.

Finally, for purposes of a runtime agent, I added a convenience method. Simply start your agent with:

new AgentBuilder.Default()
  .disableClassFormatChanges()
  .with(AgentBuilder.RedefinitionStrategy.RETRANSFORMATION);

and you are good to go. The overall setup for redefining agents was a bit too verbose for my taste.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 7, 2016

Contributor

That's great news!

It will probably take a while until I find the time to migrate all of the Javassist code to Byte Buddy. But I think some migrations won't be that easy. With Javassist It's easy to make dynamic templates by just concatenating strings. For example, I'm using this in the MonitorRequestsInstrumenter to compile the dynamic request name into the bytecode that should be inserted. In Byte Buddy the bytecode templates are fairly static. Afaik, you can only customize them with the @Origin annotation. I don't know if this is possible, but one idea would be an annotation that can be put on a method which then gets evaluated at bytecode templating time (or whatever this is called) instead of be part of the bytecode that will be inserted. Does that make sense?

Also, I realized that I do need to get all method arguments. Also, is it possible to @Advice.Return Object returnValueOfAnyType?

@Advice.OnMethodEnter
public static void onEnter(@Origin Method method, @Arguments Object[] args) {
    MonitorRequestsInstrumenter.getRequestMonitor().monitorStart(new MonitoredMethodRequest(getRequestName(method), null, args));
}

@Inline
public static String getRequestName(Method Method) {
    return configuration.getBusinessTransactionNamingStrategy().getBusinessTransationName(method.getDeclaringClass().getSimpleName(), method.getName());
}

@Advice.OnMethodExit
public static void exit(@Advice.Return Object returnValueOfAnyType) {    
     MonitorRequestsInstrumenter.getRequestMonitor().monitorStop(returnValueOfAnyType);
}
Contributor

felixbarny commented Apr 7, 2016

That's great news!

It will probably take a while until I find the time to migrate all of the Javassist code to Byte Buddy. But I think some migrations won't be that easy. With Javassist It's easy to make dynamic templates by just concatenating strings. For example, I'm using this in the MonitorRequestsInstrumenter to compile the dynamic request name into the bytecode that should be inserted. In Byte Buddy the bytecode templates are fairly static. Afaik, you can only customize them with the @Origin annotation. I don't know if this is possible, but one idea would be an annotation that can be put on a method which then gets evaluated at bytecode templating time (or whatever this is called) instead of be part of the bytecode that will be inserted. Does that make sense?

Also, I realized that I do need to get all method arguments. Also, is it possible to @Advice.Return Object returnValueOfAnyType?

@Advice.OnMethodEnter
public static void onEnter(@Origin Method method, @Arguments Object[] args) {
    MonitorRequestsInstrumenter.getRequestMonitor().monitorStart(new MonitoredMethodRequest(getRequestName(method), null, args));
}

@Inline
public static String getRequestName(Method Method) {
    return configuration.getBusinessTransactionNamingStrategy().getBusinessTransationName(method.getDeclaringClass().getSimpleName(), method.getName());
}

@Advice.OnMethodExit
public static void exit(@Advice.Return Object returnValueOfAnyType) {    
     MonitorRequestsInstrumenter.getRequestMonitor().monitorStop(returnValueOfAnyType);
}
@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 7, 2016

Owner

Hi, it is no problem to implement advice adapters that are capable of reading a boxed return value or that read an array of arguments. The semantics would be pretty easy:

void advice(@Advice.BoxedReturn Object ret, @Advice.BoxedArguments Object[] args);

The boxing would happen whenever the parameter is accessed.

Adding callbacks is however less trivial but I will think about a way that allows to inject custom code by custom annotations. This would probably work something like:

Advice.to(MyAdvice.class)
           .bind(MyAnnotation.class, method -> method.getReturnValue())
           .on(myMatcher);

The first two are quick fixes but the latter one can take some time. I hope to find some time before the end of the month.

Owner

raphw commented Apr 7, 2016

Hi, it is no problem to implement advice adapters that are capable of reading a boxed return value or that read an array of arguments. The semantics would be pretty easy:

void advice(@Advice.BoxedReturn Object ret, @Advice.BoxedArguments Object[] args);

The boxing would happen whenever the parameter is accessed.

Adding callbacks is however less trivial but I will think about a way that allows to inject custom code by custom annotations. This would probably work something like:

Advice.to(MyAdvice.class)
           .bind(MyAnnotation.class, method -> method.getReturnValue())
           .on(myMatcher);

The first two are quick fixes but the latter one can take some time. I hope to find some time before the end of the month.

felixbarny pushed a commit to stagemonitor/stagemonitor that referenced this issue Apr 9, 2016

Felix Barnsteiner
Don't use System.getProperties to share objects between classloaders
Now using a dispatcher as described here: raphw/byte-buddy#110
Also use byte-buddy-agent in favour of stagemonitor-javaagent
(closes #99)
@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 9, 2016

Contributor

I've managed to implement the dispatcher. There was one pitfall, though.

JBoss' ModuleClassLoader is not kind enough to load the class, even if it is added via Instrumentation#appendToBootstrapClassLoaderSearch(JarFile). I worked around this by reflectively loading the class and getting the dispatcher's map. See stagemonitor/stagemonitor@25afd93#diff-49e6f419628c01b107f821cb65b0934a.

The only thing that stops me from merging this is #111

Thank you very much for your help! Greatly appreciate it!

Contributor

felixbarny commented Apr 9, 2016

I've managed to implement the dispatcher. There was one pitfall, though.

JBoss' ModuleClassLoader is not kind enough to load the class, even if it is added via Instrumentation#appendToBootstrapClassLoaderSearch(JarFile). I worked around this by reflectively loading the class and getting the dispatcher's map. See stagemonitor/stagemonitor@25afd93#diff-49e6f419628c01b107f821cb65b0934a.

The only thing that stops me from merging this is #111

Thank you very much for your help! Greatly appreciate it!

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 10, 2016

Contributor

I just successfully migrated the profiler to Byte Buddy!

I really love the clean and simple API. The only tricky part is boolean logic 😆.

I encountered a strange error in combination with Javassist, though. If I apply the Byte Buddy ClassFileTransformers before the Javassist ones everything is working fine, but when I put it the other way around (by swapping the lines 85 and 86 in MainStagemonitorClassFileTransformer), suddenly this test fails. It seems that the insertAfter call in MonitorRequestsInstrumenter gets deleted then.

Is there a possibility to store the transformed class on disk with byte buddy? I did this with my Javassist based ClassFileTransformer with a decorator. But the Byte Buddy API does not seem to allow for decorators to be injected.

The classes.zip contains two versions of the SlaInstrumenterTest$SlaTestClass class. In the byte-buddy-first folder you can see the result of both transformers with Byte Buddy being the first transformer. The javassist-folder contains a version of the class with only the Javassist transformation applied (because I don't know how to export the class after the Byte Buddy transformation).

Contributor

felixbarny commented Apr 10, 2016

I just successfully migrated the profiler to Byte Buddy!

I really love the clean and simple API. The only tricky part is boolean logic 😆.

I encountered a strange error in combination with Javassist, though. If I apply the Byte Buddy ClassFileTransformers before the Javassist ones everything is working fine, but when I put it the other way around (by swapping the lines 85 and 86 in MainStagemonitorClassFileTransformer), suddenly this test fails. It seems that the insertAfter call in MonitorRequestsInstrumenter gets deleted then.

Is there a possibility to store the transformed class on disk with byte buddy? I did this with my Javassist based ClassFileTransformer with a decorator. But the Byte Buddy API does not seem to allow for decorators to be injected.

The classes.zip contains two versions of the SlaInstrumenterTest$SlaTestClass class. In the byte-buddy-first folder you can see the result of both transformers with Byte Buddy being the first transformer. The javassist-folder contains a version of the class with only the Javassist transformation applied (because I don't know how to export the class after the Byte Buddy transformation).

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 10, 2016

Owner

That went fast. Thank you for giving the library a chance, that is exciting!

In the mean time, I have implemented all the required additions to the Advice adapter on the development branch. I still need to add documentation and some additional tests before I merge it to master and release a new version. However, if you want to try the features out already now, simply build the branch yourself without the checks Maven profile:

You can now read a return value as an Object type by @Advice.BoxedReturn and arguments by @Advice.BoxedArguments. You can also inject individual values into the advice methods by registering your own annotations using Advice.withCustomMapping() and using them on your advice class.

For exporting byte code, you can register an AgentBuilder.Listener where the byte code is exposed via the DynamicType instance on the onTransformation method call. From this callback, you can persist the byte array representing the transformed class file.

I can only speculate why the order of the transformers matters in your code. Byte Buddy respects transformations that were applied before it, i.e. it takes the byte array that the class file transformer hands over and does not read the class file from disk. I assume that there might be an error or some problem with the class file. A listener can expose such potential problems.

Owner

raphw commented Apr 10, 2016

That went fast. Thank you for giving the library a chance, that is exciting!

In the mean time, I have implemented all the required additions to the Advice adapter on the development branch. I still need to add documentation and some additional tests before I merge it to master and release a new version. However, if you want to try the features out already now, simply build the branch yourself without the checks Maven profile:

You can now read a return value as an Object type by @Advice.BoxedReturn and arguments by @Advice.BoxedArguments. You can also inject individual values into the advice methods by registering your own annotations using Advice.withCustomMapping() and using them on your advice class.

For exporting byte code, you can register an AgentBuilder.Listener where the byte code is exposed via the DynamicType instance on the onTransformation method call. From this callback, you can persist the byte array representing the transformed class file.

I can only speculate why the order of the transformers matters in your code. Byte Buddy respects transformations that were applied before it, i.e. it takes the byte array that the class file transformer hands over and does not read the class file from disk. I assume that there might be an error or some problem with the class file. A listener can expose such potential problems.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 10, 2016

Contributor

Thx for the hint, I've now also exported the class when the Byte Buddy transformer was last:
classes.zip

Interestingly, the monitorStart call via insertBefore does get executed but the monitorStop call added via insertAfter does not.

But all calls seem to be included in the resulting bytecode. This is an excerpt of the decompiled method (via jd):

javassist-first (not working)

@SLAs({@SLA(metric={SLA.Metric.P95, SLA.Metric.MAX}, threshold={0.0D, 0.0D}), @SLA(errorRateThreshold=0.0D)})
  @MonitorRequests
  void monitorSla()
  {
    Profiler.start("org.stagemonitor.alerting.annotation.SlaInstrumenterTest$SlaTestClass.monitorSla()V");
    try
    {
      try
      {
        MonitorRequestsInstrumenter.getRequestMonitor().monitorStart(new MonitoredMethodRequest("Monitor Sla", null, new Object[0]));throw null;
      }
      catch (Exception localException)
      {
        localException = localException;MonitorRequestsInstrumenter.getRequestMonitor().recordException(localException);throw localException;
      }
      finally
      {
        localObject2 = finally;Object localObject1 = null;MonitorRequestsInstrumenter.getRequestMonitor().monitorStop();throw ((Throwable)localObject2);
      }
    }
    finally
    {
      Profiler.stop();
      if (localException == null) {
        return;
      }
    }
  }

byte-buddy-first (working)

  @SLAs({@SLA(metric={SLA.Metric.P95, SLA.Metric.MAX}, threshold={0.0D, 0.0D}), @SLA(errorRateThreshold=0.0D)})
  @MonitorRequests
  void monitorSla()
  {
    try
    {
      MonitorRequestsInstrumenter.getRequestMonitor().monitorStart(new MonitoredMethodRequest("Monitor Sla", null, new Object[0]));Profiler.start("org.stagemonitor.alerting.annotation.SlaInstrumenterTest$SlaTestClass.monitorSla()V");
      try
      {
        throw null;
      }
      finally
      {
        Profiler.stop();
      }
      break label73;
      throw ((Throwable)localObject1);
    }
    catch (Exception localException)
    {
      localException = localException;MonitorRequestsInstrumenter.getRequestMonitor().recordException(localException);throw localException;
    }
    finally
    {
      localObject3 = finally;localObject2 = null;MonitorRequestsInstrumenter.getRequestMonitor().monitorStop();throw ((Throwable)localObject3);
    }
    label73:
    Object localObject2 = null;
    MonitorRequestsInstrumenter.getRequestMonitor().monitorStop();
  }
Contributor

felixbarny commented Apr 10, 2016

Thx for the hint, I've now also exported the class when the Byte Buddy transformer was last:
classes.zip

Interestingly, the monitorStart call via insertBefore does get executed but the monitorStop call added via insertAfter does not.

But all calls seem to be included in the resulting bytecode. This is an excerpt of the decompiled method (via jd):

javassist-first (not working)

@SLAs({@SLA(metric={SLA.Metric.P95, SLA.Metric.MAX}, threshold={0.0D, 0.0D}), @SLA(errorRateThreshold=0.0D)})
  @MonitorRequests
  void monitorSla()
  {
    Profiler.start("org.stagemonitor.alerting.annotation.SlaInstrumenterTest$SlaTestClass.monitorSla()V");
    try
    {
      try
      {
        MonitorRequestsInstrumenter.getRequestMonitor().monitorStart(new MonitoredMethodRequest("Monitor Sla", null, new Object[0]));throw null;
      }
      catch (Exception localException)
      {
        localException = localException;MonitorRequestsInstrumenter.getRequestMonitor().recordException(localException);throw localException;
      }
      finally
      {
        localObject2 = finally;Object localObject1 = null;MonitorRequestsInstrumenter.getRequestMonitor().monitorStop();throw ((Throwable)localObject2);
      }
    }
    finally
    {
      Profiler.stop();
      if (localException == null) {
        return;
      }
    }
  }

byte-buddy-first (working)

  @SLAs({@SLA(metric={SLA.Metric.P95, SLA.Metric.MAX}, threshold={0.0D, 0.0D}), @SLA(errorRateThreshold=0.0D)})
  @MonitorRequests
  void monitorSla()
  {
    try
    {
      MonitorRequestsInstrumenter.getRequestMonitor().monitorStart(new MonitoredMethodRequest("Monitor Sla", null, new Object[0]));Profiler.start("org.stagemonitor.alerting.annotation.SlaInstrumenterTest$SlaTestClass.monitorSla()V");
      try
      {
        throw null;
      }
      finally
      {
        Profiler.stop();
      }
      break label73;
      throw ((Throwable)localObject1);
    }
    catch (Exception localException)
    {
      localException = localException;MonitorRequestsInstrumenter.getRequestMonitor().recordException(localException);throw localException;
    }
    finally
    {
      localObject3 = finally;localObject2 = null;MonitorRequestsInstrumenter.getRequestMonitor().monitorStop();throw ((Throwable)localObject3);
    }
    label73:
    Object localObject2 = null;
    MonitorRequestsInstrumenter.getRequestMonitor().monitorStop();
  }
@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 10, 2016

Owner

I think I understand the problem now. I do not register the exception handler in the correct order. In case of an exception, Byte Buddy steals any exception from a nested try-catch block and the inner exception handler is never triggered. This is of course not supposed to happen and can cause significant problems. Good catch!

Owner

raphw commented Apr 10, 2016

I think I understand the problem now. I do not register the exception handler in the correct order. In case of an exception, Byte Buddy steals any exception from a nested try-catch block and the inner exception handler is never triggered. This is of course not supposed to happen and can cause significant problems. Good catch!

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 10, 2016

Contributor

nice! glad i could help!

Contributor

felixbarny commented Apr 10, 2016

nice! glad i could help!

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 12, 2016

Owner

All fixed now with Byte Buddy 1.3.11. The agent order does no longer matter and the new annotations are introduced as well as a mechanism for adding custom annotations with runtime-dependant constant values was added. I also extended the @Advice.Origin annotation a bit but if this does not suffice, you can build a string programmatically by using the Advice.withCustomMapping() API.

The new version is currently uploaded to JCenter and should synchronize with Maven Central until the end of the day.

Owner

raphw commented Apr 12, 2016

All fixed now with Byte Buddy 1.3.11. The agent order does no longer matter and the new annotations are introduced as well as a mechanism for adding custom annotations with runtime-dependant constant values was added. I also extended the @Advice.Origin annotation a bit but if this does not suffice, you can build a string programmatically by using the Advice.withCustomMapping() API.

The new version is currently uploaded to JCenter and should synchronize with Maven Central until the end of the day.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 12, 2016

Contributor

Thx, you are awesome!

Contributor

felixbarny commented Apr 12, 2016

Thx, you are awesome!

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 12, 2016

Contributor

The withCustomMapping() API is working great! What did you extend in the @Advice.Origin annotation? More patterns? Could not find it in the docs.

I also have encountered a VerifyError when the @Advice.OnMethodExit is more complicated. When the annotation is placed here instead of there, I get a verify error when executing the tests.

Contributor

felixbarny commented Apr 12, 2016

The withCustomMapping() API is working great! What did you extend in the @Advice.Origin annotation? More patterns? Could not find it in the docs.

I also have encountered a VerifyError when the @Advice.OnMethodExit is more complicated. When the annotation is placed here instead of there, I get a verify error when executing the tests.

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 12, 2016

Owner

Hi, thanks for the report. It seems as if there are still some issues with the Advice component, sigh. I try to have it fixed as quickly as possible. Also, I indeed forgot the docs. I added #s for the signature and #r for the return type.

Owner

raphw commented Apr 12, 2016

Hi, thanks for the report. It seems as if there are still some issues with the Advice component, sigh. I try to have it fixed as quickly as possible. Also, I indeed forgot the docs. I added #s for the signature and #r for the return type.

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 12, 2016

Owner

The problems you found showcased another problem with the computation of remapped offsets in the advice. Sorry for these childhood diseases, the component is six weeks old and testing stack map frames is not trivial.

Please let me know if anything else blows up and I will fix it. All that you reported in is fixed in 1.3.12 that is already synchronized to Maven Central and JCenter.

Owner

raphw commented Apr 12, 2016

The problems you found showcased another problem with the computation of remapped offsets in the advice. Sorry for these childhood diseases, the component is six weeks old and testing stack map frames is not trivial.

Please let me know if anything else blows up and I will fix it. All that you reported in is fixed in 1.3.12 that is already synchronized to Maven Central and JCenter.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 13, 2016

Contributor

Do you think it would be better to recompute stack frames in production for now? Would this avoid such problems? I could even make this configurable.

It is astonishing how fast you are in fixing bugs and implementing features! Kudos! Are you working full time on Byte Buddy?

Contributor

felixbarny commented Apr 13, 2016

Do you think it would be better to recompute stack frames in production for now? Would this avoid such problems? I could even make this configurable.

It is astonishing how fast you are in fixing bugs and implementing features! Kudos! Are you working full time on Byte Buddy?

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 13, 2016

Owner

I am currently contracting for a firm who needed such a component and they were generous enough to let me open-source. All this gave me the time to bootstrap Byte Buddy Advice. Ever since, the component is a part element of my current contract. The library itself is however a free-time project. Not that I did not like to get payed to work on it from time to time, I would love to add much more documentation, for example.

The bugs you encountered were not big issues from an implementation perspective and were therefore fixable in short time. The last bug was basically a one-off error when remapping exception tables what is much more difficult than it should be because of the order in which ASM processes meta data. As a matter of fact, the errors always seem much worse than they are. The JVM is (for a good reason) very resolute even about small inconsistencies in a class file. Java is meant to be a safe platform; anything slightly ambiguous fails the class loading even if it would not affect the execution result.

You can set an option for ASM to recompute the frames from scratch but be aware that the frame computation is much heavier than Byte Buddy's remapping. For it, ASM needs to parse all class files that are super types of any class that are referenced within a method what causes a lot of I/O. One of the major reasons for avoiding the recomputation is also that ASM sometimes fails to compute correct frames; the remapping is another way to cover corner cases where ASM cannot help out. If you implement a switch, you might however be able to offer a quick fix for some people, so you might just make it an option but I would not recommend making it the default as this option can also break complex code. (Btw, Javassist is also known to struggle sometimes, especially when instrumenting dynamic code such as Clojure or Javasscript where the languages need to map everything to static types in their byte code what sometimes results in ridiculous combinations.) None of this is common, but bug reports still pop up regularly.

The idea of remapping frames means that we use stack map frames that were already inserted by the origin-compiler which has more information about the original program as it knows the source code. For example, javac knows control flow such as if-else or while whereas all of this is reduced to goto statements in byte code. When recomputing stack map frames from byte code, you face an inherently more difficult task than javac as you need to compute a common set of types that is available at any statement that can be possibly reached from a goto. The control-flow is less structured than in most high-level programs, so to speak. If these gotos are nested, this can become quite intense and in the worst case, the approximation of these computed stack map frames does not match the verifier's expectations even with ASM. This is quite frustrating if you do not have another choice as Javassist or generic ASM does not but I found this loop-hole to make Byte Buddy work with existing frames that were inserted by the root compiler as long as the advice method is of byte code level six or higher. In this case, if the remapping is working correctly, I can make only small changes to already existing stack map frames what allows me to do the computation in a single pass-through of the byte code while not suffering the same problems when instrumenting less deterministic languages such as Clojure. But as it has not been done before I experience a few bumps on the road with byte code information that is non-sequential to the instruction set (such as exception tables). All this just as a quick overview to the problems you experienced and what trade-off to consider when offering to activate the stack map computation by ASM. I really hope this is the last version I need to deploy to fix the Advice component, though.

You should also know that all of the previous problems were caused by the byte code generation engine which then fed wrong stack map information to the meta data processor. ASM "fixed" this problem by voiding certain stack map indices in its recomputation which worked for some (common) scenarios where the voided index was not accessed at a later point within the advice method. Making ASM recompute the frames did however not offer a universal solution as there was still an inconsistency in the generated byte code / exception table. In the end, I will always rather fight the cause, not the symptoms but I understand that a quick-fix might be desirable for a production environment.

Owner

raphw commented Apr 13, 2016

I am currently contracting for a firm who needed such a component and they were generous enough to let me open-source. All this gave me the time to bootstrap Byte Buddy Advice. Ever since, the component is a part element of my current contract. The library itself is however a free-time project. Not that I did not like to get payed to work on it from time to time, I would love to add much more documentation, for example.

The bugs you encountered were not big issues from an implementation perspective and were therefore fixable in short time. The last bug was basically a one-off error when remapping exception tables what is much more difficult than it should be because of the order in which ASM processes meta data. As a matter of fact, the errors always seem much worse than they are. The JVM is (for a good reason) very resolute even about small inconsistencies in a class file. Java is meant to be a safe platform; anything slightly ambiguous fails the class loading even if it would not affect the execution result.

You can set an option for ASM to recompute the frames from scratch but be aware that the frame computation is much heavier than Byte Buddy's remapping. For it, ASM needs to parse all class files that are super types of any class that are referenced within a method what causes a lot of I/O. One of the major reasons for avoiding the recomputation is also that ASM sometimes fails to compute correct frames; the remapping is another way to cover corner cases where ASM cannot help out. If you implement a switch, you might however be able to offer a quick fix for some people, so you might just make it an option but I would not recommend making it the default as this option can also break complex code. (Btw, Javassist is also known to struggle sometimes, especially when instrumenting dynamic code such as Clojure or Javasscript where the languages need to map everything to static types in their byte code what sometimes results in ridiculous combinations.) None of this is common, but bug reports still pop up regularly.

The idea of remapping frames means that we use stack map frames that were already inserted by the origin-compiler which has more information about the original program as it knows the source code. For example, javac knows control flow such as if-else or while whereas all of this is reduced to goto statements in byte code. When recomputing stack map frames from byte code, you face an inherently more difficult task than javac as you need to compute a common set of types that is available at any statement that can be possibly reached from a goto. The control-flow is less structured than in most high-level programs, so to speak. If these gotos are nested, this can become quite intense and in the worst case, the approximation of these computed stack map frames does not match the verifier's expectations even with ASM. This is quite frustrating if you do not have another choice as Javassist or generic ASM does not but I found this loop-hole to make Byte Buddy work with existing frames that were inserted by the root compiler as long as the advice method is of byte code level six or higher. In this case, if the remapping is working correctly, I can make only small changes to already existing stack map frames what allows me to do the computation in a single pass-through of the byte code while not suffering the same problems when instrumenting less deterministic languages such as Clojure. But as it has not been done before I experience a few bumps on the road with byte code information that is non-sequential to the instruction set (such as exception tables). All this just as a quick overview to the problems you experienced and what trade-off to consider when offering to activate the stack map computation by ASM. I really hope this is the last version I need to deploy to fix the Advice component, though.

You should also know that all of the previous problems were caused by the byte code generation engine which then fed wrong stack map information to the meta data processor. ASM "fixed" this problem by voiding certain stack map indices in its recomputation which worked for some (common) scenarios where the voided index was not accessed at a later point within the advice method. Making ASM recompute the frames did however not offer a universal solution as there was still an inconsistency in the generated byte code / exception table. In the end, I will always rather fight the cause, not the symptoms but I understand that a quick-fix might be desirable for a production environment.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 13, 2016

Contributor

Thx for the tips.

I've just noticed that I can't debug classes that are transformed. Even with previous versions of Byte Buddy. Is this some sort of limitation or am I doing something terribly wrong?

Contributor

felixbarny commented Apr 13, 2016

Thx for the tips.

I've just noticed that I can't debug classes that are transformed. Even with previous versions of Byte Buddy. Is this some sort of limitation or am I doing something terribly wrong?

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 13, 2016

Owner

You can not put a break point into the advice method, that is true. You can however set break points in the original method even after it was instrumented. The original debugging information is kept intact by Byte Buddy. Triggering breakpoints from the inlined advice method is not possible on the JVM as any class file can only maintain a single reference to any source file. Within a method itself, only line numbers are set as debugging information and no separate source file can be referenced. Therefore, mixed origin sources are not possible.

You can however call your advice methods directly from a unit test where setting break points is possible.

Owner

raphw commented Apr 13, 2016

You can not put a break point into the advice method, that is true. You can however set break points in the original method even after it was instrumented. The original debugging information is kept intact by Byte Buddy. Triggering breakpoints from the inlined advice method is not possible on the JVM as any class file can only maintain a single reference to any source file. Within a method itself, only line numbers are set as debugging information and no separate source file can be referenced. Therefore, mixed origin sources are not possible.

You can however call your advice methods directly from a unit test where setting break points is possible.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 13, 2016

Contributor

The problem is that I can't debug the "client code" I'm instrumenting which is quite scary. For example when I'm instrumenting a Elasticsearch class, I'm not able to debug it anymore. Maybe this is an issue of IntelliJ 2016.1.0... I'll try it with a different IDE.

Contributor

felixbarny commented Apr 13, 2016

The problem is that I can't debug the "client code" I'm instrumenting which is quite scary. For example when I'm instrumenting a Elasticsearch class, I'm not able to debug it anymore. Maybe this is an issue of IntelliJ 2016.1.0... I'll try it with a different IDE.

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 13, 2016

Owner

That is indeed strange. IntelliJ has added some functionality to discover mismatches of byte code and source code in 2016.1 to discover debugging the wrong source in case of a version mismatch. Maybe that changed something (and maybe you can turn it off)? Debugging client code works fine on 15.0.5. I rolled back to that version myself after my current work project did not play together well with 2016.

Owner

raphw commented Apr 13, 2016

That is indeed strange. IntelliJ has added some functionality to discover mismatches of byte code and source code in 2016.1 to discover debugging the wrong source in case of a version mismatch. Maybe that changed something (and maybe you can turn it off)? Debugging client code works fine on 15.0.5. I rolled back to that version myself after my current work project did not play together well with 2016.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 13, 2016

Contributor

I have a question on how to match a method in a tricky case.

I want to instrument the javax.sql.DataSource.getConnection() method. Sounds easy, but if this method is declared in a superclass which does not implement the DataSource interface, things get tricky.

private class AbstractTestDataSource {
    Connection connection;

    public Connection getConnection() throws SQLException {
        return connection;
    }
}

private class TestDataSource extends AbstractTestDataSource implements DataSource {

}

My first naive attempt was to use this type matcher

nameEndsWith("DataSource")
        .and(isSubTypeOf(DataSource.class))
        .and(not(isInterface()))

and this method matcher:

named("getConnection")
        .and(returns(Connection.class))
        .and(isPublic())
        .and(takesArguments(String.class, String.class).or(takesArguments(0)))

But this obviously fails, because the javax.sql.DataSource.getConnection() method is not declared inside TestDataSource. Can you think of a clever way to match this? Is this a matter of AgentBuilder.TypeStrategy.Default.REDEFINE vs REDEFINE_DECLARED_ONLY?

Note that I also want to inject @Advice.This DataSource into the @Advice.OnMethodExit like this:

@Advice.OnMethodExit
private static void addDirectMonitorMethodCall(@Advice.This DataSource dataSource, 
                                                @Advice.Return(readOnly = false) Connection connection, 
                                                @Advice.Enter long startTime) {
    connection = monitorGetConnection(dataSource, connection, startTime);
}
Contributor

felixbarny commented Apr 13, 2016

I have a question on how to match a method in a tricky case.

I want to instrument the javax.sql.DataSource.getConnection() method. Sounds easy, but if this method is declared in a superclass which does not implement the DataSource interface, things get tricky.

private class AbstractTestDataSource {
    Connection connection;

    public Connection getConnection() throws SQLException {
        return connection;
    }
}

private class TestDataSource extends AbstractTestDataSource implements DataSource {

}

My first naive attempt was to use this type matcher

nameEndsWith("DataSource")
        .and(isSubTypeOf(DataSource.class))
        .and(not(isInterface()))

and this method matcher:

named("getConnection")
        .and(returns(Connection.class))
        .and(isPublic())
        .and(takesArguments(String.class, String.class).or(takesArguments(0)))

But this obviously fails, because the javax.sql.DataSource.getConnection() method is not declared inside TestDataSource. Can you think of a clever way to match this? Is this a matter of AgentBuilder.TypeStrategy.Default.REDEFINE vs REDEFINE_DECLARED_ONLY?

Note that I also want to inject @Advice.This DataSource into the @Advice.OnMethodExit like this:

@Advice.OnMethodExit
private static void addDirectMonitorMethodCall(@Advice.This DataSource dataSource, 
                                                @Advice.Return(readOnly = false) Connection connection, 
                                                @Advice.Enter long startTime) {
    connection = monitorGetConnection(dataSource, connection, startTime);
}
@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 13, 2016

Owner

Since you are applying runtime attachment, you want to use REDEFINE_DECLARED_ONLY (which the Advice enforces implicitly anyways) as you must not change the class file layout what is not supported by the JVM.

The easiest would be to simply drop the requirement isSubTypeOf(DataSource.class) and define the advice as @Advice.This Object dataSource instead where you add a condition:

if (!(dataSource instanceOf DataSource)) { 
  return; 
}

in the beginning of the method. This is the only way for Byte Buddy to guarantee that the advised method can always fulfill the contract of the advice method. As the AbstractTestDataSource is the only type that declares the method, doing the type check upon the method call is the only way of doing it anyways, even if Byte Buddy did the check for you but I think making it explicit makes this more intuitive. Byte Buddy can not virtualize the call for as this would imply changing the class layout for subtypes. If any implementation of AbstractTestDataSource is an instance of DataSource, the JVM will remove the check anyways at runtime.

Owner

raphw commented Apr 13, 2016

Since you are applying runtime attachment, you want to use REDEFINE_DECLARED_ONLY (which the Advice enforces implicitly anyways) as you must not change the class file layout what is not supported by the JVM.

The easiest would be to simply drop the requirement isSubTypeOf(DataSource.class) and define the advice as @Advice.This Object dataSource instead where you add a condition:

if (!(dataSource instanceOf DataSource)) { 
  return; 
}

in the beginning of the method. This is the only way for Byte Buddy to guarantee that the advised method can always fulfill the contract of the advice method. As the AbstractTestDataSource is the only type that declares the method, doing the type check upon the method call is the only way of doing it anyways, even if Byte Buddy did the check for you but I think making it explicit makes this more intuitive. Byte Buddy can not virtualize the call for as this would imply changing the class layout for subtypes. If any implementation of AbstractTestDataSource is an instance of DataSource, the JVM will remove the check anyways at runtime.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 13, 2016

Contributor

Aah, of course. I see what you did there ;)
At runtime the @Advice.This Object dataSource is TestDataSource which implements DataSource and not AbstractTestDataSource.
Thx for your great support. Asking you is much easier than thinking myself 😆

Contributor

felixbarny commented Apr 13, 2016

Aah, of course. I see what you did there ;)
At runtime the @Advice.This Object dataSource is TestDataSource which implements DataSource and not AbstractTestDataSource.
Thx for your great support. Asking you is much easier than thinking myself 😆

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 19, 2016

Owner

The problem with the weak concurrent map is that it would require the class loaders to be collected for being effective. Most applications load by some class loader(s) and at some point in life almost all classes are loaded where the transformer is no longer active. The class loaders are however still strongly referenced by the application such that nothing could be collected from the cached type pools.

In order to make this meaningful, one needs to know something about the life-cycle of the instrumentation procedure. Most users do not have such information and if you attach an agent at startup, class loading might just be distributed over the first few hours of the app's life time. I gave this a lot of thought already but so far, I did not really come up with something good, in the end you probably need a dedicated cache to take that job and that is out of scope for Byte Buddy.

Owner

raphw commented Apr 19, 2016

The problem with the weak concurrent map is that it would require the class loaders to be collected for being effective. Most applications load by some class loader(s) and at some point in life almost all classes are loaded where the transformer is no longer active. The class loaders are however still strongly referenced by the application such that nothing could be collected from the cached type pools.

In order to make this meaningful, one needs to know something about the life-cycle of the instrumentation procedure. Most users do not have such information and if you attach an agent at startup, class loading might just be distributed over the first few hours of the app's life time. I gave this a lot of thought already but so far, I did not really come up with something good, in the end you probably need a dedicated cache to take that job and that is out of scope for Byte Buddy.

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 19, 2016

Owner

I have now added a simple implementation based on a ConcurrentMap. I think, ideally one would use a different dependency for something like this such as expiring map or Caffeine which are dedicated to this task.

Owner

raphw commented Apr 19, 2016

I have now added a simple implementation based on a ConcurrentMap. I think, ideally one would use a different dependency for something like this such as expiring map or Caffeine which are dedicated to this task.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 19, 2016

Contributor

Is there a way to unset the cacheProviders map after the startup of the application? That would speed up the startup in most cases and limit the scope of the potential memory leak. I'm not feeling quite comfortable putting class loaders as a key into a map and keep them there. Even if I'm calling clear, there could still be classes loaded after the startup which would populate the cache again.

Contributor

felixbarny commented Apr 19, 2016

Is there a way to unset the cacheProviders map after the startup of the application? That would speed up the startup in most cases and limit the scope of the potential memory leak. I'm not feeling quite comfortable putting class loaders as a key into a map and keep them there. Even if I'm calling clear, there could still be classes loaded after the startup which would populate the cache again.

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 19, 2016

Owner

You could implement your own BinaryLocator for this. Simply add an AtomicBoolean switch that always returns a fresh CacheProvider.Simple once it was turned and clears the concurrent map you used for caching upon changing the mode. You can use the skeleton for such an implementation.

Owner

raphw commented Apr 19, 2016

You could implement your own BinaryLocator for this. Simply add an AtomicBoolean switch that always returns a fresh CacheProvider.Simple once it was turned and clears the concurrent map you used for caching upon changing the mode. You can use the skeleton for such an implementation.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 19, 2016

Contributor

Using Simple I get a lot of NPEs:

10:25:47.537 [main] WARN org.stagemonitor.core.instrument.ErrorLoggingListener - ERROR on transformation java.util.concurrent.ConcurrentHashMap$MapEntry
java.lang.NullPointerException: null
    at java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936)
    at net.bytebuddy.agent.builder.AgentBuilder$BinaryLocator$WithTypePoolCache$Simple.locate(AgentBuilder.java:1082)
    at net.bytebuddy.agent.builder.AgentBuilder$BinaryLocator$WithTypePoolCache.typePool(AgentBuilder.java:1034)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.transform(AgentBuilder.java:5045)
    at sun.instrument.TransformerManager.transform(TransformerManager.java:188)
    at sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:428)
    at java.util.concurrent.ConcurrentHashMap$EntryIterator.next(ConcurrentHashMap.java:3461)
    at java.util.concurrent.ConcurrentHashMap$EntryIterator.next(ConcurrentHashMap.java:3446)
    at org.stagemonitor.core.metrics.metrics2.Metric2Registry.getMetrics(Metric2Registry.java:297)
    at org.stagemonitor.core.metrics.metrics2.Metric2Registry.getGauges(Metric2Registry.java:195)
    at org.stagemonitor.core.metrics.metrics2.ScheduledMetrics2Reporter.report(ScheduledMetrics2Reporter.java:46)
    at org.stagemonitor.core.instrument.TimedElementMatcherDecorator.logMetrics(TimedElementMatcherDecorator.java:59)
    at org.stagemonitor.core.instrument.AgentAttacher.performRuntimeAttachment(AgentAttacher.java:85)
    at org.stagemonitor.core.Stagemonitor.reset(Stagemonitor.java:249)
    at org.stagemonitor.core.Stagemonitor.<clinit>(Stagemonitor.java:40)
    at org.stagemonitor.requestmonitor.prof.ProfilerTest.attachProfiler(ProfilerTest.java:16)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:24)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:119)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:42)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:234)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:74)

Maybe because the reference for the bootstrap class loader is represented as null? I'm a bit confused why Byte Buddy seems to transform java.util.concurrent.ConcurrentHashMap$MapEntry as I excluded the java.* namespace. But probably Byte Buddy still needs to parse the class file if it parses a class which has a field of type ConcurrentHashMap?

Regarding the onThrowable change in @Advice.OnMethodExit: The @Advice.Thrown annotation still seems to require a Throwable type. I was expecting that the type can be of the same type as the onThrowable type (or a super type).

java.lang.IllegalStateException: Parameter must be a throwable type for java.lang.Exception arg2
    at net.bytebuddy.asm.Advice$Dispatcher$OffsetMapping$ForThrowable$Factory.make(Advice.java:4457)
    at net.bytebuddy.asm.Advice$Dispatcher$Delegating$Resolved.<init>(Advice.java:6195)
    at net.bytebuddy.asm.Advice$Dispatcher$Delegating$Resolved$ForMethodExit.<init>(Advice.java:6582)
    at net.bytebuddy.asm.Advice$Dispatcher$Delegating$Resolved$ForMethodExit$WithExceptionHandler.<init>(Advice.java:6674)
    at net.bytebuddy.asm.Advice$Dispatcher$Delegating$Resolved$ForMethodExit.of(Advice.java:6614)
    at net.bytebuddy.asm.Advice$Dispatcher$Delegating.asMethodExitTo(Advice.java:6137)
    at net.bytebuddy.asm.Advice.to(Advice.java:231)
    at net.bytebuddy.asm.Advice$WithCustomMapping.to(Advice.java:7284)
    at net.bytebuddy.asm.Advice$WithCustomMapping.to(Advice.java:7269)
    at net.bytebuddy.asm.Advice$WithCustomMapping.to(Advice.java:7258)
    at org.stagemonitor.core.instrument.StagemonitorByteBuddyTransformer$1.transform(StagemonitorByteBuddyTransformer.java:97)
    at net.bytebuddy.agent.builder.AgentBuilder$Transformer$Compound.transform(AgentBuilder.java:866)
    at net.bytebuddy.agent.builder.AgentBuilder$Transformer$Compound.transform(AgentBuilder.java:866)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$Transformation$Simple$Resolution.apply(AgentBuilder.java:4746)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.transform(AgentBuilder.java:5050)
    at sun.instrument.TransformerManager.transform(TransformerManager.java:188)
    at sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:428)
    at sun.instrument.InstrumentationImpl.retransformClasses0(Native Method)
    at sun.instrument.InstrumentationImpl.retransformClasses(InstrumentationImpl.java:144)
    at net.bytebuddy.agent.builder.AgentBuilder$RedefinitionStrategy$Collector$ForRetransformation.apply(AgentBuilder.java:2373)
    at net.bytebuddy.agent.builder.AgentBuilder$Default.installOn(AgentBuilder.java:4178)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$Delegator.installOn(AgentBuilder.java:5231)
    at org.stagemonitor.core.instrument.AgentAttacher.initByteBuddyClassFileTransformer(AgentAttacher.java:166)
    at org.stagemonitor.core.instrument.AgentAttacher.performRuntimeAttachment(AgentAttacher.java:81)
    at org.stagemonitor.core.Stagemonitor.reset(Stagemonitor.java:249)
    at org.stagemonitor.core.Stagemonitor.<clinit>(Stagemonitor.java:40)
    at org.stagemonitor.requestmonitor.prof.ProfilerTest.attachProfiler(ProfilerTest.java:17)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:24)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:119)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:42)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:234)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:74)
Contributor

felixbarny commented Apr 19, 2016

Using Simple I get a lot of NPEs:

10:25:47.537 [main] WARN org.stagemonitor.core.instrument.ErrorLoggingListener - ERROR on transformation java.util.concurrent.ConcurrentHashMap$MapEntry
java.lang.NullPointerException: null
    at java.util.concurrent.ConcurrentHashMap.get(ConcurrentHashMap.java:936)
    at net.bytebuddy.agent.builder.AgentBuilder$BinaryLocator$WithTypePoolCache$Simple.locate(AgentBuilder.java:1082)
    at net.bytebuddy.agent.builder.AgentBuilder$BinaryLocator$WithTypePoolCache.typePool(AgentBuilder.java:1034)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.transform(AgentBuilder.java:5045)
    at sun.instrument.TransformerManager.transform(TransformerManager.java:188)
    at sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:428)
    at java.util.concurrent.ConcurrentHashMap$EntryIterator.next(ConcurrentHashMap.java:3461)
    at java.util.concurrent.ConcurrentHashMap$EntryIterator.next(ConcurrentHashMap.java:3446)
    at org.stagemonitor.core.metrics.metrics2.Metric2Registry.getMetrics(Metric2Registry.java:297)
    at org.stagemonitor.core.metrics.metrics2.Metric2Registry.getGauges(Metric2Registry.java:195)
    at org.stagemonitor.core.metrics.metrics2.ScheduledMetrics2Reporter.report(ScheduledMetrics2Reporter.java:46)
    at org.stagemonitor.core.instrument.TimedElementMatcherDecorator.logMetrics(TimedElementMatcherDecorator.java:59)
    at org.stagemonitor.core.instrument.AgentAttacher.performRuntimeAttachment(AgentAttacher.java:85)
    at org.stagemonitor.core.Stagemonitor.reset(Stagemonitor.java:249)
    at org.stagemonitor.core.Stagemonitor.<clinit>(Stagemonitor.java:40)
    at org.stagemonitor.requestmonitor.prof.ProfilerTest.attachProfiler(ProfilerTest.java:16)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:24)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:119)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:42)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:234)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:74)

Maybe because the reference for the bootstrap class loader is represented as null? I'm a bit confused why Byte Buddy seems to transform java.util.concurrent.ConcurrentHashMap$MapEntry as I excluded the java.* namespace. But probably Byte Buddy still needs to parse the class file if it parses a class which has a field of type ConcurrentHashMap?

Regarding the onThrowable change in @Advice.OnMethodExit: The @Advice.Thrown annotation still seems to require a Throwable type. I was expecting that the type can be of the same type as the onThrowable type (or a super type).

java.lang.IllegalStateException: Parameter must be a throwable type for java.lang.Exception arg2
    at net.bytebuddy.asm.Advice$Dispatcher$OffsetMapping$ForThrowable$Factory.make(Advice.java:4457)
    at net.bytebuddy.asm.Advice$Dispatcher$Delegating$Resolved.<init>(Advice.java:6195)
    at net.bytebuddy.asm.Advice$Dispatcher$Delegating$Resolved$ForMethodExit.<init>(Advice.java:6582)
    at net.bytebuddy.asm.Advice$Dispatcher$Delegating$Resolved$ForMethodExit$WithExceptionHandler.<init>(Advice.java:6674)
    at net.bytebuddy.asm.Advice$Dispatcher$Delegating$Resolved$ForMethodExit.of(Advice.java:6614)
    at net.bytebuddy.asm.Advice$Dispatcher$Delegating.asMethodExitTo(Advice.java:6137)
    at net.bytebuddy.asm.Advice.to(Advice.java:231)
    at net.bytebuddy.asm.Advice$WithCustomMapping.to(Advice.java:7284)
    at net.bytebuddy.asm.Advice$WithCustomMapping.to(Advice.java:7269)
    at net.bytebuddy.asm.Advice$WithCustomMapping.to(Advice.java:7258)
    at org.stagemonitor.core.instrument.StagemonitorByteBuddyTransformer$1.transform(StagemonitorByteBuddyTransformer.java:97)
    at net.bytebuddy.agent.builder.AgentBuilder$Transformer$Compound.transform(AgentBuilder.java:866)
    at net.bytebuddy.agent.builder.AgentBuilder$Transformer$Compound.transform(AgentBuilder.java:866)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$Transformation$Simple$Resolution.apply(AgentBuilder.java:4746)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.transform(AgentBuilder.java:5050)
    at sun.instrument.TransformerManager.transform(TransformerManager.java:188)
    at sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:428)
    at sun.instrument.InstrumentationImpl.retransformClasses0(Native Method)
    at sun.instrument.InstrumentationImpl.retransformClasses(InstrumentationImpl.java:144)
    at net.bytebuddy.agent.builder.AgentBuilder$RedefinitionStrategy$Collector$ForRetransformation.apply(AgentBuilder.java:2373)
    at net.bytebuddy.agent.builder.AgentBuilder$Default.installOn(AgentBuilder.java:4178)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$Delegator.installOn(AgentBuilder.java:5231)
    at org.stagemonitor.core.instrument.AgentAttacher.initByteBuddyClassFileTransformer(AgentAttacher.java:166)
    at org.stagemonitor.core.instrument.AgentAttacher.performRuntimeAttachment(AgentAttacher.java:81)
    at org.stagemonitor.core.Stagemonitor.reset(Stagemonitor.java:249)
    at org.stagemonitor.core.Stagemonitor.<clinit>(Stagemonitor.java:40)
    at org.stagemonitor.requestmonitor.prof.ProfilerTest.attachProfiler(ProfilerTest.java:17)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:24)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:119)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:42)
    at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:234)
    at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:74)
@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 19, 2016

Owner

You are right, the bootstrap class loader which is null is not allowed in a concurrent hash map. Look at the current head where I fixed it.

Owner

raphw commented Apr 19, 2016

You are right, the bootstrap class loader which is null is not allowed in a concurrent hash map. Look at the current head where I fixed it.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 19, 2016

Contributor

With the latest version I'm getting strange IncompatibleClassChangeErrors. While installing Byte Buddy I made sure everything is mvn clean. In my unit tests and with Spring PetClinic everything is working fine, but when I use it with a project of one of my customers, those exceptions suddenly pop up and the transformation fails. This project is started with gradle and an embedded Tomcat 7 server if that matters. It happens with and without a BinaryLocator set. The performance with a caching BinaryLocator looks promising in combination with Spring PetClinic.

ERROR on transformation org.elasticsearch.action.search.SearchRequestBuilder
java.lang.IncompatibleClassChangeError: class net.bytebuddy.dynamic.scaffold.TypeWriter$Default$ValidatingClassVisitor has interface org.objectweb.asm.ClassVisitor as super class
    at java.lang.ClassLoader.defineClass1(Native Method)
    at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
    at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
    at java.net.URLClassLoader.defineClass(URLClassLoader.java:467)
    at java.net.URLClassLoader.access$100(URLClassLoader.java:73)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:368)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:362)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.net.URLClassLoader.findClass(URLClassLoader.java:361)
    at org.apache.catalina.loader.WebappClassLoader.findClass(WebappClassLoader.java:1227)
    at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1690)
    at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1571)
    at net.bytebuddy.dynamic.scaffold.TypeWriter$Default$ForInlining.doCreate(TypeWriter.java:2812)
    at net.bytebuddy.dynamic.scaffold.TypeWriter$Default$ForInlining.create(TypeWriter.java:2794)
    at net.bytebuddy.dynamic.scaffold.TypeWriter$Default.make(TypeWriter.java:1572)
    at net.bytebuddy.dynamic.scaffold.inline.RedefinitionDynamicTypeBuilder.make(RedefinitionDynamicTypeBuilder.java:179)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$Transformation$Simple$Resolution.apply(AgentBuilder.java:4788)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.transform(AgentBuilder.java:5089)
    at sun.instrument.TransformerManager.transform(TransformerManager.java:188)
    at sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:428)
    at sun.instrument.InstrumentationImpl.retransformClasses0(Native Method)
    at sun.instrument.InstrumentationImpl.retransformClasses(InstrumentationImpl.java:144)
    at net.bytebuddy.agent.builder.AgentBuilder$RedefinitionStrategy$Collector$ForRetransformation.apply(AgentBuilder.java:2412)
    at net.bytebuddy.agent.builder.AgentBuilder$Default.installOn(AgentBuilder.java:4217)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$Delegator.installOn(AgentBuilder.java:5270)
    at org.stagemonitor.core.instrument.AgentAttacher.initByteBuddyClassFileTransformer(AgentAttacher.java:165)
    at org.stagemonitor.core.instrument.AgentAttacher.performRuntimeAttachment(AgentAttacher.java:79)
    at org.stagemonitor.core.Stagemonitor.reset(Stagemonitor.java:249)
    at org.stagemonitor.core.Stagemonitor.<clinit>(Stagemonitor.java:40)
    at org.stagemonitor.web.WebPlugin.<clinit>(WebPlugin.java:57)
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:348)
    at org.apache.catalina.startup.WebappServiceLoader.loadServices(WebappServiceLoader.java:187)
    at org.apache.catalina.startup.WebappServiceLoader.load(WebappServiceLoader.java:152)
    at org.apache.catalina.startup.ContextConfig.processServletContainerInitializers(ContextConfig.java:1559)
    at org.apache.catalina.startup.ContextConfig.webConfig(ContextConfig.java:1281)
    at org.apache.catalina.startup.ContextConfig.configureStart(ContextConfig.java:889)
    at org.apache.catalina.startup.ContextConfig.lifecycleEvent(ContextConfig.java:386)
    at org.apache.catalina.util.LifecycleSupport.fireLifecycleEvent(LifecycleSupport.java:117)
    at org.apache.catalina.util.LifecycleBase.fireLifecycleEvent(LifecycleBase.java:90)
    at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5380)
    at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150)
    at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1575)
    at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1565)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)
ERROR on transformation org.webstage.shop.ui.http.SessionCollector
java.lang.IncompatibleClassChangeError: class net.bytebuddy.pool.TypePool$Default$TypeExtractor has interface org.objectweb.asm.ClassVisitor as super class
    at java.lang.ClassLoader.defineClass1(Native Method)
    at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
    at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
    at java.net.URLClassLoader.defineClass(URLClassLoader.java:467)
    at java.net.URLClassLoader.access$100(URLClassLoader.java:73)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:368)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:362)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.net.URLClassLoader.findClass(URLClassLoader.java:361)
    at org.apache.catalina.loader.WebappClassLoader.findClass(WebappClassLoader.java:1227)
    at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1690)
    at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1571)
    at net.bytebuddy.pool.TypePool$Default.parse(TypePool.java:1098)
    at net.bytebuddy.pool.TypePool$Default.doDescribe(TypePool.java:1083)
    at net.bytebuddy.pool.TypePool$Default$Precomputed.doDescribe(TypePool.java:8218)
    at net.bytebuddy.pool.TypePool$AbstractBase.describe(TypePool.java:383)
    at net.bytebuddy.pool.TypePool$AbstractBase$Hierarchical.describe(TypePool.java:440)
    at net.bytebuddy.pool.TypePool$LazyFacade$LazyResolution$LazyTypeDescription.resolve(TypePool.java:8387)
    at net.bytebuddy.pool.TypePool$LazyFacade$LazyResolution$LazyTypeDescription.getDeclaredAnnotations(TypePool.java:8452)
    at net.bytebuddy.matcher.DeclaringAnnotationMatcher.matches(DeclaringAnnotationMatcher.java:29)
    at net.bytebuddy.matcher.DeclaringAnnotationMatcher.matches(DeclaringAnnotationMatcher.java:11)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Conjunction.matches(ElementMatcher.java:98)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Conjunction.matches(ElementMatcher.java:98)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Conjunction.matches(ElementMatcher.java:98)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Conjunction.matches(ElementMatcher.java:98)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Conjunction.matches(ElementMatcher.java:98)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Conjunction.matches(ElementMatcher.java:98)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at org.stagemonitor.core.instrument.TimedElementMatcherDecorator.matches(TimedElementMatcherDecorator.java:49)
    at net.bytebuddy.agent.builder.AgentBuilder$RawMatcher$ForElementMatcherPair.matches(AgentBuilder.java:688)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$Transformation$Simple.resolve(AgentBuilder.java:4700)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$Transformation$Compound.resolve(AgentBuilder.java:4945)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.transform(AgentBuilder.java:5083)
    at sun.instrument.TransformerManager.transform(TransformerManager.java:188)
    at sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:428)
    at java.lang.ClassLoader.defineClass1(Native Method)
    at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
    at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
    at java.net.URLClassLoader.defineClass(URLClassLoader.java:467)
    at java.net.URLClassLoader.access$100(URLClassLoader.java:73)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:368)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:362)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.net.URLClassLoader.findClass(URLClassLoader.java:361)
    at org.apache.catalina.loader.WebappClassLoader.findClass(WebappClassLoader.java:1227)
    at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1690)
    at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1571)
    at org.apache.catalina.util.Introspection.loadClass(Introspection.java:143)
    at org.apache.catalina.startup.WebAnnotationSet.loadApplicationListenerAnnotations(WebAnnotationSet.java:82)
    at org.apache.catalina.startup.WebAnnotationSet.loadApplicationAnnotations(WebAnnotationSet.java:63)
    at org.apache.catalina.startup.ContextConfig.applicationAnnotationsConfig(ContextConfig.java:415)
    at org.apache.catalina.startup.ContextConfig.configureStart(ContextConfig.java:892)
    at org.apache.catalina.startup.ContextConfig.lifecycleEvent(ContextConfig.java:386)
    at org.apache.catalina.util.LifecycleSupport.fireLifecycleEvent(LifecycleSupport.java:117)
    at org.apache.catalina.util.LifecycleBase.fireLifecycleEvent(LifecycleBase.java:90)
    at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5380)
    at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150)
    at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1575)
    at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1565)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)
Contributor

felixbarny commented Apr 19, 2016

With the latest version I'm getting strange IncompatibleClassChangeErrors. While installing Byte Buddy I made sure everything is mvn clean. In my unit tests and with Spring PetClinic everything is working fine, but when I use it with a project of one of my customers, those exceptions suddenly pop up and the transformation fails. This project is started with gradle and an embedded Tomcat 7 server if that matters. It happens with and without a BinaryLocator set. The performance with a caching BinaryLocator looks promising in combination with Spring PetClinic.

ERROR on transformation org.elasticsearch.action.search.SearchRequestBuilder
java.lang.IncompatibleClassChangeError: class net.bytebuddy.dynamic.scaffold.TypeWriter$Default$ValidatingClassVisitor has interface org.objectweb.asm.ClassVisitor as super class
    at java.lang.ClassLoader.defineClass1(Native Method)
    at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
    at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
    at java.net.URLClassLoader.defineClass(URLClassLoader.java:467)
    at java.net.URLClassLoader.access$100(URLClassLoader.java:73)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:368)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:362)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.net.URLClassLoader.findClass(URLClassLoader.java:361)
    at org.apache.catalina.loader.WebappClassLoader.findClass(WebappClassLoader.java:1227)
    at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1690)
    at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1571)
    at net.bytebuddy.dynamic.scaffold.TypeWriter$Default$ForInlining.doCreate(TypeWriter.java:2812)
    at net.bytebuddy.dynamic.scaffold.TypeWriter$Default$ForInlining.create(TypeWriter.java:2794)
    at net.bytebuddy.dynamic.scaffold.TypeWriter$Default.make(TypeWriter.java:1572)
    at net.bytebuddy.dynamic.scaffold.inline.RedefinitionDynamicTypeBuilder.make(RedefinitionDynamicTypeBuilder.java:179)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$Transformation$Simple$Resolution.apply(AgentBuilder.java:4788)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.transform(AgentBuilder.java:5089)
    at sun.instrument.TransformerManager.transform(TransformerManager.java:188)
    at sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:428)
    at sun.instrument.InstrumentationImpl.retransformClasses0(Native Method)
    at sun.instrument.InstrumentationImpl.retransformClasses(InstrumentationImpl.java:144)
    at net.bytebuddy.agent.builder.AgentBuilder$RedefinitionStrategy$Collector$ForRetransformation.apply(AgentBuilder.java:2412)
    at net.bytebuddy.agent.builder.AgentBuilder$Default.installOn(AgentBuilder.java:4217)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$Delegator.installOn(AgentBuilder.java:5270)
    at org.stagemonitor.core.instrument.AgentAttacher.initByteBuddyClassFileTransformer(AgentAttacher.java:165)
    at org.stagemonitor.core.instrument.AgentAttacher.performRuntimeAttachment(AgentAttacher.java:79)
    at org.stagemonitor.core.Stagemonitor.reset(Stagemonitor.java:249)
    at org.stagemonitor.core.Stagemonitor.<clinit>(Stagemonitor.java:40)
    at org.stagemonitor.web.WebPlugin.<clinit>(WebPlugin.java:57)
    at java.lang.Class.forName0(Native Method)
    at java.lang.Class.forName(Class.java:348)
    at org.apache.catalina.startup.WebappServiceLoader.loadServices(WebappServiceLoader.java:187)
    at org.apache.catalina.startup.WebappServiceLoader.load(WebappServiceLoader.java:152)
    at org.apache.catalina.startup.ContextConfig.processServletContainerInitializers(ContextConfig.java:1559)
    at org.apache.catalina.startup.ContextConfig.webConfig(ContextConfig.java:1281)
    at org.apache.catalina.startup.ContextConfig.configureStart(ContextConfig.java:889)
    at org.apache.catalina.startup.ContextConfig.lifecycleEvent(ContextConfig.java:386)
    at org.apache.catalina.util.LifecycleSupport.fireLifecycleEvent(LifecycleSupport.java:117)
    at org.apache.catalina.util.LifecycleBase.fireLifecycleEvent(LifecycleBase.java:90)
    at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5380)
    at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150)
    at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1575)
    at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1565)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)
ERROR on transformation org.webstage.shop.ui.http.SessionCollector
java.lang.IncompatibleClassChangeError: class net.bytebuddy.pool.TypePool$Default$TypeExtractor has interface org.objectweb.asm.ClassVisitor as super class
    at java.lang.ClassLoader.defineClass1(Native Method)
    at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
    at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
    at java.net.URLClassLoader.defineClass(URLClassLoader.java:467)
    at java.net.URLClassLoader.access$100(URLClassLoader.java:73)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:368)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:362)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.net.URLClassLoader.findClass(URLClassLoader.java:361)
    at org.apache.catalina.loader.WebappClassLoader.findClass(WebappClassLoader.java:1227)
    at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1690)
    at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1571)
    at net.bytebuddy.pool.TypePool$Default.parse(TypePool.java:1098)
    at net.bytebuddy.pool.TypePool$Default.doDescribe(TypePool.java:1083)
    at net.bytebuddy.pool.TypePool$Default$Precomputed.doDescribe(TypePool.java:8218)
    at net.bytebuddy.pool.TypePool$AbstractBase.describe(TypePool.java:383)
    at net.bytebuddy.pool.TypePool$AbstractBase$Hierarchical.describe(TypePool.java:440)
    at net.bytebuddy.pool.TypePool$LazyFacade$LazyResolution$LazyTypeDescription.resolve(TypePool.java:8387)
    at net.bytebuddy.pool.TypePool$LazyFacade$LazyResolution$LazyTypeDescription.getDeclaredAnnotations(TypePool.java:8452)
    at net.bytebuddy.matcher.DeclaringAnnotationMatcher.matches(DeclaringAnnotationMatcher.java:29)
    at net.bytebuddy.matcher.DeclaringAnnotationMatcher.matches(DeclaringAnnotationMatcher.java:11)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Conjunction.matches(ElementMatcher.java:98)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Conjunction.matches(ElementMatcher.java:98)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Conjunction.matches(ElementMatcher.java:98)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Conjunction.matches(ElementMatcher.java:98)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Conjunction.matches(ElementMatcher.java:98)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Conjunction.matches(ElementMatcher.java:98)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at net.bytebuddy.matcher.ElementMatcher$Junction$Disjunction.matches(ElementMatcher.java:145)
    at org.stagemonitor.core.instrument.TimedElementMatcherDecorator.matches(TimedElementMatcherDecorator.java:49)
    at net.bytebuddy.agent.builder.AgentBuilder$RawMatcher$ForElementMatcherPair.matches(AgentBuilder.java:688)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$Transformation$Simple.resolve(AgentBuilder.java:4700)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$Transformation$Compound.resolve(AgentBuilder.java:4945)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.transform(AgentBuilder.java:5083)
    at sun.instrument.TransformerManager.transform(TransformerManager.java:188)
    at sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:428)
    at java.lang.ClassLoader.defineClass1(Native Method)
    at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
    at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
    at java.net.URLClassLoader.defineClass(URLClassLoader.java:467)
    at java.net.URLClassLoader.access$100(URLClassLoader.java:73)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:368)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:362)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.net.URLClassLoader.findClass(URLClassLoader.java:361)
    at org.apache.catalina.loader.WebappClassLoader.findClass(WebappClassLoader.java:1227)
    at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1690)
    at org.apache.catalina.loader.WebappClassLoader.loadClass(WebappClassLoader.java:1571)
    at org.apache.catalina.util.Introspection.loadClass(Introspection.java:143)
    at org.apache.catalina.startup.WebAnnotationSet.loadApplicationListenerAnnotations(WebAnnotationSet.java:82)
    at org.apache.catalina.startup.WebAnnotationSet.loadApplicationAnnotations(WebAnnotationSet.java:63)
    at org.apache.catalina.startup.ContextConfig.applicationAnnotationsConfig(ContextConfig.java:415)
    at org.apache.catalina.startup.ContextConfig.configureStart(ContextConfig.java:892)
    at org.apache.catalina.startup.ContextConfig.lifecycleEvent(ContextConfig.java:386)
    at org.apache.catalina.util.LifecycleSupport.fireLifecycleEvent(LifecycleSupport.java:117)
    at org.apache.catalina.util.LifecycleBase.fireLifecycleEvent(LifecycleBase.java:90)
    at org.apache.catalina.core.StandardContext.startInternal(StandardContext.java:5380)
    at org.apache.catalina.util.LifecycleBase.start(LifecycleBase.java:150)
    at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1575)
    at org.apache.catalina.core.ContainerBase$StartChild.call(ContainerBase.java:1565)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)
@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 19, 2016

Contributor

Probably I did something wrong when building Byte Buddy locally. I executed mvn clean source:jar install. Do I have to enable shading somehow?

Contributor

felixbarny commented Apr 19, 2016

Probably I did something wrong when building Byte Buddy locally. I executed mvn clean source:jar install. Do I have to enable shading somehow?

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 19, 2016

Contributor

Nevermind, building with mvn clean source:jar install -Pextras did the trick...

Contributor

felixbarny commented Apr 19, 2016

Nevermind, building with mvn clean source:jar install -Pextras did the trick...

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 19, 2016

Contributor

Nice! Now the overhead in startup time is virtually zero! It's within the bounds of uncertainty. Looking forward to 1.3.15 :)

Contributor

felixbarny commented Apr 19, 2016

Nice! Now the overhead in startup time is virtually zero! It's within the bounds of uncertainty. Looking forward to 1.3.15 :)

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 19, 2016

Owner

Great. That is what I originally designed Byte Buddy for, the byte code layer is really thin and I had JITWatch spinning over the entire code base to make sure there is almost no allocation left once the code becomes hot.

The error you saw is a common problem with ASM. Are you using byte-buddy-dep at some point? This one must always be shaded for the exact above reason.

I am planning to extend the AgentBuillder API some time this weekend, I will probably release afterwards.

Owner

raphw commented Apr 19, 2016

Great. That is what I originally designed Byte Buddy for, the byte code layer is really thin and I had JITWatch spinning over the entire code base to make sure there is almost no allocation left once the code becomes hot.

The error you saw is a common problem with ASM. Are you using byte-buddy-dep at some point? This one must always be shaded for the exact above reason.

I am planning to extend the AgentBuillder API some time this weekend, I will probably release afterwards.

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 19, 2016

Owner

PS: Would you do me a favour and test out how caching works when registering the 18 agents? I have a minor idea that the overhead from that approach was mainly rooted in the fact that this now optimized overhead was applied 18 times (for each transformation).

Owner

raphw commented Apr 19, 2016

PS: Would you do me a favour and test out how caching works when registering the 18 agents? I have a minor idea that the overhead from that approach was mainly rooted in the fact that this now optimized overhead was applied 18 times (for each transformation).

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 19, 2016

Contributor

Registering individual agents is way faster than before (startup time ~35s) but combining the agents is still slightly faster (~33s). Also, these are by no means scientific measurements. I just started the application a couple of times and took the average.

I'll probably stick with the combined agent. Especially since I'm deactivating the cache after startup and it is still possible that classes are loaded after that (for example compiled JSPs, classes only referenced in method bodies, ...).

Contributor

felixbarny commented Apr 19, 2016

Registering individual agents is way faster than before (startup time ~35s) but combining the agents is still slightly faster (~33s). Also, these are by no means scientific measurements. I just started the application a couple of times and took the average.

I'll probably stick with the combined agent. Especially since I'm deactivating the cache after startup and it is still possible that classes are loaded after that (for example compiled JSPs, classes only referenced in method bodies, ...).

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 19, 2016

Owner

I agree, thanks for checking though, the approximation suffices for the information I wanted out of this. I will try to improve the API; what I thought off would be to add something like:

AgentBuilder builder = builder
  .type(foo)
  .transform(bar)
  .withFallThrough();

I have to change some of the internals of the AgentBuilder for this though so I do not yet know when I will find the time but I like this better than the manual merging.

Owner

raphw commented Apr 19, 2016

I agree, thanks for checking though, the approximation suffices for the information I wanted out of this. I will try to improve the API; what I thought off would be to add something like:

AgentBuilder builder = builder
  .type(foo)
  .transform(bar)
  .withFallThrough();

I have to change some of the internals of the AgentBuilder for this though so I do not yet know when I will find the time but I like this better than the manual merging.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 19, 2016

Contributor

I agree that a better support for compound agents would be nice. What would be the semantics of withFallThrough()?

Contributor

felixbarny commented Apr 19, 2016

I agree that a better support for compound agents would be nice. What would be the semantics of withFallThrough()?

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 19, 2016

Owner

It would mean that the matched transformation would be merged with any prior transformations that also match the type until a non-fall-through transformation is reached.

Owner

raphw commented Apr 19, 2016

It would mean that the matched transformation would be merged with any prior transformations that also match the type until a non-fall-through transformation is reached.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 19, 2016

Contributor

I'm afraid I still don't quite get it.

For my use case a transformTypes method would be nice that can be called multiple times to add multiple Transformers which are applied to the corresponding matching types.

AgentBuilder#transformTypes(ElementMatcher<? super TypeDescription>, ElementMatcher<? super java.lang.ClassLoader>, Transformer)
Contributor

felixbarny commented Apr 19, 2016

I'm afraid I still don't quite get it.

For my use case a transformTypes method would be nice that can be called multiple times to add multiple Transformers which are applied to the corresponding matching types.

AgentBuilder#transformTypes(ElementMatcher<? super TypeDescription>, ElementMatcher<? super java.lang.ClassLoader>, Transformer)
@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 19, 2016

Owner

The behaviour would be as follows:

AgentBuilder builder = builder
  .type(foo)
  .transform(bar)
  .withFallThrough()
  .type(qux)
  .transform(baz)
  .withFallThrough();

would apply both bar and baz if foo and qux positively match a type. The application order from the last to the first registered type would be followed through until a matcher is reached that does not declare itself as fall-through.

Currently, if qux matches, no other matcher is applied after; therefore fallThrough but I welcome suggestions for a different name.

Owner

raphw commented Apr 19, 2016

The behaviour would be as follows:

AgentBuilder builder = builder
  .type(foo)
  .transform(bar)
  .withFallThrough()
  .type(qux)
  .transform(baz)
  .withFallThrough();

would apply both bar and baz if foo and qux positively match a type. The application order from the last to the first registered type would be followed through until a matcher is reached that does not declare itself as fall-through.

Currently, if qux matches, no other matcher is applied after; therefore fallThrough but I welcome suggestions for a different name.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 19, 2016

Contributor

Ok, now I get it. But I think my suggestion would probably be simpler to understand for an API user. Is it harder to implement?

Contributor

felixbarny commented Apr 19, 2016

Ok, now I get it. But I think my suggestion would probably be simpler to understand for an API user. Is it harder to implement?

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 19, 2016

Owner

I want to avoid the duplication of API and keep it fluent. Currently, it is already possible to define a matcher - transformer combination, I do not think that this alternative would make sense to a reader without proper context. With an extra word in the DSL, at least one knows what documentation to read.

Owner

raphw commented Apr 19, 2016

I want to avoid the duplication of API and keep it fluent. Currently, it is already possible to define a matcher - transformer combination, I do not think that this alternative would make sense to a reader without proper context. With an extra word in the DSL, at least one knows what documentation to read.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 19, 2016

Contributor

I see. I wasn't aware of such a method. Which method is that by the way?

Contributor

felixbarny commented Apr 19, 2016

I see. I wasn't aware of such a method. Which method is that by the way?

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 19, 2016

Owner

I just released 1.3.15 with the functionality we discussed. Wasn't really a big deal. You can now chain transformations by:

AgentBuilder agentBuilder = new AgentBuilder.Default()
  .type(foo).transform(bar)//.asDecorator() - perfectly legal
  .type(foo).transform(qux).asDecorator();

As the last (dominant) matcher is defined as decorator, it is applied additionally (after) to the bar transformation that was applied before. The matching chain continues down to up until a matcher is hit that does not declare itself as decorator.

Owner

raphw commented Apr 19, 2016

I just released 1.3.15 with the functionality we discussed. Wasn't really a big deal. You can now chain transformations by:

AgentBuilder agentBuilder = new AgentBuilder.Default()
  .type(foo).transform(bar)//.asDecorator() - perfectly legal
  .type(foo).transform(qux).asDecorator();

As the last (dominant) matcher is defined as decorator, it is applied additionally (after) to the bar transformation that was applied before. The matching chain continues down to up until a matcher is hit that does not declare itself as decorator.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 19, 2016

Contributor

What are the semantics for listeners then? Is it possible to register an onTransformation listener for a specific transformer as well as to register a global listener for onError?

Contributor

felixbarny commented Apr 19, 2016

What are the semantics for listeners then? Is it possible to register an onTransformation listener for a specific transformer as well as to register a global listener for onError?

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 19, 2016

Owner

No, by using this decorator pattern, a transformation can either be applied entirely or not at all. Once the transformers are resolved, they are applied in one wash by the core engine which is not aware of the listener which operates around this application instead.

Owner

raphw commented Apr 19, 2016

No, by using this decorator pattern, a transformation can either be applied entirely or not at all. Once the transformers are resolved, they are applied in one wash by the core engine which is not aware of the listener which operates around this application instead.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 20, 2016

Contributor

Unfortunately, I think that I can't use this feature then, because I currently rely on my custom onTransformation method being called.

Contributor

felixbarny commented Apr 20, 2016

Unfortunately, I think that I can't use this feature then, because I currently rely on my custom onTransformation method being called.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 20, 2016

Contributor

But wait, I can still call this method in my Transformers.. Only a transformer specific onIgnore (neat for debugging purposes) would be difficult.

Contributor

felixbarny commented Apr 20, 2016

But wait, I can still call this method in my Transformers.. Only a transformer specific onIgnore (neat for debugging purposes) would be difficult.

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 20, 2016

Owner

I agree that this one should be called from the Transformer and not from the Listener. You could still add logging for a ignore by wrapping the matchers with logging logic. If the wrapped matcher returns false then the transformation is not applied (even though matchers are called twice on redefinition so you would need to make them statefull in the sense that they deactivate themselves).

Owner

raphw commented Apr 20, 2016

I agree that this one should be called from the Transformer and not from the Listener. You could still add logging for a ignore by wrapping the matchers with logging logic. If the wrapped matcher returns false then the transformation is not applied (even though matchers are called twice on redefinition so you would need to make them statefull in the sense that they deactivate themselves).

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 20, 2016

Contributor

Any idea about how to implement a onIgnored callback? This ist not a deal breaker but would be nice to have.

Contributor

felixbarny commented Apr 20, 2016

Any idea about how to implement a onIgnored callback? This ist not a deal breaker but would be nice to have.

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 20, 2016

Owner

Implement a raw matcher such as:

class LogMatcher implements AgentBuilder.RawMatcher {
  WeakSet<TypeDescription, ClassLoader> avoidDuplication;

  boolean matches(TypeDescription typeDescription,
                              ClassLoader classLoader,
                              Class<?> classBeingRedefined,
                              ProtectionDomain protectionDomain) {
    boolean result = // invoke actual matcher.
    if (avoidDuplication add (typeDescription, classLoader) == false) return result;
    if (result == true) <applies instrumentation>
    if (result == false) <ignored>
    return result;
  }
}

Then, for each matcher that your components register, instead of registering them directly, you register them after wrapping them in an instance of the above class. If you also want to log types that are ignored, you also need to wrap the ignore logic where you only log for a true return value which means that the value is not transformed, for false, you do nothing as that one is rehandled by the actual type matchers.

Owner

raphw commented Apr 20, 2016

Implement a raw matcher such as:

class LogMatcher implements AgentBuilder.RawMatcher {
  WeakSet<TypeDescription, ClassLoader> avoidDuplication;

  boolean matches(TypeDescription typeDescription,
                              ClassLoader classLoader,
                              Class<?> classBeingRedefined,
                              ProtectionDomain protectionDomain) {
    boolean result = // invoke actual matcher.
    if (avoidDuplication add (typeDescription, classLoader) == false) return result;
    if (result == true) <applies instrumentation>
    if (result == false) <ignored>
    return result;
  }
}

Then, for each matcher that your components register, instead of registering them directly, you register them after wrapping them in an instance of the above class. If you also want to log types that are ignored, you also need to wrap the ignore logic where you only log for a true return value which means that the value is not transformed, for false, you do nothing as that one is rehandled by the actual type matchers.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 20, 2016

Contributor

The stagemonitor build is currently failing, because EhCache's AgentSizeOf can't be instantiated: https://travis-ci.org/stagemonitor/stagemonitor/builds/124459926#L489. Unfortunately, I can't reproduce it locally. Before using ByteBuddyAgent I sometimes had this problem when wanting to attach my own agent: java.lang.UnsatisfiedLinkError: Native Library /usr/java/jdk1.8.0_40/jre/lib/amd64/libattach.so already loaded in another classloader. I was able to solve that with a pretty dirty hack: https://github.com/stagemonitor/stagemonitor/blob/6d477f49afaf11c7af747f873784557b056ffec7/stagemonitor-core/src/main/java/org/stagemonitor/core/instrument/AgentLoader.java#L64.

Do you think that could be the problem or does Byte Buddy work around the problem that the native library for the attachment could already be loaded by another agent's ClassLoader? For now, I'll probably remove the instantiation of AgentSizeOf and see if it causes reproducible real problems.

Contributor

felixbarny commented Apr 20, 2016

The stagemonitor build is currently failing, because EhCache's AgentSizeOf can't be instantiated: https://travis-ci.org/stagemonitor/stagemonitor/builds/124459926#L489. Unfortunately, I can't reproduce it locally. Before using ByteBuddyAgent I sometimes had this problem when wanting to attach my own agent: java.lang.UnsatisfiedLinkError: Native Library /usr/java/jdk1.8.0_40/jre/lib/amd64/libattach.so already loaded in another classloader. I was able to solve that with a pretty dirty hack: https://github.com/stagemonitor/stagemonitor/blob/6d477f49afaf11c7af747f873784557b056ffec7/stagemonitor-core/src/main/java/org/stagemonitor/core/instrument/AgentLoader.java#L64.

Do you think that could be the problem or does Byte Buddy work around the problem that the native library for the attachment could already be loaded by another agent's ClassLoader? For now, I'll probably remove the instantiation of AgentSizeOf and see if it causes reproducible real problems.

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 20, 2016

Owner

This is more of a really annoying limitation of the HotSpot VM. Once you load the VirtualMachine class from any ClassLoader, the attachment-process is spoiled and cannot be repeated via some other class loader from the same VM process. If EhCache is collected, it is not even possible to do it ever again.

The "easiest" would be to run Stagemonitor from a different process but I guess you do not want to do that. One possibility would be to implement your own ByteBuddyAgent.AttachmentProvider which checks for the existance of the EhCache class upon attachment. Then simply call:

ByteBuddyAgent.install(new AttachmentProvider.Compound(
  new MyEhCacheArghsStupidVmProvider(),
  AttachmentProvider.DEFAULT
));

instead of ByteBuddyAgent.install(). All you really need to do for this is to implement the isAvailable() method that checks if EhCache loaded the virtual machine class and if so, you need to return the loaded class from the getVirtualMachienType method.

Owner

raphw commented Apr 20, 2016

This is more of a really annoying limitation of the HotSpot VM. Once you load the VirtualMachine class from any ClassLoader, the attachment-process is spoiled and cannot be repeated via some other class loader from the same VM process. If EhCache is collected, it is not even possible to do it ever again.

The "easiest" would be to run Stagemonitor from a different process but I guess you do not want to do that. One possibility would be to implement your own ByteBuddyAgent.AttachmentProvider which checks for the existance of the EhCache class upon attachment. Then simply call:

ByteBuddyAgent.install(new AttachmentProvider.Compound(
  new MyEhCacheArghsStupidVmProvider(),
  AttachmentProvider.DEFAULT
));

instead of ByteBuddyAgent.install(). All you really need to do for this is to implement the isAvailable() method that checks if EhCache loaded the virtual machine class and if so, you need to return the loaded class from the getVirtualMachienType method.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 20, 2016

Contributor

So basically I have to re-apply my dirty hack 😞 . But at least it is abstracted in a better way. I hope that the Byte Buddy Agent will become the standard for runtime attachment as it can be used even without using Byte Buddy itself. Then this would not be a problem anymore...

Would it make sense to include the detection of known other agents into byte-buddy-agent?

Contributor

felixbarny commented Apr 20, 2016

So basically I have to re-apply my dirty hack 😞 . But at least it is abstracted in a better way. I hope that the Byte Buddy Agent will become the standard for runtime attachment as it can be used even without using Byte Buddy itself. Then this would not be a problem anymore...

Would it make sense to include the detection of known other agents into byte-buddy-agent?

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 20, 2016

Owner

I thought about it but the problem is that it will not work in app-servers etc as you do not necessarily find the right libraries on the current class loader. The problem will go away with Java 9, though, that is something.

Owner

raphw commented Apr 20, 2016

I thought about it but the problem is that it will not work in app-servers etc as you do not necessarily find the right libraries on the current class loader. The problem will go away with Java 9, though, that is something.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Apr 20, 2016

Contributor

You mean for example if EhCache is included within the app server? But even then, you still have access to the AgentSizeOf, don't you? What is the difference if I'm doing this within stagemonitor?

Contributor

felixbarny commented Apr 20, 2016

You mean for example if EhCache is included within the app server? But even then, you still have access to the AgentSizeOf, don't you? What is the difference if I'm doing this within stagemonitor?

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Apr 20, 2016

Owner

None, really, your approach is not guaranteed to work either. Calling: Class.forName("net.sf.ehcache.pool.sizeof.AgentLoader") resolves the class relatively to the current class loader. If the current class loader is not the same as that of EhCache, you will not discover the class and end up with the same problem.

The problem is that the VM cannot load two versions of the same native library. Even EhCache would break other instances of EhCache with this behavior.

Owner

raphw commented Apr 20, 2016

None, really, your approach is not guaranteed to work either. Calling: Class.forName("net.sf.ehcache.pool.sizeof.AgentLoader") resolves the class relatively to the current class loader. If the current class loader is not the same as that of EhCache, you will not discover the class and end up with the same problem.

The problem is that the VM cannot load two versions of the same native library. Even EhCache would break other instances of EhCache with this behavior.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Oct 12, 2016

Contributor

FYI: placing the dispatcher class inside the java. package can lead to a SecurityException. I haven't observed this behavior myself but I got a but report about it. See stagemonitor/stagemonitor#204. I don't quite know the circumstances which lead to the exception and why it has always been working for me.

In the ClassLoader class, there is a check which prevents defining classes inside the java. package: http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/8u40-b25/java/lang/ClassLoader.java/#656

To solve this and also work around the JBoss ModuleLoader issue, I reexamined the Module source code: https://github.com/jboss-modules/jboss-modules/blob/master/src/main/java/org/jboss/modules/Module.java#L92. JBoss seems to treat the package name __redirected. as a system package as well. So my dirty workaround is to move my dispatcher class to __redirected.org.stagemonitor.dispatcher.Dispatcher which seems to work. I can only hope that they don't change this magic package name. It's a pitty to say the least that the guys at JBoss don't seem like they want to address the issue you've created (https://issues.jboss.org/browse/MODULES-250).

Contributor

felixbarny commented Oct 12, 2016

FYI: placing the dispatcher class inside the java. package can lead to a SecurityException. I haven't observed this behavior myself but I got a but report about it. See stagemonitor/stagemonitor#204. I don't quite know the circumstances which lead to the exception and why it has always been working for me.

In the ClassLoader class, there is a check which prevents defining classes inside the java. package: http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/8u40-b25/java/lang/ClassLoader.java/#656

To solve this and also work around the JBoss ModuleLoader issue, I reexamined the Module source code: https://github.com/jboss-modules/jboss-modules/blob/master/src/main/java/org/jboss/modules/Module.java#L92. JBoss seems to treat the package name __redirected. as a system package as well. So my dirty workaround is to move my dispatcher class to __redirected.org.stagemonitor.dispatcher.Dispatcher which seems to work. I can only hope that they don't change this magic package name. It's a pitty to say the least that the guys at JBoss don't seem like they want to address the issue you've created (https://issues.jboss.org/browse/MODULES-250).

@CodingFabian

This comment has been minimized.

Show comment
Hide comment
@CodingFabian

CodingFabian Oct 12, 2016

Contributor

hey @felixbarny, if i read the exception correctly it is the Tomcat Classloader which is involved with creating the class. It should be the System classloader which is loading your java. prefixed classes.

Contributor

CodingFabian commented Oct 12, 2016

hey @felixbarny, if i read the exception correctly it is the Tomcat Classloader which is involved with creating the class. It should be the System classloader which is loading your java. prefixed classes.

@raphw

This comment has been minimized.

Show comment
Hide comment
@raphw

raphw Oct 12, 2016

Owner

Those classes must be injected into the bootstrap loader. Only then the java prefix is allowed and only then the classes are guaranteed to be universally visible.

Owner

raphw commented Oct 12, 2016

Those classes must be injected into the bootstrap loader. Only then the java prefix is allowed and only then the classes are guaranteed to be universally visible.

@felixbarny

This comment has been minimized.

Show comment
Hide comment
@felixbarny

felixbarny Oct 12, 2016

Contributor

You are right. The exception is triggered here: https://github.com/stagemonitor/stagemonitor/blob/0.27.0/stagemonitor-core/src/main/java/org/stagemonitor/core/instrument/AgentAttacher.java#L105

Something must have gone wrong in ensureDispatcherIsAppendedToBootstrapClasspath. I think bootstrapClassloader.loadClass(DISPATCHER_CLASS_NAME) did for some reason not throw an exception so it is assumed the class has already been injected. Probably it's better for the class to stay in the java package rather than in the weird JBoss specific __redirected package.

Contributor

felixbarny commented Oct 12, 2016

You are right. The exception is triggered here: https://github.com/stagemonitor/stagemonitor/blob/0.27.0/stagemonitor-core/src/main/java/org/stagemonitor/core/instrument/AgentAttacher.java#L105

Something must have gone wrong in ensureDispatcherIsAppendedToBootstrapClasspath. I think bootstrapClassloader.loadClass(DISPATCHER_CLASS_NAME) did for some reason not throw an exception so it is assumed the class has already been injected. Probably it's better for the class to stay in the java package rather than in the weird JBoss specific __redirected package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment