Do not eager load non-eager module fragments.
diff --git a/platform/o.n.bootstrap/src/org/netbeans/FixedModule.java b/platform/o.n.bootstrap/src/org/netbeans/FixedModule.java index 9628d1a..9281473 100644 --- a/platform/o.n.bootstrap/src/org/netbeans/FixedModule.java +++ b/platform/o.n.bootstrap/src/org/netbeans/FixedModule.java
@@ -222,6 +222,11 @@ protected void classLoaderUp(Set<Module> parents) throws IOException { return; // no need } + + @Override + void releaseClassLoader() { + return; // don't touch it + } /** Turn off the classloader and release all resources. */ protected void classLoaderDown() {
diff --git a/platform/o.n.bootstrap/src/org/netbeans/Module.java b/platform/o.n.bootstrap/src/org/netbeans/Module.java index 5e9348f..45dffce 100644 --- a/platform/o.n.bootstrap/src/org/netbeans/Module.java +++ b/platform/o.n.bootstrap/src/org/netbeans/Module.java
@@ -605,6 +605,14 @@ void unregisterInstrumentation() { NbInstrumentation.unregisterAgent(instr); } + + /** + * Release references to the ClassLoader. Package-private for now as only (?) FixedModule + * should retain the CL instance. + */ + void releaseClassLoader() { + this.classloader = null; + } /** Struct representing a package exported from a module. * @since org.netbeans.core/1 > 1.4
diff --git a/platform/o.n.bootstrap/src/org/netbeans/ModuleManager.java b/platform/o.n.bootstrap/src/org/netbeans/ModuleManager.java index d41a168..3e15e51 100644 --- a/platform/o.n.bootstrap/src/org/netbeans/ModuleManager.java +++ b/platform/o.n.bootstrap/src/org/netbeans/ModuleManager.java
@@ -932,10 +932,12 @@ return; } for (Module inject : injectList) { - Util.err.log(Level.FINER, "Compat: injecting contents of fragment " + inject.getCodeNameBase() + " into " + m.getCodeNameBase()); - List<File> allJars = inject.getAllJars(); - // PENDING: shouldn't we add those jars first, so they take precedence ? - path.addAll(allJars); + if (isOrWillEnable(inject)) { + Util.err.log(Level.FINER, "Compat: injecting contents of fragment " + inject.getCodeNameBase() + " into " + m.getCodeNameBase()); + List<File> allJars = inject.getAllJars(); + // PENDING: shouldn't we add those jars first, so they take precedence ? + path.addAll(allJars); + } } } @@ -1253,6 +1255,34 @@ public void enable(Set<Module> modules) throws IllegalArgumentException, InvalidException { enable(modules, true); } + + /** + * Context of the pending 'enable' operation. Some calls go back to ModuleManager + * from other objects, that can't be compatibly passed 'willEnable' info. + */ + static class EnableContext { + final List<Module> willEnable; + + public EnableContext(List<Module> willEnable) { + this.willEnable = willEnable; + } + } + + private final ThreadLocal<EnableContext> enableContext = new ThreadLocal<>(); + + /** + * Checks if the module is enabled or WILL be enabled by the current enable operation. + * @param m module to check + * @return true, if the module is/will enable. + */ + boolean isOrWillEnable(Module m) { + if (m.isEnabled()) { + return true; + } + EnableContext ctx = enableContext.get(); + return ctx != null && ctx.willEnable.contains(m); + } + private void enable(Set<Module> modules, boolean honorAutoloadEager) throws IllegalArgumentException, InvalidException { assertWritable(); Util.err.log(Level.FINE, "enable: {0}", modules); @@ -1296,15 +1326,16 @@ ev.log(Events.START_ENABLE_MODULES, toEnable); netigso.willEnable(toEnable); + try { + enableContext.set(new EnableContext(toEnable)); + doEnable(toEnable); + } finally { + enableContext.remove(); + } + } + + private void doEnable(List<Module> toEnable) throws IllegalArgumentException, InvalidException { for (;;) { - // first connect fragments to their hosts, so classloaders are populated - for (Module m: toEnable) { - if (m.isEnabled()) { - continue; - } - // store information from fragment modules for early initialization of hosting classlaoder: - attachModuleFragment(m); - } // Actually turn on the listed modules. // List of modules that need to be "rolled back". LinkedList<Module> fallback = new LinkedList<Module>(); @@ -1518,7 +1549,14 @@ installer.dispose(m); m.setEnabled(false); m.unregisterInstrumentation(); - m.classLoaderDown(); + // do not down classloader for fragments, as they are shared with the + // hosting module. + if (m.getFragmentHostCodeName() == null) { + m.classLoaderDown(); + } + // release the classloader from the module; it will be created again by + // classLoaderUp. + m.releaseClassLoader(); } System.gc(); // hope OneModuleClassLoader.finalize() is called... System.runFinalization(); @@ -1572,10 +1610,15 @@ Collection<Module> fragments = getAttachedFragments(m); if (!fragments.isEmpty()) { for (Module frag : fragments) { - Set<Module> mods = calculateParents(frag); - mods.remove(m); - res.addAll(mods); + if (isOrWillEnable(frag)) { + Set<Module> mods = calculateParents(frag); + res.addAll(mods); + } } + // remove m and m's fragments from parent classloaders, as fragment + // jars are merged into m's own classloader already. + res.remove(m); + res.removeAll(fragments); } return res; } @@ -1769,7 +1812,9 @@ } Collection<Module> frags = getAttachedFragments(m); for (Module fragMod : frags) { - if (! fragMod.isEnabled()) { + // do not enable regular fragments unless eager: if something depends on a fragment, it will + // enable the fragment along with normal dependencies. + if (fragMod.isEager()) { maybeAddToEnableList(willEnable, mightEnable, fragMod, fragMod.isAutoload() || fragMod.isEager()); } } @@ -1793,6 +1838,13 @@ continue; } if (m.isEager()) { + if (m.getFragmentHostCodeName() != null) { + Module host = modulesByName.get(m.getFragmentHostCodeName()); + if (host == null || (!m.isEnabled() && !willEnable.contains(m))) { + // will not enable if its host is not enabled or will not be enabled + continue; + } + } if (couldBeEnabledWithEagers(m, willEnable, new HashSet<Module>())) { // Go for it! found = true;
diff --git a/platform/o.n.bootstrap/src/org/netbeans/Util.java b/platform/o.n.bootstrap/src/org/netbeans/Util.java index 5d687e9..395de60 100644 --- a/platform/o.n.bootstrap/src/org/netbeans/Util.java +++ b/platform/o.n.bootstrap/src/org/netbeans/Util.java
@@ -261,7 +261,7 @@ for (Dependency dep : m1.getDependenciesArray()) { if (dep.getType() == Dependency.TYPE_REQUIRES) { List<Module> providers = providersOf.get(dep.getName()); - + if (providers != null) { l = fillMapSlot(m, m1); l.addAll(providers); @@ -284,9 +284,10 @@ if (frags != null && !frags.isEmpty()) { frags = new HashSet<>(frags); frags.retainAll(modules); - + for (Module f : frags) { List<Module> fragmentDep = fillMapSlot(m, f); + // move fragment after its host module in the sort order fragmentDep.add(m1); for (Dependency dep : f.getDependenciesArray()) { if (dep.getType() == Dependency.TYPE_REQUIRES) { @@ -294,7 +295,6 @@ if (providers != null) { if (providers.contains(m1)) { providers = new ArrayList<>(providers); - providers.remove(m1); } l = fillMapSlot(m, m1); l.addAll(providers); @@ -313,6 +313,8 @@ } if (l != null) { l.remove(m1); + // remove fragments for m1 from m1's dependencies + l.removeAll(frags); } } if (l != null) {
diff --git a/platform/o.n.bootstrap/test/unit/data/jars/agent.mf b/platform/o.n.bootstrap/test/unit/data/jars/agent.mf index 9d7152b..10b1556 100644 --- a/platform/o.n.bootstrap/test/unit/data/jars/agent.mf +++ b/platform/o.n.bootstrap/test/unit/data/jars/agent.mf
@@ -2,3 +2,4 @@ OpenIDE-Module: org.agent/1 OpenIDE-Module-Name: Hello World Agent Agent-Class: org.agent.HelloWorldAgent +OpenIDE-Module-Public-Packages: org.agent.*
diff --git a/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-client.mf b/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-client.mf new file mode 100644 index 0000000..b96fde79 --- /dev/null +++ b/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-client.mf
@@ -0,0 +1,5 @@ +Manifest-Version: 1.0 +OpenIDE-Module: org.foo.javafx.client +OpenIDE-Module-Name: Foo JavaFX client +OpenIDE-Module-Public-Packages: - +OpenIDE-Module-Module-Dependencies: org.foo.javafx
diff --git a/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-core.mf b/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-core.mf new file mode 100644 index 0000000..fff41f6 --- /dev/null +++ b/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-core.mf
@@ -0,0 +1,5 @@ +Manifest-Version: 1.0 +OpenIDE-Module: org.foo.javafx +OpenIDE-Module-Name: Simulated Core Javafx package +OpenIDE-Module-Public-Packages: org.foo.javafx.* +OpenIDE-Module-Needs: org.openide.modules.jre.JavaFX
diff --git a/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-linux-eager.mf b/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-linux-eager.mf new file mode 100644 index 0000000..b22fd75 --- /dev/null +++ b/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-linux-eager.mf
@@ -0,0 +1,6 @@ +Manifest-Version: 1.0 +OpenIDE-Module: org.foo.javafx.linux.eager +OpenIDE-Module-Name: Simulated Linux Javafx package +OpenIDE-Module-Public-Packages: org.foo.javafx.* +OpenIDE-Module-Module-Dependencies: org.foo.javafx, org.foo.javafx.linux +OpenIDE-Module-Fragment-Host: org.foo.javafx
diff --git a/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-linux.mf b/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-linux.mf new file mode 100644 index 0000000..5c495ac --- /dev/null +++ b/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-linux.mf
@@ -0,0 +1,8 @@ +Manifest-Version: 1.0 +OpenIDE-Module: org.foo.javafx.linux +OpenIDE-Module-Name: Simulated Linux Javafx package +OpenIDE-Module-Public-Packages: org.foo.javafx.* +OpenIDE-Module-Requires: org.openide.modules.os.Linux +OpenIDE-Module-Provides: org.openide.modules.jre.JavaFX +OpenIDE-Module-Fragment-Host: org.foo.javafx +OpenIDE-Module-Module-Dependencies: org.foo.javafx
diff --git a/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-windows.mf b/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-windows.mf new file mode 100644 index 0000000..835b65e --- /dev/null +++ b/platform/o.n.bootstrap/test/unit/data/jars/foo-javafx-windows.mf
@@ -0,0 +1,8 @@ +Manifest-Version: 1.0 +OpenIDE-Module: org.foo.javafx.windows +OpenIDE-Module-Name: Simulated Windows Javafx package +OpenIDE-Module-Public-Packages: org.foo.javafx.* +OpenIDE-Module-Requires: org.openide.modules.os.Windows +OpenIDE-Module-Provides: org.openide.modules.jre.JavaFX +OpenIDE-Module-Fragment-Host: org.foo.javafx +OpenIDE-Module-Module-Dependencies: org.foo.javafx, org.agent/1
diff --git a/platform/o.n.bootstrap/test/unit/data/jars/javafx-core/org/foo/javafx/Bundle.properties b/platform/o.n.bootstrap/test/unit/data/jars/javafx-core/org/foo/javafx/Bundle.properties new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/platform/o.n.bootstrap/test/unit/data/jars/javafx-core/org/foo/javafx/Bundle.properties
@@ -0,0 +1 @@ +
diff --git a/platform/o.n.bootstrap/test/unit/data/jars/javafx-linux-eager/org/foo/javafx/Eager.properties b/platform/o.n.bootstrap/test/unit/data/jars/javafx-linux-eager/org/foo/javafx/Eager.properties new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/platform/o.n.bootstrap/test/unit/data/jars/javafx-linux-eager/org/foo/javafx/Eager.properties
@@ -0,0 +1 @@ +
diff --git a/platform/o.n.bootstrap/test/unit/data/jars/javafx-linux/org/foo/javafx/Linux.properties b/platform/o.n.bootstrap/test/unit/data/jars/javafx-linux/org/foo/javafx/Linux.properties new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/platform/o.n.bootstrap/test/unit/data/jars/javafx-linux/org/foo/javafx/Linux.properties
@@ -0,0 +1 @@ +
diff --git a/platform/o.n.bootstrap/test/unit/data/jars/javafx-windows/org/foo/javafx/Windows.properties b/platform/o.n.bootstrap/test/unit/data/jars/javafx-windows/org/foo/javafx/Windows.properties new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/platform/o.n.bootstrap/test/unit/data/jars/javafx-windows/org/foo/javafx/Windows.properties
@@ -0,0 +1 @@ +
diff --git a/platform/o.n.bootstrap/test/unit/src/org/netbeans/MockModuleInstaller.java b/platform/o.n.bootstrap/test/unit/src/org/netbeans/MockModuleInstaller.java index e62a382..9602fbb 100644 --- a/platform/o.n.bootstrap/test/unit/src/org/netbeans/MockModuleInstaller.java +++ b/platform/o.n.bootstrap/test/unit/src/org/netbeans/MockModuleInstaller.java
@@ -20,8 +20,10 @@ package org.netbeans; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; public class MockModuleInstaller extends ModuleInstaller { @@ -29,6 +31,7 @@ // For examining results of what happened: public final List<String> actions = new ArrayList<String>(); public final List<Object> args = new ArrayList<Object>(); + public final Map<String, String[]> provides = new HashMap<>(); public void clear() { actions.clear(); @@ -39,6 +42,11 @@ // For adding modules that don't want to close: public final Set<Module> wontclose = new HashSet<Module>(); + @Override + public String[] refineProvides(Module m) { + return provides.get(m.getCodeNameBase()); + } + public void prepare(Module m) throws InvalidException { if (delinquents.contains(m)) { throw new InvalidException(m, "not supposed to be installed");
diff --git a/platform/o.n.bootstrap/test/unit/src/org/netbeans/ModuleManagerTest.java b/platform/o.n.bootstrap/test/unit/src/org/netbeans/ModuleManagerTest.java index 3f33cd4..0681384 100644 --- a/platform/o.n.bootstrap/test/unit/src/org/netbeans/ModuleManagerTest.java +++ b/platform/o.n.bootstrap/test/unit/src/org/netbeans/ModuleManagerTest.java
@@ -2834,14 +2834,21 @@ MockEvents ev = new MockEvents(); ModuleManager mgr = new ModuleManager(installer, ev); mgr.mutexPrivileged().enterWriteAccess(); + + createTestJAR(data, jars, "fragment-module-reg", "fragment-module"); // m1 autoload, m2 normal, m3 eager Module m1 = mgr.create(new File(jars, "host-module.jar"), null, false, false, false); - Module m2 = mgr.create(new File(jars, "fragment-module.jar"), null, false, false, false); + Module m2 = mgr.create(new File(jars, "fragment-module-reg.jar"), null, false, false, false); + Module m3 = mgr.create(new File(jars, "fragment-module.jar"), null, false, false, true); List<Module> toEnable = mgr.simulateEnable(Collections.singleton(m2)); assertTrue("Host will be enabled", toEnable.contains(m1)); assertTrue("Known fragment must be merged in", toEnable.contains(m2)); + assertTrue("Eager fragment must be merged in", toEnable.contains(m3)); + + // cannot explicitly enable eager module: + toEnable.remove(m3); mgr.enable(new HashSet<>(toEnable)); } @@ -2911,7 +2918,7 @@ c.checkHostAndOtherFragmentsLoaded(c.fragmentAutoload); } - public void testAutoloadHostEnablesAllFragments() throws Exception { + public void testAutoloadHostEnablesEagerFragments() throws Exception { ModsCreator c = new ModsCreator(); createTestJAR(data, jars, "client-module-depend-host", "client-module"); c.loadModules(); @@ -2919,7 +2926,7 @@ Module client = c.mgr.create(new File(jars, "client-module-depend-host.jar"), null, false, false, false); c.mgr.enable(client); - c.checkHostAndOtherFragmentsLoaded(c.host); + c.checkHostAndOtherFragmentsLoaded(); } public void testBrokenAutoloadFragmentDepend() throws Exception { @@ -2969,6 +2976,92 @@ assertFalse(mgr.getEnabledModules().contains(fragment)); assertFalse(mgr.getEnabledModules().contains(client)); } + + /** + * Tests the situation with JavaFX support: + * - client depends on core module + * - core module NEEDS or REQUIRES a platform module + * - platform modules are AUTOLOADS and are fragments + * - noone depends on platform modules directly + * - each platform module REQUIRES a token + * - one token is provided + * Under normal circumstances, the autoload fragments would not load since noone depends + * on them. But they are providers, and SOME provider is needed. Hence the matching one(s) + * will load. + * + * @throws Exception + */ + public void testPickFromAutoloadFragmentsByToken() throws Exception { + MockModuleInstaller installer = new MockModuleInstaller(); + installer.provides.put("org.foo.javafx", new String[] { "org.openide.modules.os.Linux", "org.openide.modules.os.Unix" }); + MockEvents ev = new MockEvents(); + ModuleManager mgr = new ModuleManager(installer, ev); + mgr.mutexPrivileged().enterWriteAccess(); + + createTestJAR(data, jars, "foo-javafx-core", "javafx-core"); + createTestJAR(data, jars, "foo-javafx-linux", "javafx-linux"); + createTestJAR(data, jars, "foo-javafx-windows", "javafx-windows"); + createTestJAR(data, jars, "foo-javafx-client", "client-module"); + createTestJAR(data, jars, "foo-javafx-linux-eager", "javafx-linux-eager"); + + Module host = mgr.create(new File(jars, "foo-javafx-core.jar"), null, false, true, false); + Module linuxFrag = mgr.create(new File(jars, "foo-javafx-linux.jar"), null, false, true, false); + Module linuxEager = mgr.create(new File(jars, "foo-javafx-linux-eager.jar"), null, false, false, true); + Module winFrag = mgr.create(new File(jars, "foo-javafx-windows.jar"), null, false, true, false); + Module client = mgr.create(new File(jars, "foo-javafx-client.jar"), null, false, false, false); + + mgr.enable(client); + + assertTrue(mgr.getEnabledModules().contains(host)); + assertTrue(mgr.getEnabledModules().contains(linuxFrag)); + assertTrue(mgr.getEnabledModules().contains(linuxEager)); + + assertFalse(mgr.getEnabledModules().contains(winFrag)); + assertTrue(mgr.getEnabledModules().contains(client)); + + assertNotNull(client.getClassLoader().getResource("org/foo/javafx/Bundle.properties")); + assertNotNull(client.getClassLoader().getResource("org/foo/javafx/Linux.properties")); + assertNull(client.getClassLoader().getResource("org/foo/javafx/Windows.properties")); + assertNotNull(client.getClassLoader().getResource("org/foo/javafx/Eager.properties")); + } + + /** + * Checks that dependencies introduced by the fragment are injected into the + * host module. + * @throws Exception + */ + public void testFragmentDependenciesInjectedIntoMain() throws Exception { + MockModuleInstaller installer = new MockModuleInstaller(); + installer.provides.put("org.foo.javafx", new String[] { "org.openide.modules.os.Windows" }); + MockEvents ev = new MockEvents(); + ModuleManager mgr = new ModuleManager(installer, ev); + mgr.mutexPrivileged().enterWriteAccess(); + + createTestJAR(data, jars, "foo-javafx-core", "javafx-core"); + createTestJAR(data, jars, "foo-javafx-linux", "javafx-linux"); + createTestJAR(data, jars, "foo-javafx-windows", "javafx-windows"); + createTestJAR(data, jars, "foo-javafx-client", "client-module"); + createTestJAR(data, jars, "foo-javafx-linux-eager", "javafx-linux-eager"); + createTestJAR(data, jars, "agent", "agent"); + + Module host = mgr.create(new File(jars, "foo-javafx-core.jar"), null, false, true, false); + Module linuxFrag = mgr.create(new File(jars, "foo-javafx-linux.jar"), null, false, true, false); + Module linuxEager = mgr.create(new File(jars, "foo-javafx-linux-eager.jar"), null, false, false, true); + Module winFrag = mgr.create(new File(jars, "foo-javafx-windows.jar"), null, false, true, false); + Module client = mgr.create(new File(jars, "foo-javafx-client.jar"), null, false, false, false); + Module agent = mgr.create(new File(jars, "agent.jar"), null, false, true, false); + + mgr.enable(client); + + assertTrue(mgr.getEnabledModules().contains(host)); + assertFalse(mgr.getEnabledModules().contains(linuxFrag)); + assertFalse(mgr.getEnabledModules().contains(linuxEager)); + assertTrue(mgr.getEnabledModules().contains(winFrag)); + assertTrue(mgr.getEnabledModules().contains(client)); + + assertNotNull(host.getClassLoader().getResource("org/agent/HelloWorld.class")); + assertNull(client.getClassLoader().getResource("org/agent/HelloWorld.class")); + } private class LogHandler extends Handler { private List<LogRecord> warnings = new ArrayList<>(); @@ -3016,7 +3109,7 @@ host = mgr.create(new File(jars, "host-module.jar"), null, false, true, false); } if (fragmentService == null) { - fragmentService = mgr.create(new File(jars, "fragment-module.jar"), null, false, true, false); + fragmentService = mgr.create(new File(jars, "fragment-module.jar"), null, false, false, true); } if (fragmentRegular == null) { fragmentRegular = mgr.create(new File(jars, "fragment-module-reg.jar"), null, false, false, false); @@ -3032,13 +3125,17 @@ liveFragments.add(fragmentAutoload); } - void checkHostAndOtherFragmentsLoaded(Module pickedDep) { + void checkHostAndOtherFragmentsLoaded(Module... pickedDeps) { assertTrue("Fragment host must enable", mgr.getEnabledModules().contains(host)); + assertTrue("Eager fragment must load with host", mgr.getEnabledModules().contains(fragmentService)); + Set<Module> picked = new HashSet<>(pickedDeps == null ? Collections.emptyList() : Arrays.asList(pickedDeps)); for (Module m : liveFragments) { - if (m != pickedDep) { - assertTrue("Peer fragment must be autoloaded", mgr.getEnabledModules().contains(m)); + if (picked.contains(m)) { + assertTrue("Peer fragment must be loaded: " + m.getCodeNameBase(), mgr.getEnabledModules().contains(m)); + } else if (m != fragmentService) { + assertFalse("Fragment must not activate: " + m.getCodeNameBase(), mgr.getEnabledModules().contains(m)); } - } + } // the fragment with unsatisfied "needs" is not reported, as it is autoload being triggered by host module. assertTrue(logHandler.warnings.isEmpty()); }