A compromise approach on module isolation

David M. Lloyd david.lloyd at redhat.com
Fri May 12 18:56:26 UTC 2017


It seems to work in my tests.  Can you explain what you mean?

$ jlink --module-path $JAVA_HOME/jmods:out/production --add-modules 
org.module.two,org.module.one --output new-test 
-J-Djdk.module.detect-cycles=false -J-Djdk.module.isolated=true 
--launcher one=org.module.one/org.module.one.Main --launcher 
two=org.module.two/org.module.two.Main
$ new-test/bin/two
This is module two, my 
cl=jdk.internal.loader.ClassLoaders$AppModuleClassLoader at 2b05039f

On 05/12/2017 01:54 PM, Remi Forax wrote:
> It doesn't work because application modules becomes system modules,
> so even if you propagate the flags, an application modules becomes a 
> system module hence is not deployed in a class loader anymore.
> 
> Rémi
> 
> 
> On May 12, 2017 6:45:18 PM GMT+02:00, "David M. Lloyd" 
> <david.lloyd at redhat.com> wrote:
> 
>     Here's a commit which does so (and also covers the circularity case), if
>     you want to play with it a bit:
> 
>     https://github.com/dmlloyd/openjdk-modules/commit/ffbca0555aeb934181ac92ec3a2bf48fae7efe04
> 
>     On 05/12/2017 08:45 AM, David M. Lloyd wrote:
> 
>         Hi Rémi, thanks for the feedback.
> 
>         At the moment you can fix this by editing the launcher scripts and
>         adding "-Djdk.module.isolated=true" to the end of the
>         "JLINK_VM_OPTIONS=" line (and similarly for Windows).
> 
>         I think the tool can do this automatically with only minimal
>         changes.
>         I'll have a look at it.
> 
>         On 05/12/2017 03:28 AM, Remi Forax wrote:
> 
>             Hi David,
>             with this patch, there is an issue if people (like me :) )
>             are using
>             jlink.
> 
>             With jlink, application modules becomes system modules so
>             depending if
>             the application is delivered as a bunch of jars or as a
>             custom jlink
>             image, so behavior will be different because the application
>             modules
>             deploy with jlink will not be sandboxed.
> 
>             Maybe it means that jlink has to be changed, i do not know,
>             but i
>             think i would prefer that the module itself should decide
>             using the
>             module-info if it should run its own classloader or not, at
>             least
>             jlink can refuse to work if a module ask for being deployed
>             in its own
>             classloader
> 
>             And i'm still not sure that this kind of code has to be in
>             the jdk, it
>             can be easily written outside without putting the burden of
>             having to
>             maintain it in the JDK.
> 
>             regards,
>             Rémi
> 
> 
>             ----- Mail original -----
> 
>                 De: "David M. Lloyd" <david.lloyd at redhat.com>
>                 À: jpms-spec-experts at openjdk.java.net
>                 Envoyé: Vendredi 12 Mai 2017 01:17:45
>                 Objet: A compromise approach on module isolation
> 
> 
>                 In the wake of the PR vote, one of the most oft-cited
>                 unmet expectations
>                 was the lack of package namespace isolation among
>                 modules. I'd like to
>                 propose a patch [1] which would allow this type of
>                 module isolation to
>                 be used by the run time on an opt-in or opt-out basis.
> 
>                 Justification:
> 
>                 • Large applications are more likely to encounter
>                 concealed and
>                 non-concealed package conflicts.
>                 • By making the change opt-in or opt-out, any
>                 application which expects
>                 all of its modules to be loaded by a single class loader
>                 can still
>                 function, while allowing more complex applications
>                 (which already would
>                 necessarily have required at least some separated class
>                 loaders) to
>                 continue to work.
>                 • The patch seems to have a small complexity and
>                 maintenance footprint.
> 
>                 Drawbacks:
> 
>                 None that I am aware of.
> 
>                 Patch information:
> 
>                 The approach of this patch simply looks for a system
>                 property and,
>                 depending on the value of this property, establishes
>                 isolated module
>                 loaders. I relinquish any copyright claim to the patch
>                 in this mail.
>                 Please do not get bogged down by any formatting problems
>                 introduced by
>                 the mailing list; the purpose of directly including the
>                 patch is to give
>                 a clear, unambiguous subject for discussion. I can
>                 provide a proper
>                 webrev (or whatever other form is requested) if needed.
> 
>                 Other than the system property (which is just one
>                 possible approach), no
>                 API or specification changes should be needed. This
>                 version uses the
>                 property "jdk.module.isolated" defaulting to "false".
> 
>                 [1]:
> 
>                 diff --git
>                 a/jdk/src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java
>                 <http://ClassLoaders.java>
>                 b/jdk/src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java
>                 <http://ClassLoaders.java>
>                 index d976aab..f06adfb 100644
>                 ---
>                 a/jdk/src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java
>                 <http://ClassLoaders.java>
>                 +++
>                 b/jdk/src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java
>                 <http://ClassLoaders.java>
>                 @@ -112,6 +112,15 @@ public static ClassLoader
>                 appClassLoader() {
>                 }
> 
>                 /**
>                 + * Returns a new isolated application module class loader.
>                 + *
>                 + * @return the new application module class loader (not
>                 {@code
>                 null})
>                 + */
>                 + public static ClassLoader appModuleClassLoader() {
>                 + return new AppModuleClassLoader(APP_LOADER);
>                 + }
>                 +
>                 + /**
>                 * The class loader that is used to find resources in modules
>                 defined to
>                 * the boot class loader. It is not used for class loading.
>                 */
>                 @@ -220,6 +229,21 @@ protected Package
>                 defineOrCheckPackage(String pn,
>                 Manifest man, URL url) {
>                 }
> 
>                 /**
>                 + * An isolated application module class loader that is
>                 a {@code
>                 BuiltinClassLoader} with
>                 + * the application class loader as parent.
>                 + */
>                 + private static class AppModuleClassLoader extends
>                 BuiltinClassLoader {
>                 + static {
>                 + if (!ClassLoader.registerAsParallelCapable())
>                 + throw new InternalError();
>                 + }
>                 +
>                 + AppModuleClassLoader(AppClassLoader parent) {
>                 + super("app", parent, null);
>                 + }
>                 + }
>                 +
>                 + /**
>                 * Returns a {@code URLClassPath} of file URLs to each of the
>                 elements in
>                 * the given class path.
>                 */
>                 diff --git
>                 a/jdk/src/java.base/share/classes/jdk/internal/module/ModuleLoaderMap.java
>                 <http://ModuleLoaderMap.java>
> 
>                 b/jdk/src/java.base/share/classes/jdk/internal/module/ModuleLoaderMap.java
>                 <http://ModuleLoaderMap.java>
> 
>                 index b0f465d..cc2bdb6 100644
>                 ---
>                 a/jdk/src/java.base/share/classes/jdk/internal/module/ModuleLoaderMap.java
>                 <http://ModuleLoaderMap.java>
> 
>                 +++
>                 b/jdk/src/java.base/share/classes/jdk/internal/module/ModuleLoaderMap.java
>                 <http://ModuleLoaderMap.java>
> 
>                 @@ -85,6 +85,8 @@ public ClassLoader apply(String name) {
>                 return platformModules;
>                 }
> 
>                 + private static final boolean ISOLATED_MODULE_PATH =
>                 Boolean.parseBoolean(System.getProperty("jdk.module.isolated",
> 
>                 "false"));
>                 +
>                 /**
>                 * Returns the function to map modules in the given
>                 configuration
>                 to the
>                 * built-in class loaders.
>                 @@ -102,6 +104,8 @@ public ClassLoader apply(String name) {
>                 if (!bootModules.contains(mn)) {
>                 if (platformModules.contains(mn)) {
>                 map.put(mn, platformClassLoader);
>                 + } else if (ISOLATED_MODULE_PATH) {
>                 + map.put(mn, ClassLoaders.appModuleClassLoader());
>                 } else {
>                 map.put(mn, appClassLoader);
>                 }
> 
> 
>                 -- 
>                 - DML
> 
> 
> 
> 
> 
> -- 
> Sent from my Android device with K-9 Mail. Please excuse my brevity.


-- 
- DML


More information about the jpms-spec-experts mailing list