Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Watson2016-02-19 15:28:02 +0000
committerThomas Watson2016-02-19 18:54:58 +0000
commit91d2439c80d81fccdcd8fe1150dbed119182eadf (patch)
tree8bb2f817319963f1dd8acf144dbbe0b7023d2b33
parent05d4b77e904d06e6d2c1bff98231447db2ab7195 (diff)
downloadrt.equinox.framework-91d2439c80d81fccdcd8fe1150dbed119182eadf.tar.gz
rt.equinox.framework-91d2439c80d81fccdcd8fe1150dbed119182eadf.tar.xz
rt.equinox.framework-91d2439c80d81fccdcd8fe1150dbed119182eadf.zip
Bug 487842 - Unable to acquire state change lock for the module when
starting a bundle Must increment inStart BEFORE acquiring the STARTED state the first time. There is still an unavoidable timing issue. The window is now as small as possible. A new flag is added to disable auto-starting newly resolved bundles that are marked for persistent start. Also fixed bug in how modules are restarted after a refresh. They must use their activation policy if it is set. Change-Id: I8175af9ed29e611659a8b8bc0b64b0a3874e68bf Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
-rw-r--r--bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java20
-rw-r--r--bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/container/TestModuleContainer.java20
-rw-r--r--bundles/org.eclipse.osgi/.gitignore1
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/Module.java25
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleContainer.java30
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/SystemModule.java11
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxConfiguration.java1
7 files changed, 78 insertions, 30 deletions
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java
index 57d1bd811..1f623a1fe 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ClassLoadingBundleTests.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2006, 2015 IBM Corporation and others.
+ * Copyright (c) 2006, 2016 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
@@ -98,7 +98,7 @@ public class ClassLoadingBundleTests extends AbstractBundleTests {
Bundle chainTestD = installer.installBundle("chain.test.d"); //$NON-NLS-1$
chainTest.loadClass("chain.test.TestMultiChain").newInstance(); //$NON-NLS-1$
- Object[] expectedEvents = new Object[12];
+ Object[] expectedEvents = new Object[8];
expectedEvents[0] = new BundleEvent(BundleEvent.STARTED, chainTestD);
expectedEvents[1] = new BundleEvent(BundleEvent.STARTED, chainTestB);
expectedEvents[2] = new BundleEvent(BundleEvent.STARTED, chainTestC);
@@ -107,14 +107,20 @@ public class ClassLoadingBundleTests extends AbstractBundleTests {
expectedEvents[5] = new BundleEvent(BundleEvent.STOPPED, chainTestB);
expectedEvents[6] = new BundleEvent(BundleEvent.STOPPED, chainTestC);
expectedEvents[7] = new BundleEvent(BundleEvent.STOPPED, chainTestD);
- expectedEvents[8] = new BundleEvent(BundleEvent.STARTED, chainTestD);
- expectedEvents[9] = new BundleEvent(BundleEvent.STARTED, chainTestC);
- expectedEvents[10] = new BundleEvent(BundleEvent.STARTED, chainTestB);
- expectedEvents[11] = new BundleEvent(BundleEvent.STARTED, chainTestA);
installer.refreshPackages(new Bundle[] {chainTestC, chainTestD});
- Object[] actualEvents = simpleResults.getResults(12);
+ Object[] actualEvents = simpleResults.getResults(8);
+ compareResults(expectedEvents, actualEvents);
+
+ chainTest.loadClass("chain.test.TestMultiChain").newInstance(); //$NON-NLS-1$
+ expectedEvents = new Object[4];
+ expectedEvents[0] = new BundleEvent(BundleEvent.STARTED, chainTestD);
+ expectedEvents[1] = new BundleEvent(BundleEvent.STARTED, chainTestB);
+ expectedEvents[2] = new BundleEvent(BundleEvent.STARTED, chainTestC);
+ expectedEvents[3] = new BundleEvent(BundleEvent.STARTED, chainTestA);
+
+ actualEvents = simpleResults.getResults(4);
compareResults(expectedEvents, actualEvents);
}
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 efb7eda25..90b6cad05 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
@@ -988,7 +988,7 @@ public class TestModuleContainer extends AbstractTest {
container.refresh(Arrays.asList(lazy1));
actual = database.getModuleEvents();
- expected = new ArrayList<DummyModuleEvent>(Arrays.asList(new DummyModuleEvent(lazy1, ModuleEvent.STOPPING, State.STOPPING), new DummyModuleEvent(lazy1, ModuleEvent.STOPPED, State.RESOLVED), new DummyModuleEvent(lazy1, ModuleEvent.UNRESOLVED, State.INSTALLED), new DummyModuleEvent(lazy1, ModuleEvent.RESOLVED, State.RESOLVED), new DummyModuleEvent(lazy1, ModuleEvent.STARTING, State.STARTING), new DummyModuleEvent(lazy1, ModuleEvent.STARTED, State.ACTIVE)));
+ expected = new ArrayList<DummyModuleEvent>(Arrays.asList(new DummyModuleEvent(lazy1, ModuleEvent.STOPPING, State.STOPPING), new DummyModuleEvent(lazy1, ModuleEvent.STOPPED, State.RESOLVED), new DummyModuleEvent(lazy1, ModuleEvent.UNRESOLVED, State.INSTALLED), new DummyModuleEvent(lazy1, ModuleEvent.RESOLVED, State.RESOLVED), new DummyModuleEvent(lazy1, ModuleEvent.LAZY_ACTIVATION, State.LAZY_STARTING)));
assertEvents(expected, actual, true);
container.update(lazy1, OSGiManifestBuilderFactory.createBuilder(getManifest("lazy1_v1.MF")), null);
@@ -2369,7 +2369,20 @@ public class TestModuleContainer extends AbstractTest {
@Test
public void testStartOnResolve() throws BundleException, IOException {
- DummyContainerAdaptor adaptor = createDummyAdaptor();
+ doTestStartOnResolve(true);
+ }
+
+ @Test
+ public void testDisableStartOnResolve() throws BundleException, IOException {
+ doTestStartOnResolve(false);
+ }
+
+ private void doTestStartOnResolve(boolean enabled) throws BundleException, IOException {
+ Map<String, String> configuration = new HashMap<String, String>();
+ if (!enabled) {
+ configuration.put(EquinoxConfiguration.PROP_MODULE_AUTO_START_ON_RESOLVE, Boolean.toString(false));
+ }
+ DummyContainerAdaptor adaptor = new DummyContainerAdaptor(new DummyCollisionHook(false), configuration);
ModuleContainer container = adaptor.getContainer();
// install the system.bundle
@@ -2405,8 +2418,9 @@ public class TestModuleContainer extends AbstractTest {
report = container.resolve(Collections.<Module> emptySet(), false);
Assert.assertNull("Found a error.", report.getResolutionException());
+ State expectedState = enabled ? State.ACTIVE : State.RESOLVED;
for (Module module : modules) {
- Assert.assertEquals("Wrong state.", State.ACTIVE, module.getState());
+ Assert.assertEquals("Wrong state.", expectedState, module.getState());
}
}
diff --git a/bundles/org.eclipse.osgi/.gitignore b/bundles/org.eclipse.osgi/.gitignore
index 7d67e02a9..bb575b389 100644
--- a/bundles/org.eclipse.osgi/.gitignore
+++ b/bundles/org.eclipse.osgi/.gitignore
@@ -8,3 +8,4 @@ eclipseAdaptor.jar
resolver.jar
/generated/
.DS_Store
+/configuration/
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 7970b4f8b..39c2ba26b 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
@@ -161,7 +161,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 AtomicInteger inStartResolve = new AtomicInteger(0);
+ final AtomicInteger inStart = new AtomicInteger(0);
private volatile State state = State.INSTALLED;
private volatile int startlevel;
private volatile long lastModified;
@@ -394,22 +394,25 @@ public abstract class Module implements BundleReference, BundleStartLevel, Compa
}
BundleException startError = null;
boolean lockedStarted = false;
- lockStateChange(ModuleEvent.STARTED);
+ // Indicate we are in the middle of a start.
+ // This must be incremented before we acquire the STARTED lock the first time.
+ inStart.incrementAndGet();
try {
- inStartResolve.incrementAndGet();
+ lockStateChange(ModuleEvent.STARTED);
lockedStarted = true;
checkValid();
if (StartOptions.TRANSIENT_IF_AUTO_START.isContained(options) && !settings.contains(Settings.AUTO_START)) {
- // Do nothing
+ // Do nothing; this is a request to start only if the module is set for auto start
return;
}
checkFragment();
persistStartOptions(options);
if (getStartLevel() > getRevisions().getContainer().getStartLevel()) {
if (StartOptions.TRANSIENT.isContained(options)) {
+ // it is an error to attempt to transient start a bundle without its start level met
throw new BundleException(Msg.Module_Transient_StartError, BundleException.START_TRANSIENT_ERROR);
}
- // DO nothing
+ // Do nothing; start level is not met
return;
}
if (State.ACTIVE.equals(getState()))
@@ -421,7 +424,6 @@ public abstract class Module implements BundleReference, BundleStartLevel, Compa
unlockStateChange(ModuleEvent.STARTED);
lockedStarted = false;
try {
-
report = getRevisions().getContainer().resolve(Arrays.asList(this), true);
} finally {
lockStateChange(ModuleEvent.STARTED);
@@ -456,7 +458,7 @@ public abstract class Module implements BundleReference, BundleStartLevel, Compa
if (lockedStarted) {
unlockStateChange(ModuleEvent.STARTED);
}
- inStartResolve.decrementAndGet();
+ inStart.decrementAndGet();
}
if (event != null) {
@@ -701,7 +703,12 @@ public abstract class Module implements BundleReference, BundleStartLevel, Compa
return current == null ? false : current.hasLazyActivatePolicy();
}
- final boolean inStartResolve() {
- return inStartResolve.get() > 0;
+ /**
+ * Used internally by the container to determine if any thread is in the middle
+ * of a start operation on this module.
+ * @return
+ */
+ final boolean inStart() {
+ return inStart.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 c7e02ab7c..00c40a0c8 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
@@ -90,6 +90,8 @@ public final class ModuleContainer implements DebugOptionsListener {
private final long moduleLockTimeout;
+ private final boolean autoStartOnResolve;
+
boolean DEBUG_MONITOR_LAZY = false;
/**
@@ -121,6 +123,12 @@ public final class ModuleContainer implements DebugOptionsListener {
if (debugOptions != null) {
this.DEBUG_MONITOR_LAZY = debugOptions.getBooleanOption(Debug.OPTION_MONITOR_LAZY, false);
}
+
+ String autoStartOnResolveProp = adaptor.getProperty(EquinoxConfiguration.PROP_MODULE_AUTO_START_ON_RESOLVE);
+ if (autoStartOnResolveProp == null) {
+ autoStartOnResolveProp = Boolean.toString(true);
+ }
+ this.autoStartOnResolve = Boolean.parseBoolean(autoStartOnResolveProp);
}
/**
@@ -702,15 +710,22 @@ public final class ModuleContainer implements DebugOptionsListener {
if (restartTriggers) {
for (Module module : triggers) {
if (module.getId() != 0 && Module.RESOLVED_SET.contains(module.getState())) {
- start(module, StartOptions.TRANSIENT);
+ start(module, StartOptions.TRANSIENT_RESUME);
}
}
}
- // 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)) {
- start(module, StartOptions.TRANSIENT_IF_AUTO_START, StartOptions.TRANSIENT_RESUME);
+ if (autoStartOnResolve) {
+ // 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) {
+ // Note that we check inStart here. There is still a timing issue that is impossible to avoid.
+ // Another thread could attempt to start the module but we could check inStart() before that thread
+ // increments inStart. One thread will win the race to grab the module STARTED lock. That thread
+ // will end up actually starting the module and the other thread will block. If a timeout occurs
+ // the blocking thread will get an exception.
+ if (!module.inStart() && module.getId() != 0 && !triggerSet.contains(module)) {
+ start(module, StartOptions.TRANSIENT_IF_AUTO_START, StartOptions.TRANSIENT_RESUME);
+ }
}
}
return true;
@@ -860,8 +875,7 @@ public final class ModuleContainer implements DebugOptionsListener {
} catch (BundleException e) {
adaptor.publishContainerEvent(ContainerEvent.ERROR, refreshModule, e);
}
- }
- if (!State.ACTIVE.equals(previousState)) {
+ } else {
iTriggers.remove();
}
}
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/SystemModule.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/SystemModule.java
index 9c586109b..58cd1e93f 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/SystemModule.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/SystemModule.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2012, 2014 IBM Corporation and others.
+ * Copyright (c) 2012, 2016 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
@@ -40,11 +40,15 @@ public abstract class SystemModule extends Module {
*/
public final void init() throws BundleException {
getRevisions().getContainer().checkAdminPermission(getBundle(), AdminPermission.EXECUTE);
+
boolean lockedStarted = false;
- lockStateChange(ModuleEvent.STARTED);
+ // Indicate we are in the middle of a start.
+ // This must be incremented before we acquire the STARTED lock the first time.
+ inStart.incrementAndGet();
try {
- getContainer().getAdaptor().initBegin();
+ lockStateChange(ModuleEvent.STARTED);
lockedStarted = true;
+ getContainer().getAdaptor().initBegin();
checkValid();
if (ACTIVE_SET.contains(getState()))
return;
@@ -97,6 +101,7 @@ public abstract class SystemModule extends Module {
if (lockedStarted) {
unlockStateChange(ModuleEvent.STARTED);
}
+ inStart.decrementAndGet();
}
}
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxConfiguration.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxConfiguration.java
index 8000f44db..5c05ac31e 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxConfiguration.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/framework/EquinoxConfiguration.java
@@ -192,6 +192,7 @@ public class EquinoxConfiguration implements EnvironmentInfo {
public static final String PROP_STATE_SAVE_DELAY_INTERVAL = "eclipse.stateSaveDelayInterval"; //$NON-NLS-1$
public static final String PROP_MODULE_LOCK_TIMEOUT = "osgi.module.lock.timeout"; //$NON-NLS-1$
+ public static final String PROP_MODULE_AUTO_START_ON_RESOLVE = "osgi.module.auto.start.on.resolve"; //$NON-NLS-1$
public static final String PROP_ALLOW_RESTRICTED_PROVIDES = "osgi.equinox.allow.restricted.provides"; //$NON-NLS-1$
public static final String PROP_LOG_HISTORY_MAX = "equinox.log.history.max"; //$NON-NLS-1$

Back to the top