-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
JEP 238 - Multi-Release JAR files break bytecode scanning #1797
Comments
Multi-Release JAR Files are documented at http://openjdk.java.net/jeps/238 |
…bytecode scanning Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@janbartel please see PR #1801 for a proposed fix on this. |
Bringing this conversation back over to the issue (from PR #1801) to preserve the history of the issue better. Relying on our ASM support version isn't sufficient for the skip of Its the JVM version that dictates the use of With Java 1.8, there is no support for However, to follow the JEP 238 guidelines, and the proposed increased velocity on JDK releases, we should also keep in mind that
Incidentally, we don't need ASM 6 to support Java 9 bytecode scanning. |
The current PR #1801 does the appropriate thing for Java 1.8 on Jetty 9.4+ However, @janbartel has pointed out that the replacement / override behavior of class files in Multi-Release JARs in our bytecode scanning isn't correct on the Java 9+ use cases. What this means ... Lets take
Of these two classes
If using If using Java 9 JVM, you get the The code used to test: private static void findResource(URLClassLoader classLoader, String name)
{
System.out.printf("Resource[%s] = %s%n", name, classLoader.findResource(name));
} The output seen:
Interestingly, if you use the The code private static void findResources(URLClassLoader classLoader, String name) throws IOException
{
Enumeration<URL> resources = classLoader.findResources(name);
System.out.printf("Resources[%s]%n", name);
while(resources.hasMoreElements())
{
URL url = resources.nextElement();
System.out.printf(" = %s%n", url);
}
System.out.println();
} The output
When running on Java 9, the root level resource is not returned on a The Java 9+ use case for this is to have our bytecode scanning use the same override logic that JEP238 uses to not scan the root level class if the However, a interesting wrinkle on this behavior is exposed by the
The If you trust the Java 9 |
@gregw bring up questions about ignoring the root level class efficiently during a bytecode scan. I wonder what we do when we encounter a duplicate class in our bytecode/annotation scanning now? Eg:
If we assume the scan occurs with Does the second appearance of If we replace, what do we do when the relationships in the class change (the I would assume we have logic for this. If we have this kind of safety net, then the JEP238 root replacement logic doesn't seem complicated. |
Thinking about this some more, and interesting impact of any kind of "selection" of duplicate classes during bytecode scanning is that the behavior has to match what the ClassLoader later performs. In the Does this imply that the bytecode scanning results should be communicated to the |
Perhaps the bytecode scanning should |
Maybe the Log4J inner class |
There is no code already in place to try and second guess what the class loader will do with duplicate classes. The servlet spec only provides jar ordering/exclusion as a mechanism to control annotation scanning. This problem is another level of granularity - within the jar itself - and the spec has no means to deal with it. |
Ideally we would not have to second guess the jvm's classloader and could just call a library method to access the jvm's logic... even if we have to put that code in a versioned class ourselves. However, at this stage, the experiments that Jan and I have done indicate that the java 9 versioned JarFile class is broken... or at least does not do what the javadoc says it will do. So currently I think we will have to duplicate the logic in our own code. However, short term, let's just apply the current PR for jetty-9.4.7, as this will allow multi-version jars to be deployed on java 8 and run correctly. We then need to do the work so that a jetty 9 can run on java 9 and that we will scan the correct version (may need to wait for a new version of ASM). |
Issue #1797 - Safety checks for Java 8 and Multi-Release JARs during bytecode scanning
I think the handling of inner classes is indeed going to be difficult. I've posted to the servlet EG and reached out to a CDI contact to seek clarification. The problem as I see it is that a multi versioned jar might contain something like:
It is clear that there is a java 9 version of Foo. But what is unclear is the inner class Foo$Bar? Is that only used by the base Foo version? or does the java 9 version also use the Foo$Bar inner class, but it didn't use any java 9 features, so the base version is able to be used?? So it looks like we are going to need to analyse the actual Foo class version used to see if Foo$Bar is referenced and only then scan Foo$Bar for annotations (and recursive analysis for Foo$Bar$Bob class )! It means that given the index only of a jar, it is impossible to come up with the list of classes that will be provided by that jar for a given JVM! The only alternative would be if there was an enforced convention that any versioned class would also version all of it's inner classes - which may be a reasonable assumption given that they would be compiled together, but we see nothing in the specifications that force a jar to be assembled that way. |
Link to Servlet EG mailing list about this topic (that @gregw mentioned) https://javaee.groups.io/g/servlet-spec/topic/annotation_scanning_with_java/6012186 |
Link to bug raised with Oracle: http://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8187528 |
@janbartel see comments at https://bugs.openjdk.java.net/browse/JDK-8187528, apparently it's not a bug ? |
Those comments make no sense. The javadoc for jdk-9 for JarFile.getJarEntry(String name) says:
The observed behaviour from a test case based on the log4j-api-2.9.0.jar is that the base version only of a versioned class is returned. Furthermore, code inspection of the implementation of JarFile.getJarEntry method shows that it searches the base first and returns it if it exists. It only ever examines versions if there is no base. |
@janbartel can you comment on the OpenJDK issue tracker ? |
@sbordet no, apparently not. It seems you have to be an "Author", where that is defined here: http://openjdk.java.net/bylaws#author |
@janbartel I guess the next action is to write an email to a jdk9 mailing list referencing the bug. |
Some tests of JarFile ... $ jar -tvf log4j-api-2.9.0.jar | grep StackLocator.class
6336 Sat Aug 26 13:37:16 GMT 2017 org/apache/logging/log4j/util/StackLocator.class
7237 Sat Aug 26 13:37:10 GMT 2017 META-INF/versions/9/org/apache/logging/log4j/util/StackLocator.class New Java 9 JarFile Constructor: Using the new JarFile Constructor from Java 9, in example code like this ... File file = new File("log4j-api-2.9.0.jar");
String javaVersion = System.getProperty("java.version");
Runtime.Version runtimeVersion = Runtime.Version.parse(javaVersion);
System.out.println("java.version=" + javaVersion);
System.out.println("Runtime.Version=" + runtimeVersion);
try (JarFile jarfile = new JarFile(file, true, ZipFile.OPEN_READ, runtimeVersion))
{
JarEntry entry = jarfile.getJarEntry("org/apache/logging/log4j/util/StackLocator.class");
System.out.println("JarEntry: " + entry);
System.out.println("Compressed Size: " + entry.getCompressedSize());
System.out.println(" Size: " + entry.getSize());
} Output is:
Which is the correct Default JarFile Constructor: However, if we change to use default try (JarFile jarfile = new JarFile(file))
{
JarEntry entry = jarfile.getJarEntry("org/apache/logging/log4j/util/StackLocator.class");
System.out.println("JarEntry: " + entry);
System.out.println("Compressed Size: " + entry.getCompressedSize());
System.out.println(" Size: " + entry.getSize());
} Then we get the root level
Gotchas: To be fair, this "get versioned entry" behavior seems to only be supported via the direct Iterating through the entries with Interesting Notes: Of note in this discovery, the
|
Our current Heck, we can even use Multi-Release JAR ourselves in the |
It seems multirelease JARs are causing lots of mayhem for tools (mojohaus/animal-sniffer#32 etc.), for pretty marginal benefit: you can generally get the same effect with a small amount of reflection, or |
…bytecode scanning Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Changes made for jetty-9.3.x, 9.4.x and 10 to support multirelease jars and asm6. |
reopened due to NPE discovered in #1892 |
Made MultiReleaseJarFile closeable and using try-with-resources in AnnotationParser to avoid leaking file descriptors. Made few code simplifications. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
…ng. (#1951) Made MultiReleaseJarFile closeable and using try-with-resources in AnnotationParser to avoid leaking file descriptors. Made few code simplifications. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
…ng. Fixed for java 10 Signed-off-by: Greg Wilkins <gregw@webtide.com>
Thanks. Similar issue here: https://stackoverflow.com/questions/49492583/sprint-boot-web-application-running-on-google-app-engine-throws-jetty-exceptio . So then how can we run a Spring Boot application (.war) with Gradle build on App Engine Standard. |
Hi!
Runing Jetty 9.4.6 gives a startup error with the "multi-version" JARs. An example is e.g. LOG4J2 2.9.0.
This seems to be similar to #1692
Here are the stack traces:
The text was updated successfully, but these errors were encountered: