diff options
10 files changed, 154 insertions, 55 deletions
diff --git a/bundles/org.eclipse.osgi.tests/bundles_src/storage.hooks.a/org/eclipse/osgi/tests/hooks/framework/storage/a/TestHookConfigurator.java b/bundles/org.eclipse.osgi.tests/bundles_src/storage.hooks.a/org/eclipse/osgi/tests/hooks/framework/storage/a/TestHookConfigurator.java index 57eb3e108..3f68e6690 100644 --- a/bundles/org.eclipse.osgi.tests/bundles_src/storage.hooks.a/org/eclipse/osgi/tests/hooks/framework/storage/a/TestHookConfigurator.java +++ b/bundles/org.eclipse.osgi.tests/bundles_src/storage.hooks.a/org/eclipse/osgi/tests/hooks/framework/storage/a/TestHookConfigurator.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2013, 2016 IBM Corporation and others. + * Copyright (c) 2013, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -63,6 +63,8 @@ public class TestHookConfigurator implements HookConfigurator { public ModuleRevisionBuilder adaptModuleRevisionBuilder(ModuleEvent operation, Module origin, ModuleRevisionBuilder builder) { if (TestHookConfigurator.replaceModuleBuilder) { ModuleRevisionBuilder replace = new ModuleRevisionBuilder(); + // try setting the ID to something which is checked during the test + replace.setId(5678); replace.setSymbolicName("replace"); replace.setVersion(Version.parseVersion("1.1.1")); replace.addCapability("replace", Collections.<String, String> emptyMap(), Collections.<String, Object> emptyMap()); @@ -71,6 +73,8 @@ public class TestHookConfigurator implements HookConfigurator { return replace; } if (TestHookConfigurator.adaptManifest) { + // try setting the ID to something which is checked during the test + builder.setId(5678); Map<String, String> dirs = Collections.emptyMap(); Map<String, Object> attrs = new HashMap<String, Object>(); attrs.put("test.file.path", getGeneration().getContent().getPath() + " - " + adaptCount.getAndIncrement()); diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/AbstractTest.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/AbstractTest.java index 846d8b97a..2b06d17b5 100644 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/AbstractTest.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/AbstractTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2013, 2016 IBM Corporation and others. + * Copyright (c) 2013, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -87,7 +87,14 @@ public abstract class AbstractTest { } protected Module installDummyModule(Map<String, String> manifest, String location, ModuleContainer container) throws BundleException { + return installDummyModule(manifest, -1, location, container); + } + + protected Module installDummyModule(Map<String, String> manifest, long id, String location, ModuleContainer container) throws BundleException { ModuleRevisionBuilder builder = OSGiManifestBuilderFactory.createBuilder(manifest); + if (id > 0) { + builder.setId(id); + } Module system = container.getModule(0); return container.install(system, location, builder, null); } diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java index 884901693..553819d87 100644 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java @@ -3274,6 +3274,45 @@ public class TestModuleContainer extends AbstractTest { Assert.assertNull("Failed to resolve test.", report.getResolutionException()); } + @Test + public void testModuleIDSetting() throws BundleException, IOException { + DummyContainerAdaptor adaptor = createDummyAdaptor(); + ModuleContainer container = adaptor.getContainer(); + + // install the system.bundle + Module systemBundle = installDummyModule("system.bundle.MF", Constants.SYSTEM_BUNDLE_LOCATION, Constants.SYSTEM_BUNDLE_SYMBOLICNAME, null, null, container); + ResolutionReport report = container.resolve(Arrays.asList(systemBundle), true); + Assert.assertNull("Failed to resolve system.bundle.", report.getResolutionException()); + + Map<String, String> manifest = new HashMap<String, String>(); + + // test by installing bundles with decreasing IDs + List<Module> modules = new ArrayList<Module>(); + for (int i = 5; i > 0; i--) { + manifest.clear(); + manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); + manifest.put(Constants.BUNDLE_SYMBOLICNAME, String.valueOf(i)); + modules.add(installDummyModule(manifest, i, manifest.get(Constants.BUNDLE_SYMBOLICNAME), container)); + } + + // test that the modules have decreasing ID starting at 5 + long id = 5; + for (Module module : modules) { + Assert.assertEquals("Wrong ID found.", id--, module.getId().longValue()); + } + + // test that error occurs when trying to use an existing ID + manifest.clear(); + manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2"); + manifest.put(Constants.BUNDLE_SYMBOLICNAME, String.valueOf("test.dup.id")); + try { + installDummyModule(manifest, 5, manifest.get(Constants.BUNDLE_SYMBOLICNAME), container); + fail("Expected to fail installation with duplicate ID."); + } catch (IllegalStateException e) { + // expected + } + } + private static void assertWires(List<ModuleWire> required, List<ModuleWire>... provided) { for (ModuleWire requiredWire : required) { for (List<ModuleWire> providedList : provided) { diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/hooks/framework/StorageHookTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/hooks/framework/StorageHookTests.java index 67556d945..ad52a358f 100644 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/hooks/framework/StorageHookTests.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/hooks/framework/StorageHookTests.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2013, 2016 IBM Corporation and others. + * Copyright (c) 2013, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -147,6 +147,7 @@ public class StorageHookTests extends AbstractFrameworkHookTests { installBundle(); Bundle b = framework.getBundleContext().getBundle(location); + assertNotEquals("Wrong ID.", 5678, b.getBundleId()); assertNotNull("Missing test bundle.", b); List<Capability> testCaps = b.adapt(BundleRevision.class).getCapabilities("test.file.path"); assertEquals("Wrong number of test caps.", 1, testCaps.size()); @@ -158,6 +159,7 @@ public class StorageHookTests extends AbstractFrameworkHookTests { assertEquals("Wrong origin", framework.getBundleContext().getBundle().getLocation(), location1); b.update(); + assertNotEquals("Wrong ID.", 5678, b.getBundleId()); testCaps = b.adapt(BundleRevision.class).getCapabilities("test.file.path"); assertEquals("Wrong number of test caps.", 1, testCaps.size()); String path2 = (String) testCaps.get(0).getAttributes().get("test.file.path"); @@ -176,6 +178,7 @@ public class StorageHookTests extends AbstractFrameworkHookTests { framework = createFramework(configuration); framework.start(); b = framework.getBundleContext().getBundle(location); + assertNotEquals("Wrong ID.", 5678, b.getBundleId()); testCaps = b.adapt(BundleRevision.class).getCapabilities("test.file.path"); assertEquals("Wrong number of test caps.", 1, testCaps.size()); path2 = (String) testCaps.get(0).getAttributes().get("test.file.path"); @@ -190,6 +193,7 @@ public class StorageHookTests extends AbstractFrameworkHookTests { b.uninstall(); installBundle(); b = framework.getBundleContext().getBundle(location); + assertNotEquals("Wrong ID.", 5678, b.getBundleId()); assertNotNull("Missing test bundle.", b); assertEquals("Wrong BSN.", "replace", b.getSymbolicName()); testCaps = b.adapt(BundleRevision.class).getCapabilities("replace"); diff --git a/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF b/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF index 8447cd538..501cb6902 100644 --- a/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF +++ b/bundles/org.eclipse.osgi/META-INF/MANIFEST.MF @@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2 Export-Package: org.eclipse.core.runtime.adaptor;x-friends:="org.eclipse.core.runtime", org.eclipse.core.runtime.internal.adaptor;x-internal:=true, org.eclipse.equinox.log;version="1.0";uses:="org.osgi.framework,org.osgi.service.log", - org.eclipse.osgi.container;version="1.2"; + org.eclipse.osgi.container;version="1.3"; uses:="org.eclipse.osgi.report.resolution, org.osgi.framework.wiring, org.osgi.framework.startlevel, diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java index 802b9f6a1..5756be216 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2012, 2016 IBM Corporation and others. + * Copyright (c) 2012, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -196,8 +196,11 @@ public final class ModuleContainer implements DebugOptionsListener { * @throws BundleException if some error occurs installing the module */ public Module install(Module origin, String location, ModuleRevisionBuilder builder, Object revisionInfo) throws BundleException { + long id = builder.getId(); ModuleRevisionBuilder adaptBuilder = getAdaptor().adaptModuleRevisionBuilder(ModuleEvent.INSTALLED, origin, builder, revisionInfo); if (adaptBuilder != null) { + // be sure to restore the id from the original builder + adaptBuilder.setInternalId(id); builder = adaptBuilder; } String name = builder.getSymbolicName(); @@ -288,8 +291,11 @@ public final class ModuleContainer implements DebugOptionsListener { * @throws BundleException if some error occurs updating the module */ public void update(Module module, ModuleRevisionBuilder builder, Object revisionInfo) throws BundleException { + long id = builder.getId(); ModuleRevisionBuilder adaptBuilder = getAdaptor().adaptModuleRevisionBuilder(ModuleEvent.UPDATED, module, builder, revisionInfo); if (adaptBuilder != null) { + // be sure to restore the id from the original builder + adaptBuilder.setInternalId(id); builder = adaptBuilder; } checkAdminPermission(module.getBundle(), AdminPermission.LIFECYCLE); diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleDatabase.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleDatabase.java index 001e020dc..56518e72a 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleDatabase.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleDatabase.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2012, 2016 IBM Corporation and others. + * Copyright (c) 2012, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -200,7 +200,14 @@ public class ModuleDatabase { writeLock(); try { int startlevel = Constants.SYSTEM_BUNDLE_LOCATION.equals(location) ? 0 : getInitialModuleStartLevel(); - long id = Constants.SYSTEM_BUNDLE_LOCATION.equals(location) ? 0 : getNextIdAndIncrement(); + long id = Constants.SYSTEM_BUNDLE_LOCATION.equals(location) ? 0 : builder.getId(); + if (id == -1) { + // the id is not set by the builder; get and increment the next ID + id = getAndIncrementNextId(); + } + if (getModule(id) != null) { + throw new IllegalStateException("Duplicate module id: " + id + " used by module: " + getModule(id)); //$NON-NLS-1$//$NON-NLS-2$ + } EnumSet<Settings> settings = getActivationPolicySettings(builder); Module module = load(location, builder, revisionInfo, id, settings, startlevel); long currentTime = System.currentTimeMillis(); @@ -634,23 +641,13 @@ public class ModuleDatabase { return moduleCycles; } - /** - * Increments by one the next module ID - */ - private long getNextIdAndIncrement() { - // sanity check - checkWrite(); - return nextId.getAndIncrement(); - - } - private void checkWrite() { if (monitor.getWriteHoldCount() == 0) throw new IllegalMonitorStateException("Must hold the write lock."); //$NON-NLS-1$ } /** - * returns the next module ID + * returns the next module ID. * <p> * A read operation protected by the {@link #readLock() read} lock. * @return the next module ID @@ -665,6 +662,22 @@ public class ModuleDatabase { } /** + * Atomically increments by one the next module ID. + * <p> + * A write operation protected by the {@link #writeLock()} lock. + * @return the previous module ID + * @since 3.13 + */ + public final long getAndIncrementNextId() { + writeLock(); + try { + return nextId.getAndIncrement(); + } finally { + writeUnlock(); + } + } + + /** * Returns the current timestamp for the revisions of this database. * The timestamp is incremented any time a modification * is made to the revisions in this database. For example: diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleRevisionBuilder.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleRevisionBuilder.java index ebad63049..412e5371d 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleRevisionBuilder.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleRevisionBuilder.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2012, 2016 IBM Corporation and others. + * Copyright (c) 2012, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -75,6 +75,7 @@ public final class ModuleRevisionBuilder { private int types = 0; private final List<GenericInfo> capabilityInfos = new ArrayList<>(); private final List<GenericInfo> requirementInfos = new ArrayList<>(); + private long id = -1; /** * Constructs a new module builder @@ -108,6 +109,33 @@ public final class ModuleRevisionBuilder { } /** + * Sets the module ID for the builder. + * <p> + * This module ID will be used if this builder is used to + * {@link ModuleContainer#install(Module, String, ModuleRevisionBuilder, Object) install} + * a module. If the ID is not set then a module ID will be generated by the module + * container at install time. If a module already exists with the specified ID + * then an error will occur when attempting to install a new module with this + * builder. + * <p> + * Note that the system module with location {@link Constants#SYSTEM_BUNDLE_LOCATION} + * always gets module ID of zero. The builder for the system module is not + * asked to provide the module ID for the system module at install time. + * @param id the module ID to use. Must be >= 1. + * @since 3.13 + */ + public void setId(long id) { + if (id < 1) { + throw new IllegalArgumentException("ID must be >=1."); //$NON-NLS-1$ + } + this.id = id; + } + + void setInternalId(long id) { + this.id = id; + } + + /** * Adds a capability to this builder using the specified namespace, directives and attributes * @param namespace the namespace of the capability * @param directives the directives of the capability @@ -168,6 +196,18 @@ public final class ModuleRevisionBuilder { } /** + * Returns the module id for this builder. A value of -1 + * indicates that the module ID will be generated by the + * module container at {@link ModuleContainer#install(Module, String, ModuleRevisionBuilder, Object) install} + * time. + * @return the module id for this builder. + * @since 3.13 + */ + public long getId() { + return id; + } + + /** * Used by the container to build a new revision for a module. * This builder is used to build a new {@link Module#getCurrentRevision() current} * revision for the specified module. diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java index 85c9ecd0a..6aa42af98 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java @@ -14,7 +14,6 @@ import java.io.*; import java.net.*; import java.security.*; import java.util.*; -import java.util.concurrent.TimeUnit; import org.eclipse.core.runtime.adaptor.EclipseStarter; import org.eclipse.osgi.container.*; import org.eclipse.osgi.container.ModuleRevisionBuilder.GenericInfo; @@ -530,20 +529,22 @@ public class Storage { boolean isReference = in instanceof ReferenceInputStream; File staged = stageContent(in, sourceURL); Generation generation = null; - Long lockedID = getNextRootID(); try { - BundleInfo info = new BundleInfo(this, lockedID, bundleLocation, 0); + Long nextID = moduleDatabase.getAndIncrementNextId(); + BundleInfo info = new BundleInfo(this, nextID, bundleLocation, 0); generation = info.createGeneration(); - File contentFile = getContentFile(staged, isReference, lockedID, generation.getGenerationId()); + File contentFile = getContentFile(staged, isReference, nextID, generation.getGenerationId()); generation.setContent(contentFile, isReference); // Check that we can open the bundle file generation.getBundleFile().open(); setStorageHooks(generation); ModuleRevisionBuilder builder = getBuilder(generation); + builder.setId(nextID); + Module m = moduleContainer.install(origin, bundleLocation, builder, generation); - if (!lockedID.equals(m.getId())) { + if (!nextID.equals(m.getId())) { // this revision is already installed. delete the generation generation.delete(); return (Generation) m.getCurrentRevision().getRevisionInfo(); @@ -577,7 +578,6 @@ public class Storage { if (generation != null) { generation.getBundleInfo().unlockGeneration(generation); } - idLocks.unlock(lockedID); } } @@ -875,33 +875,6 @@ public class Storage { } } - private Long getNextRootID() throws BundleException { - // Try up to 500 times - for (int i = 0; i < 500; i++) { - moduleDatabase.readLock(); - try { - Long nextID = moduleDatabase.getNextId(); - try { - if (idLocks.tryLock(nextID, 0, TimeUnit.SECONDS)) { - return nextID; - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new BundleException("Failed to obtain id locks for installation.", BundleException.STATECHANGE_ERROR, e); //$NON-NLS-1$ - } - } finally { - moduleDatabase.readUnlock(); - } - // sleep to allow another thread to get the database lock - try { - Thread.sleep(50); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - } - } - throw new BundleException("Failed to obtain id locks for installation.", BundleException.STATECHANGE_ERROR); //$NON-NLS-1$ - } - /** * Attempts to set the permissions of the file in a system dependent way. * @param file the file to set the permissions on diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/MRUBundleFileList.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/MRUBundleFileList.java index acdcf4fdc..e105c0d75 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/MRUBundleFileList.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/MRUBundleFileList.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2005, 2016 IBM Corporation and others. + * Copyright (c) 2005, 2017 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -12,7 +12,9 @@ package org.eclipse.osgi.storage.bundlefile; import java.io.IOException; +import java.util.Collections; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; import org.eclipse.osgi.framework.eventmgr.*; /** @@ -40,14 +42,15 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle // used to work around bug 275166 private boolean firstDispatch = true; + private AtomicInteger pending = new AtomicInteger(); + public MRUBundleFileList(int fileLimit) { // only enable the MRU if the initFileLimit is > MIN this.fileLimit = fileLimit; if (fileLimit >= MIN) { this.bundleFileList = new BundleFile[fileLimit]; this.useStampList = new long[fileLimit]; - this.bundleFileCloser = new CopyOnWriteIdentityMap<>(); - this.bundleFileCloser.put(this, this); + this.bundleFileCloser = Collections.<Object, Object> singletonMap(this, this); } else { this.bundleFileList = null; this.useStampList = null; @@ -173,12 +176,22 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle // TODO should log ?? } finally { closingBundleFile.set(null); + pending.decrementAndGet(); } } private void closeBundleFile(BundleFile toRemove, EventManager manager) { if (toRemove == null) return; + int pendingNum = pending.incrementAndGet(); + if (pendingNum > fileLimit) { + // delay to allow the closer to catchup + try { + Thread.sleep(Math.min(500, pendingNum)); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } try { /* queue to hold set of listeners */ ListenerQueue<Object, Object, BundleFile> queue = new ListenerQueue<>(manager); |