restrict plugin classpath

[bastiK]

Currently, we have one joined classpath for all plugins. This means a plugin can load classes from all other plugins the user has enabled.

It would be cleaner, if access was restricted to JOSM core and all dependencies (required plugins). This is implemented in the attached patch, a separate ClassLoader for each plugin is created: It first looks up a class in JOSM core, then refers to the class loader of dependent plugins and finally in the plugin jar.

One consequence is that you can include different conflicting versions of the same library in different plugins. In general, we should aim to share libraries by creating a library plugin, but in certain situations this can be impractical or impossible.

[bastiK]

WIP

[Don-vip]

Have you tested it with Java 9 ? The classpath changes a lot with this version.

[bastiK]

It appears to work (build 9-ea+166). Haven't they made it backward compatible for the most part?

[Don-vip]

OK I'll give it a try :)

[Don-vip]

It doesn't work on my setup, crash at startup:

Revision:12318
Is-Local-Build:true
Build-Date:2017-06-05 22:27:44

Identification: JOSM/1.5 (12318 SVN en) Windows 10 64-Bit
OS Build number: Windows 10 Pro 1703 (15063)
Memory Usage: 370 MB / 4088 MB (158 MB allocated, but free)
Java version: 9-ea+172, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: \Display0 1920x1080, \Display1 1920x1080, \Display2 1280x1024
Maximum Screen Size: 1920x1080
VM arguments: [--add-exports=java.base/sun.security.util=ALL-UNNAMED, --add-exports=java.base/sun.security.x509=ALL-UNNAMED, --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED, --add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED, --add-exports=jdk.deploy/com.sun.deploy.config=ALL-UNNAMED, --add-modules=java.se.ee, --add-opens=java.desktop/javax.imageio.spi=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.spi=ALL-UNNAMED, --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED, --add-opens=java.base/java.lang=ALL-UNNAMED, --add-exports=java.desktop/com.apple.eawt=ALL-UNNAMED, -Dfile.encoding=UTF-8]
Program arguments: [--debug]

Plugins:
+ DxfImport
+ ImportImagePlugin
+ apache-commons
+ apache-http
+ canvec_helper
+ ejml
+ epci-fr (32699)
+ geotools
+ gson
+ imagery-xml-bounds
+ jna
+ jts
+ log4j (32699)
+ o5m
+ opendata
+ pbf
+ public_transport_layer
+ tag2link
+ utilsplugin2

Last errors/warnings:
- E: Handled by bug report queue: java.lang.AssertionError
- W: No configuration settings found.  Using hardcoded default values for all pools.


=== REPORTED CRASH DATA ===
BugReportExceptionHandler#handleException:
No data collected.

Warning issued by: BugReportExceptionHandler#handleException

=== STACK TRACE ===
Thread: main (1)
java.lang.AssertionError
	at org.openstreetmap.josm.plugins.PluginHandler.loadPlugins(PluginHandler.java:835)
	at org.openstreetmap.josm.plugins.PluginHandler.loadLatePlugins(PluginHandler.java:882)
	at org.openstreetmap.josm.gui.MainApplication.loadLatePlugins(MainApplication.java:435)
	at org.openstreetmap.josm.gui.MainApplication.main(MainApplication.java:362)

[bastiK]

Replying to Don-vip:

It doesn't work on my setup, crash at startup:

Yes, there was a bug - I've updated the patch.

[Don-vip]

OK it works, even with transitive dependencies and Java 9.

Some minor style remarks:

1. Can we get rid of label statement DEPENDENCIES: ?
2. Don't forget to document all public APIs ;)

[bastiK]

Replying to Don-vip:

OK it works, even with transitive dependencies

Yes, it should work.

and Java 9.

Not that much different in terms of used API, so if it fails in Java 9, then I would expect it to fail with or without the patch.

Some minor style remarks:

1. Can we get rid of label statement DEPENDENCIES: ?

Label statements make the code shorter and easier to understand, what's wrong with them?

[Don-vip]

I found them harder to understand, personal taste :) If it's too complicated, no problem.

[bastiK]

In 12322/josm:

fixed #14901 - restrict plugin classpath

Separate class loader for each plugin instead of unified class loader.

[bastiK]

In GeoToolsPlugin.java, the now deprecated PluginHandler.getPluginClassLoader() is used. I'm not sure what it does and if it is still working, but this call should probably be replaced by GeoToolsPlugin.class.getClassLoader().

[bastiK]

In 12323/josm:

see #14901 - some cleanup

[stoecker]

I believe Integration test fail because of this. The videomapping plugin. But I didn't find the reason why it fails now and did not fail before.

[Don-vip]

slf4j is required by vlcj, but was provided only through OsmRecPlugin. It proves the system works as expected. As both plugins are rarely used, the best solution is to include slf4j in videomapping plugin.

[Don-vip]

Done in [o33376:33377].

[Don-vip]

GeoTools plugin updated as well.