AsmReflector throws exceptions on JDK8

Question

After the commit of the updated ASM library for CLJ-713, Clojure builds and passes all tests except for one, compare-reflect-and-asm in reflect.clj.

This can be narrowed down somewhat to a difference in behavior of the following 2 forms evaluated with the latest Clojure and JDK8:

`
;; The following two lines work with the latest (Jan 11 2014) Clojure 1.6.0-master-SNAPSHOT
;; if run on JDK 6 or JDK 7, but throw an exception with JDK 8.

(import '[clojure.asm ClassReader ClassVisitor Type Opcodes])
(def r (ClassReader. "java.lang.Object"))
`

I am not certain, but from a bit of Google searching it appears that this may be a limitation of the ASM library version 4 -- it throws exceptions when attempting to read class files produced by JDK 8, because of a newer classfile version number. Links that seem to support this conclusion:

http://mail-archive.ow2.org/asm/2013-02/msg00000.html

http://forge.ow2.org/tracker/index.php?func=detail&aid=316375&group_id=23&atid=350023

A couple of alternatives are:

(1) update ASM again to one that supports JDK 8 class files

(2) disable the compare-reflect-and-asm test. Clojure itself does not use the AsmReflector for anything except this unit test. The Java reflector is the default one.

Answer 1

Comment made by: alexmiller

1) There is no released ASM that supports JDK 8 yet. ASM 5 will but it will not be final till JDK 8 is in the final stages of release.

2) Probably more likely.

Answer 2

Comment made by: bronsa

As of now, both JDK8 and ASM5 are out.
I just tried compiling clojure on JDK8 with ASM5 and all compiles fine

Answer 3

Comment made by: alexmiller

How are you running this test?

Answer 4

Comment made by: bronsa

I downloades ASM5, replaced the bundled ASM that comes with clojure with that one after changing che package name to "clojure.asm" and run mvn install, all the tests pass.

Answer 5

Comment made by: alexmiller

I was actually talking about the JDK 8 change - was curious about exactly what was being changed?

Answer 6

Comment made by: alexmiller

In particular, I'm assuming that you're not altering the build.xml to change the compilation -source or -target and running with JAVA_HOME / path set to JDK 8.

We don't have any plans to actually build Clojure with JDK 8 any time soon, so I'm not overly concerned about that. But it does appear that the embedded ASM 4 cannot read newer class files from JDK 8. Afaik, the only place that happens is in clojure.reflect.java in the AsmReflector, which is not the default reflector. The JavaReflector will properly reflect the Java 8 classes.

ASM 5 has only been out a couple days and already has at least one serious bug reported - I'd like that to see more use before we switch to it, so maybe this is a good target for the release after Clojure 1.6.

Answer 7

Comment made by: alexmiller

Patch to temporarily disable the failing test until we have an ASM that supports JDK 8.

Answer 8

Comment made by: jafingerhut

I have tested the latest Clojure 1.10.0-RC3 code, with the test that was disabled re-enabled again, using the patch in attachment clj-1323-experiment-v1.patch.

Without the change to file src/clj/clojure/reflect/java.clj, it fails when running with OpenJDK 11, because the uncommented test throws an exception when it attempts to call {{(type-reflect classname :reflector asm-reflector)}} for class {{java.io.FileInputStream}} (and perhaps others after that in the vector).

With the patch as written, all tests pass using these version combinations:
+ Mac OS X 10.13.6 plus Oracle Java 1.8.0_192
+ Ubuntu 16.04.5 plus OpenJDK 11 - the uncommented test throws

I am not saying that I know this patch is good, or the full ramifications of the change I made in file src/clj/clojure/reflect/java.clj. I was poking at the code to run an experiment to see if I could make the test that was commented out pass again. Someone like Ghadi Shayban would be better than I at assessing the ramifications of such a change.

Answer 9

Comment made by: alexmiller

Not planning to do this for Clojure 1.10 but can assess for 1.11.

Answer 10

Comment made by: jafingerhut

Definitely understood -- no rush for 1.10 release here. Just scanning through the list of open defect tickets for the first time in a while, and seeing which ones I might have something to say a little bit about.

Answer 11

Reference: https://clojure.atlassian.net/browse/CLJ-1323 (reported by jafingerhut)