Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Watson2016-02-15 20:12:19 +0000
committerThomas Watson2016-02-15 21:44:56 +0000
commit33c7f42010a0bd10d01fb0eaf3c7a6db43e64b74 (patch)
tree5dc71e66731c6bcbf69c1db9e6b643aabe066516
parentd2699fce816bd389b5c17cd54aa20a44f63f1473 (diff)
downloadrt.equinox.framework-33c7f42010a0bd10d01fb0eaf3c7a6db43e64b74.tar.gz
rt.equinox.framework-33c7f42010a0bd10d01fb0eaf3c7a6db43e64b74.tar.xz
rt.equinox.framework-33c7f42010a0bd10d01fb0eaf3c7a6db43e64b74.zip
Bug 487842 - Unable to acquire state change lock for the module whenI20160216-1400I20160216-0800
starting a bundle Change-Id: Ibc7c58aaf610cbd7b0c2aaef154559e2ed7134c4 Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
-rw-r--r--bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java96
-rw-r--r--bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/dummys/DummyContainerAdaptor.java14
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/Module.java32
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java49
4 files changed, 156 insertions, 35 deletions
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 c1a7acb6f..efb7eda25 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
@@ -12,6 +12,7 @@ package org.eclipse.osgi.tests.container;
import static java.util.jar.Attributes.Name.MANIFEST_VERSION;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
import java.io.*;
import java.util.*;
@@ -2366,6 +2367,101 @@ public class TestModuleContainer extends AbstractTest {
}
}
+ @Test
+ public void testStartOnResolve() 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());
+ systemBundle.start();
+
+ // install a bunch of modules
+ Map<String, String> manifest = new HashMap<String, String>();
+ List<Module> modules = new ArrayList<Module>();
+ for (int i = 0; i < 5; i++) {
+ manifest.clear();
+ manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+ manifest.put(Constants.BUNDLE_SYMBOLICNAME, "module." + i);
+ manifest.put(Constants.IMPORT_PACKAGE, "export");
+ Module module = installDummyModule(manifest, manifest.get(Constants.BUNDLE_SYMBOLICNAME), container);
+ try {
+ module.start();
+ fail("expected a bundle exception.");
+ } catch (BundleException e) {
+ // do nothing
+ }
+ modules.add(module);
+ }
+
+ manifest.clear();
+ manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+ manifest.put(Constants.BUNDLE_SYMBOLICNAME, "exporter");
+ manifest.put(Constants.EXPORT_PACKAGE, "export");
+ installDummyModule(manifest, manifest.get(Constants.BUNDLE_SYMBOLICNAME), container);
+
+ report = container.resolve(Collections.<Module> emptySet(), false);
+ Assert.assertNull("Found a error.", report.getResolutionException());
+
+ for (Module module : modules) {
+ Assert.assertEquals("Wrong state.", State.ACTIVE, module.getState());
+ }
+ }
+
+ @Test
+ public void testResolveDeadlock() throws BundleException, IOException, InterruptedException {
+ 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());
+ systemBundle.start();
+
+ // install a bunch of modules
+ Map<String, String> manifest = new HashMap<String, String>();
+ List<Module> modules = new ArrayList<Module>();
+ for (int i = 0; i < 5; i++) {
+ manifest.clear();
+ manifest.put(Constants.BUNDLE_MANIFESTVERSION, "2");
+ manifest.put(Constants.BUNDLE_SYMBOLICNAME, "module." + i);
+ modules.add(installDummyModule(manifest, manifest.get(Constants.BUNDLE_SYMBOLICNAME), container));
+ }
+ adaptor.setSlowdownEvents(true);
+ final ConcurrentLinkedQueue<BundleException> startErrors = new ConcurrentLinkedQueue<BundleException>();
+ final ExecutorService executor = Executors.newFixedThreadPool(10);
+ try {
+ for (final Module module : modules) {
+
+ executor.execute(new Runnable() {
+
+ @Override
+ public void run() {
+ try {
+ module.start();
+ } catch (BundleException e) {
+ startErrors.offer(e);
+ e.printStackTrace();
+ }
+ }
+ });
+ }
+ } finally {
+ executor.shutdown();
+ executor.awaitTermination(5, TimeUnit.MINUTES);
+ systemBundle.stop();
+ }
+
+ Assert.assertNull("Found a error.", startErrors.poll());
+ List<DummyContainerEvent> events = adaptor.getDatabase().getContainerEvents();
+ for (DummyContainerEvent event : events) {
+ Assert.assertNotEquals("Found an error.", ContainerEvent.ERROR, event.type);
+ }
+ }
+
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/container/dummys/DummyContainerAdaptor.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/dummys/DummyContainerAdaptor.java
index 645004e35..630b2f43e 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/dummys/DummyContainerAdaptor.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/dummys/DummyContainerAdaptor.java
@@ -13,6 +13,7 @@ package org.eclipse.osgi.tests.container.dummys;
import java.util.EnumSet;
import java.util.Map;
import java.util.concurrent.Executor;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.eclipse.osgi.container.*;
import org.eclipse.osgi.container.Module.Settings;
import org.eclipse.osgi.service.debug.DebugOptions;
@@ -22,6 +23,7 @@ import org.osgi.framework.FrameworkListener;
import org.osgi.framework.hooks.resolver.ResolverHookFactory;
public class DummyContainerAdaptor extends ModuleContainerAdaptor {
+ private AtomicBoolean slowdownEvents = new AtomicBoolean(false);
private final ModuleCollisionHook collisionHook;
private final Map<String, String> configuration;
private final DummyModuleDatabase moduleDatabase;
@@ -86,8 +88,20 @@ public class DummyContainerAdaptor extends ModuleContainerAdaptor {
return moduleDatabase;
}
+ public void setSlowdownEvents(boolean slowdown) {
+ slowdownEvents.set(slowdown);
+ }
+
@Override
public void publishModuleEvent(ModuleEvent type, Module module, Module origin) {
+ if (type == ModuleEvent.STARTING && slowdownEvents.get()) {
+ try {
+ Thread.sleep(6000);
+ } catch (InterruptedException e) {
+ // ignore
+ Thread.currentThread().interrupt();
+ }
+ }
moduleDatabase.addEvent(new DummyModuleEvent(module, type, module.getState()));
}
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/Module.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/Module.java
index 2bd9e9fb5..868e9b937 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/Module.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/Module.java
@@ -13,6 +13,7 @@ package org.eclipse.osgi.container;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.osgi.container.ModuleContainerAdaptor.ModuleEvent;
import org.eclipse.osgi.internal.container.EquinoxReentrantLock;
import org.eclipse.osgi.internal.debug.Debug;
@@ -159,7 +160,7 @@ public abstract class Module implements BundleReference, BundleStartLevel, Compa
final EquinoxReentrantLock stateChangeLock = new EquinoxReentrantLock();
private final EnumSet<ModuleEvent> stateTransitionEvents = EnumSet.noneOf(ModuleEvent.class);
private final EnumSet<Settings> settings;
- private final ThreadLocal<Boolean> inStartResolve = new ThreadLocal<Boolean>();
+ private final AtomicInteger inStartResolve = new AtomicInteger(0);
private volatile State state = State.INSTALLED;
private volatile int startlevel;
private volatile long lastModified;
@@ -401,18 +402,22 @@ public abstract class Module implements BundleReference, BundleStartLevel, Compa
if (State.ACTIVE.equals(getState()))
return;
if (getState().equals(State.INSTALLED)) {
- // must unlock to avoid out of order locks when multiple unresolved
- // bundles are started at the same time from different threads
- unlockStateChange(ModuleEvent.STARTED);
- lockedStarted = false;
ResolutionReport report;
+ inStartResolve.incrementAndGet();
try {
- inStartResolve.set(Boolean.TRUE);
- report = getRevisions().getContainer().resolve(Arrays.asList(this), true);
+ // must unlock to avoid out of order locks when multiple unresolved
+ // bundles are started at the same time from different threads
+ unlockStateChange(ModuleEvent.STARTED);
+ lockedStarted = false;
+ try {
+
+ report = getRevisions().getContainer().resolve(Arrays.asList(this), true);
+ } finally {
+ lockStateChange(ModuleEvent.STARTED);
+ lockedStarted = true;
+ }
} finally {
- inStartResolve.set(Boolean.FALSE);
- lockStateChange(ModuleEvent.STARTED);
- lockedStarted = true;
+ inStartResolve.decrementAndGet();
}
// need to check valid again in case someone uninstalled the bundle
checkValid();
@@ -440,7 +445,6 @@ public abstract class Module implements BundleReference, BundleStartLevel, Compa
event = ModuleEvent.STOPPED;
}
} finally {
- inStartResolve.set(Boolean.FALSE);
if (lockedStarted) {
unlockStateChange(ModuleEvent.STARTED);
}
@@ -689,10 +693,6 @@ public abstract class Module implements BundleReference, BundleStartLevel, Compa
}
final boolean inStartResolve() {
- Boolean value = inStartResolve.get();
- if (value == null) {
- return false;
- }
- return value.booleanValue();
+ return inStartResolve.get() > 0;
}
}
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 248d98d61..c7e02ab7c 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
@@ -623,9 +623,17 @@ public final class ModuleContainer implements DebugOptionsListener {
// lock.
for (Module module : modulesResolved) {
try {
+ // avoid grabbing the lock if the timestamp has changed
+ if (timestamp != moduleDatabase.getRevisionsTimestamp()) {
+ return false; // need to try again
+ }
module.lockStateChange(ModuleEvent.RESOLVED);
modulesLocked.add(module);
} catch (BundleException e) {
+ // before throwing an exception here, see if the timestamp has changed
+ if (timestamp != moduleDatabase.getRevisionsTimestamp()) {
+ return false; // need to try again
+ }
// TODO throw some appropriate exception
throw new IllegalStateException(Msg.ModuleContainer_StateLockError, e);
}
@@ -693,36 +701,39 @@ public final class ModuleContainer implements DebugOptionsListener {
Set<Module> triggerSet = restartTriggers ? new HashSet<Module>(triggers) : Collections.<Module> emptySet();
if (restartTriggers) {
for (Module module : triggers) {
- try {
- if (module.getId() != 0 && Module.RESOLVED_SET.contains(module.getState())) {
- secureAction.start(module, StartOptions.TRANSIENT);
- }
- } catch (BundleException e) {
- adaptor.publishContainerEvent(ContainerEvent.ERROR, module, e);
- } catch (IllegalStateException e) {
- // been uninstalled
- continue;
+ if (module.getId() != 0 && Module.RESOLVED_SET.contains(module.getState())) {
+ start(module, StartOptions.TRANSIENT);
}
}
}
// This is questionable behavior according to the spec but this was the way equinox previously behaved
// Need to auto-start any persistently started bundles that got resolved
for (Module module : modulesLocked) {
- if (module.inStartResolve() || module.getId() == 0 || triggerSet.contains(module)) {
- continue;
- }
- try {
- secureAction.start(module, StartOptions.TRANSIENT_IF_AUTO_START, StartOptions.TRANSIENT_RESUME);
- } catch (BundleException e) {
- adaptor.publishContainerEvent(ContainerEvent.ERROR, module, e);
- } catch (IllegalStateException e) {
- // been uninstalled
- continue;
+ if (!module.inStartResolve() && module.getId() != 0 && !triggerSet.contains(module)) {
+ start(module, StartOptions.TRANSIENT_IF_AUTO_START, StartOptions.TRANSIENT_RESUME);
}
}
return true;
}
+ private void start(Module module, StartOptions... options) {
+ try {
+ secureAction.start(module, options);
+ } catch (BundleException e) {
+ if (e.getType() == BundleException.STATECHANGE_ERROR) {
+ if (Module.ACTIVE_SET.contains(module.getState())) {
+ // There is still a timing issue here;
+ // but at least try to detect that another thread is starting the module
+ return;
+ }
+ }
+ adaptor.publishContainerEvent(ContainerEvent.ERROR, module, e);
+ } catch (IllegalStateException e) {
+ // been uninstalled
+ return;
+ }
+ }
+
private List<DynamicModuleRequirement> getDynamicRequirements(String dynamicPkgName, ModuleRevision revision) {
// TODO Will likely need to optimize this
if ((revision.getTypes() & BundleRevision.TYPE_FRAGMENT) != 0) {

Back to the top