Warning on JDK9

[kanwei]

From using boot on JDK9:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by dynapath.defaults$fn__1516$fn__1517 (file:/Users/kanwei/.m2/repository/boot/pod/2.7.2/pod-2.7.2.jar) to method java.net.URLClassLoader.addURL(java.net.URL)
WARNING: Please consider reporting this to the maintainers of dynapath.defaults$fn__1516$fn__1517
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

[tobias]

There's not much we can do about this warning in dynapath itself, since java 9 now warns on any reflective access to private/protected methods, and removing that from dynapath would break consumers. However, this should be fixed in boot-bin 2.7.2 by https://github.com/boot-clj/boot-bin/commit/172a3d2b12c8299b0bac7f1431f8a621c695e5ff - what does boot --version report?

[tobias]

And what are the steps to recreate it?

[kanwei]

boot --version
#http://boot-clj.com
#Sun Oct 08 13:53:07 EDT 2017
BOOT_VERSION=2.7.2
BOOT_CLOJURE_VERSION=1.9.0-beta1
BOOT_CLOJURE_NAME=org.clojure/clojure

Running pretty much any boot command (like boot dev) returns this warning for me, on multiple computers

[glts]

Same with Leiningen 2.8.0, technomancy/leiningen#2331.

[technomancy]

I feel like this might need to be fixed in dynapath at some point in the future. Here's the relevant dynapath calls from pomegranate:

(if-let [cl (last (filter dynapath/addable-classpath? classloaders))]
  (dynapath/add-classpath-url cl (.toURL (.toURI (io/file jar-or-dir))))
  (throw (IllegalStateException. (str "Could not find a suitable classloader to modify from "
                                      classloaders))))

Pomegranate is asking Dynapath whether a given classloader will work with add-classpath-url before calling that function on it. If I'm reading the warnings correctly, the heuristics for whether a given classloader can have its classpath changed are going to depend on what version of the JDK is running, right?

I believe continuing with the current path of emitting a warning is correct at present though, because the caller function is going to need to change how it uses classloaders in order not to break on future JDKs.

[technomancy]

It turns out this is a bit difficult to work around because the dynapath.defaults modifies the URLClassLoader class at load-time. It would be much nicer if there were a way to opt out of that behavior: https://github.com/tobias/dynapath/blob/master/src/dynapath/defaults.clj#L20

The existing behavior is reasonable if you assume that the reflective call will either pass or fail, but it doesn't seem like the right thing to do when it triggers a warning.

Can we bump the major version number and move this to a function call where you have to opt in to get the extend behavior?

[technomancy]

I've tested this, and moving the part where we extend URLClassLoader into a function silences the warnings in Leiningen. I can provide a PR if desired. (The patch I used to test it has no docs.)

[tobias]

Thanks @technomancy for catching that this was happening at load time. I think the opt-in approach would work, but one of the goals of dynapath is to be plumbing that consumers of pomegranate, etc shouldn't have to ever worry about. A better approach may be to just remove the option to extend URLClassLoader altogether, and recommend that libs that use dynapath ensure there is a modifiable classloader close to the top of the tree. I'll give it some thought, and try to get you a release out sometime today (though it may be this evening).

[tobias]

I've pushed a snapshot (1.0.0-SNAPSHOT) based on the https://github.com/tobias/dynapath/tree/drop-urlclassloader branch that drops addable support for URLClassLoader, but maintains the readable support. Can you test this and see if it solves your issues? It's hosted on oss.sonatype, so you'll need to add their snapshots repo to project.clj: ["sonatype-snapshots" "https://oss.sonatype.org/content/repositories/snapshots/"].

[technomancy]

Thanks @tobias. Testing against the 1.0.0-SNAPSHOT of dynapath seems to do the trick!

[tobias]

This fixed in 1.0.0, issuing a PR to pomegranate now to update it to 1.0.0 as well.