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

Don't depend on jsr308-langtools #7

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Don't depend on jsr308-langtools #7

wants to merge 7 commits into from

Conversation

wmdietl
Copy link
Member

@wmdietl wmdietl commented Aug 20, 2019

No description provided.

@wmdietl
Copy link
Member Author

wmdietl commented Sep 27, 2019

@zhangjiangqige Can you have a look at this?

@zhangjiangqige
Copy link

@zhangjiangqige Can you have a look at this?

Yes sure.

@zhangjiangqige
Copy link

It looks like that the class com.sun.tools.javac.main.Main used by CheckerFrameworkUtil in javac.jar (errorprone) is different from that in JDK 8. I tried removing the class files of com.sun.* from javac.jar, and then everything works properly. And previously there were no class files for com.sun.* in javac-jdk.jar from langtools.

However I don't know why is this happening... The static/singleton member inferenceMainInstance is initialized at the beginning before com.sun.tools.javac.main.Main.compile is called, but after that inferenceMainInstance becomes null. It looks to me that the method compile somehow created a new isolated memory space for doing all the work.

@wmdietl Do we need to investigate com.sun.tools.javac.main.Main.compile?

@zhangjiangqige
Copy link

I tried several combinations and found that:

  • if there's no com.sun.* in the bootclasspath, it will be loaded from annotation-file-utilities-all.jar, and there's no error
  • if com.sun.* is removed from the bootclasspath and javac.jar is placed in the classpath, then there's no error
  • if annotation-file-utilities-all.jar is placed in the bootclasspath instead of javac.jar, this error persists

So it seems that com.sun.* must not be in the bootclasspath. In addition, the JDK 9 version of javax.tools.StandardLocation (and perhaps some more javax classes) must be in the bootclasspath, otherwise java.lang.NoSuchFieldError: ANNOTATION_PROCESSOR_MODULE_PATH will be thrown.

Therefore, we probably need to find a jar that:

  • contains the JDK 9 version of javax.tools, and
  • doesn't have com.sun.*

and put it in the bootclasspath...

@zhangjiangqige
Copy link

Probably related issue: facebook/buck#1781

I tried creating a jar containing only javax.* from errorprone, as in Bazel which is mentioned in that issue, and putting this jar in the bootclasspath, javac.jar in the classpath, and then CFI can run in this setting.

As for the root cause, I still can't figure it out. It's like that the entire memory space is cleared when compiling if com.sun.* is in the bootclasspath; I tested adding another new irrelevant static int member variable to InferenceMain and set it to 100 in the constructor, and then this variable becomes 0 when InferenceChecker.initChecker is called. There seems to be some black magic in javac....

As for CheckerMain, it actually puts errorprone in its bootclasspath if it's JVM 8. And the reason why putting errorprone in this bootclasspath doesn't cause any problem is probably that eventually the errorprone javac is launched as a new process and there aren't many things for it to "interfere". I'm suspecting that all static members of classes outside of javac in this case are also cleared, though I haven't verified this idea yet.

@zhangjiangqige
Copy link

Somehow in eisop, different classloaders are used for loading classes before and during the "compilation". This can be seen by adding -XX:+TraceClassLoading -XX:+TraceClassUnloading when launching InferenceMain.

In eisop,

  before compile in compile (initChecker)
com.sun in bcp sun.misc.Launcher$AppClassLoader@33909752 java.net.URLClassLoader@7bfcd12c
com.sun not in bcp sun.misc.Launcher$AppClassLoader@33909752 sun.misc.Launcher$AppClassLoader@33909752

and in typetools,

  before compile in compile (initChecker)
com.sun in bcp sun.misc.Launcher$AppClassLoader@3d4eac69 sun.misc.Launcher$AppClassLoader@3d4eac69

The classloaders used for loading InferenceMain before javac.compile is called and during its execution are clearly different in eisop, and I think this explains the phenomenon "cleared memory space" as the same "class instance" of InferenceMain accessed are actually from two classloaders and thus have different memory space.

@msridhar
Copy link

I saw this issue referenced from facebook/buck#1781. I'm not sure what is going on but these instructions on how we integrated Error Prone for Buck builds may be useful:

https://github.com/uber/okbuck/wiki/Using-Error-Prone-with-Buck-and-OkBuck

@zhangjiangqige
Copy link

I saw this issue referenced from facebook/buck#1781. I'm not sure what is going on but these instructions on how we integrated Error Prone for Buck builds may be useful:

https://github.com/uber/okbuck/wiki/Using-Error-Prone-with-Buck-and-OkBuck

Thanks for the info! That script for creating the tiny jar is really useful.

@zhangjiangqige
Copy link

I tried the classloader magic from bazel in https://github.com/zhangjiangqige/checker-framework-inference/commit/394ffd36da4db105131c7329596f9e537bab53e2 and it now can succeed with the whole javac.jar in bcp. But this probably seems too magical.

@wmdietl
Copy link
Member Author

wmdietl commented Nov 5, 2019

@zhangjiangqige Thanks for getting this to work! Please make a PR with these changes against typetools.

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.

3 participants