Skip to content
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

Crash when using a custom classloader to load instrumentable code #196

Closed
mikehearn opened this issue May 31, 2016 · 4 comments
Closed

Crash when using a custom classloader to load instrumentable code #196

mikehearn opened this issue May 31, 2016 · 4 comments
Labels

Comments

@mikehearn
Copy link
Contributor

Our app loads classes dynamically inside a simple ClassLoader (it just loads class files from somewhere that isn't disk). This causes instrumentation failures for code that works fine if loaded non-dynamically.

From looking at the code it appears that there is some support for differing classloaders but it's not clear how it's meant to work. The Java agent creates a single instrumentor, with the default app class loader, and then that's always used to look up classes. The classloader passed into the transform method is added to a global weak set, but it's unclear why or where that set is used. When methods are loaded for instrumentation, the MethodDatabase in the instrumentor is used, and that uses the classloader from startup time.

Is the support for multiple classloaders unfinished? What is the purpose of the Retransform class, as it seems there are some mechanisms there to work with classloaders but IntelliJ claims these methods aren't used anywhere.

@pron
Copy link
Contributor

pron commented Jun 2, 2016

The instrumentor definitely should (i.e, in terms of specification) support multiple class loaders, but our current test suite doesn't test this, and I assume this use-case hadn't been exercised prior to you, to I'm going to mark this as a bug, and take a look at it soon.

@pron pron added the bug label Jun 2, 2016
@mikehearn
Copy link
Contributor Author

Thanks!

@circlespainter
Copy link
Member

@mikehearn As highlighted by #209, at present classes with the same name but different classloaders clash in Quasar's suspendables table. I've been able to exploit this clash to create a situation where two classes with the same name, but loaded by different classloaders, annotate the same method as @Suspendable and not suspendable resp. but it gets instrumented in both cases.

I haven't yet figured out how to make the other way around happen though: it looks to me like such clashes can only increase the "suspendable level" of methods, not decrease it. Did this happen to you instead? Or which other problematic situations have you found?

@mnesbit
Copy link

mnesbit commented Jun 29, 2016

When the JavaAgent is passed a class loaded in a custom class loader then the MethodDatabase will only call getResources on the application ClassLoader. As a result the referenced parent classes and Suspendable interfaces cannot be found if they are only reachable by the custom ClassLoader. The default behaviour is to assume the method call are suspendable and carry on. Whilst this does work it means there are lots of warnings from quasar about missing classes and presumably the unecessary instrumentation will have a performance impact.

@pron pron closed this as completed Aug 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants