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

Classloader magic #9

Merged
merged 3 commits into from
Nov 5, 2019
Merged

Classloader magic #9

merged 3 commits into from
Nov 5, 2019

Conversation

zhangjiangqige
Copy link

This PR resolves #7 and is necessary for opprop/do-like-javac#16.

As commented here, different classloaders are somehow used to load classes (i.e. IneferenceMain) before and during the compilation in CheckerFrameworkUtil. In Bazel, it seems that this similar kind of problem is met as well, and they used some kind of classloader magic to solve this classloader hierarchy issue, which is what's copied to this PR.

But this fix does look weird though...

@zhangjiangqige
Copy link
Author

I've just found the root cause of this issue... Some background knowledge is that class loaders are hierarchical.

In issue #7 , what's observed in the table is indeed related to howcom.sun.* are loaded. If they're in the bootclasspath, then they are loaded by the bootstrap loader, otherwise it's sun.misc.Launcher$AppClassLoader. Meanwhile, InferenceMain is loaded by sun.misc.Launcher$AppClassLoader as well, before the compilation.

The problem emerges during the execution of javac. The related callstack is:

at com.sun.tools.javac.file.JavacFileManager.getClassLoader(JavacFileManager.java:693)
        at com.sun.tools.javac.processing.JavacProcessingEnvironment.initProcessorLoader(JavacProcessingEnvironment.java:268)
        at com.sun.tools.javac.processing.JavacProcessingEnvironment.<init>(JavacProcessingEnvironment.java:234)
        at com.sun.tools.javac.processing.JavacProcessingEnvironment.instance(JavacProcessingEnvironment.java:191)
        at com.sun.tools.javac.main.JavaCompiler.initProcessAnnotations(JavaCompiler.java:1122)
        at com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:908)
        at com.sun.tools.javac.main.Main.compile(Main.java:302)

in the line JavacProcessingEnvironment.java:268, a dedicated classloader is used for loading annotation processors, and this leads us to BaseFileManager.java:177, which is the key to this issue.

The class loader returned here depends on how this com.sun.tools.javac.file.BaseFileManager is loaded. If com.sun.* is in the bootclasspath, then the class loader here is null, which denotes the bootstrap class loader. Then the bootstrap loader will be the parent of the URLClassLoader returned, and this URLClassLoader will be used to load InferenceChecker, which references InferenceMain in its method. InferenceMain here is also loaded by the class loader of InferenceChecker: the URLClassLoader returned by BaseFileManager. The problem is that this URLClassLoader is in a class loader hierarchy that's different from the sun.misc.Launcher$AppClassLoader hierarchy, leading to two "instances" of InferenceMain with two separate sets of static members being loaded.

So the solution could be to "hijack" javac, i.e. make it load InferenceChecker with the same classloader as the original InferenceMain: sun.misc.Launcher$AppClassLoader, which is done in the new commit: a sub-class of JavacFileManager is used as the file manager for javac. It's important that this class is defined in a place that's only in the class path, and hence it's going to be loaded by sun.misc.Launcher$AppClassLoader. With this class, BaseFileManager.java:177 will also be sun.misc.Launcher$AppClassLoader due to polymorphism. Then InferenceMain in InferenceChecker can be loaded by the same class loader as the one that's used before invoking javac.

@wmdietl
Copy link
Member

wmdietl commented Nov 5, 2019

Thanks for all your work on #7 and this PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants