Hold bundle state lock while adding/removing listeners, otherwise
clarify that we ignore the race condition. (FELIX-2748)
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1081938 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/framework/src/main/java/org/apache/felix/framework/BundleContextImpl.java b/framework/src/main/java/org/apache/felix/framework/BundleContextImpl.java
index fd39434..ba6f632 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleContextImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleContextImpl.java
@@ -68,6 +68,11 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a check-then-act situation,
+ // but we ignore it since the time window is small and
+ // the result is the same as if the calling thread had
+ // won the race condition.
+
Object sm = System.getSecurityManager();
if (sm != null)
@@ -91,6 +96,11 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a check-then-act situation,
+ // but we ignore it since the time window is small and
+ // the result is the same as if the calling thread had
+ // won the race condition.
+
return m_bundle;
}
@@ -98,6 +108,12 @@
throws InvalidSyntaxException
{
checkValidity();
+
+ // CONCURRENCY NOTE: This is a check-then-act situation,
+ // but we ignore it since the time window is small and
+ // the result is the same as if the calling thread had
+ // won the race condition.
+
return new FilterImpl(expr);
}
@@ -112,6 +128,11 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a check-then-act situation,
+ // but we ignore it since the time window is small and
+ // the result is the same as if the calling thread had
+ // won the race condition.
+
Bundle result = null;
Object sm = System.getSecurityManager();
@@ -136,6 +157,11 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a check-then-act situation,
+ // but we ignore it since the time window is small and
+ // the result is the same as if the calling thread had
+ // won the race condition.
+
return m_felix.getBundle(id);
}
@@ -143,6 +169,11 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a check-then-act situation,
+ // but we ignore it since the time window is small and
+ // the result is the same as if the calling thread had
+ // won the race condition.
+
return m_felix.getBundles();
}
@@ -150,6 +181,10 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a NOT a check-then-act situation,
+ // because internally the framework acquires the bundle state
+ // lock to ensure state consistency.
+
Object sm = System.getSecurityManager();
if (sm != null)
@@ -168,6 +203,10 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a NOT a check-then-act situation,
+ // because internally the framework acquires the bundle state
+ // lock to ensure state consistency.
+
Object sm = System.getSecurityManager();
if (sm != null)
@@ -199,6 +238,10 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a NOT a check-then-act situation,
+ // because internally the framework acquires the bundle state
+ // lock to ensure state consistency.
+
m_felix.addServiceListener(m_bundle, l, s);
}
@@ -206,6 +249,10 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a NOT a check-then-act situation,
+ // because internally the framework acquires the bundle state
+ // lock to ensure state consistency.
+
m_felix.removeServiceListener(m_bundle, l);
}
@@ -213,6 +260,10 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a NOT a check-then-act situation,
+ // because internally the framework acquires the bundle state
+ // lock to ensure state consistency.
+
m_felix.addFrameworkListener(m_bundle, l);
}
@@ -220,6 +271,10 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a NOT a check-then-act situation,
+ // because internally the framework acquires the bundle state
+ // lock to ensure state consistency.
+
m_felix.removeFrameworkListener(m_bundle, l);
}
@@ -234,6 +289,10 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a NOT a check-then-act situation,
+ // because internally the framework acquires the bundle state
+ // lock to ensure state consistency.
+
Object sm = System.getSecurityManager();
if (sm != null)
@@ -255,6 +314,11 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a check-then-act situation,
+ // but we ignore it since the time window is small and
+ // the result is the same as if the calling thread had
+ // won the race condition.
+
try
{
ServiceReference[] refs = getServiceReferences(clazz, null);
@@ -297,6 +361,11 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a check-then-act situation,
+ // but we ignore it since the time window is small and
+ // the result is the same as if the calling thread had
+ // won the race condition.
+
return m_felix.getAllowedServiceReferences(m_bundle, clazz, filter, false);
}
@@ -306,6 +375,11 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a check-then-act situation,
+ // but we ignore it since the time window is small and
+ // the result is the same as if the calling thread had
+ // won the race condition.
+
return m_felix.getAllowedServiceReferences(m_bundle, clazz, filter, true);
}
@@ -314,6 +388,11 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a check-then-act situation,
+ // but we ignore it since the time window is small and
+ // the result is the same as if the calling thread had
+ // won the race condition.
+
if (ref == null)
{
throw new NullPointerException("Specified service reference cannot be null.");
@@ -333,6 +412,11 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a check-then-act situation,
+ // but we ignore it since the time window is small and
+ // the result is the same as if the calling thread had
+ // won the race condition.
+
if (ref == null)
{
throw new NullPointerException("Specified service reference cannot be null.");
@@ -346,6 +430,11 @@
{
checkValidity();
+ // CONCURRENCY NOTE: This is a check-then-act situation,
+ // but we ignore it since the time window is small and
+ // the result is the same as if the calling thread had
+ // won the race condition.
+
return m_felix.getDataFile(m_bundle, s);
}
diff --git a/framework/src/main/java/org/apache/felix/framework/Felix.java b/framework/src/main/java/org/apache/felix/framework/Felix.java
index f75d8ca..2a15cbb 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -2716,14 +2716,50 @@
return (Bundle[]) bundles.values().toArray(new Bundle[bundles.size()]);
}
- void addBundleListener(Bundle bundle, BundleListener l)
+ void addBundleListener(BundleImpl bundle, BundleListener l)
{
- m_dispatcher.addListener(bundle, BundleListener.class, l, null);
+ // Acquire bundle lock.
+ try
+ {
+ acquireBundleLock(bundle, Bundle.STARTING | Bundle.ACTIVE);
+ }
+ catch (IllegalStateException ex)
+ {
+ throw new IllegalStateException(
+ "Can only add listeners while bundle is active or activating.");
+ }
+
+ try
+ {
+ m_dispatcher.addListener(bundle, BundleListener.class, l, null);
+ }
+ finally
+ {
+ releaseBundleLock(bundle);
+ }
}
- void removeBundleListener(Bundle bundle, BundleListener l)
+ void removeBundleListener(BundleImpl bundle, BundleListener l)
{
- m_dispatcher.removeListener(bundle, BundleListener.class, l);
+ // Acquire bundle lock.
+ try
+ {
+ acquireBundleLock(bundle, Bundle.STOPPING | Bundle.ACTIVE);
+ }
+ catch (IllegalStateException ex)
+ {
+ throw new IllegalStateException(
+ "Can only remove listeners while bundle is active or activating.");
+ }
+
+ try
+ {
+ m_dispatcher.removeListener(bundle, BundleListener.class, l);
+ }
+ finally
+ {
+ releaseBundleLock(bundle);
+ }
}
/**
@@ -2735,21 +2771,45 @@
* @param l The service listener to add to the listener list.
* @param f The filter for the listener; may be null.
**/
- void addServiceListener(Bundle bundle, ServiceListener l, String f)
+ void addServiceListener(BundleImpl bundle, ServiceListener l, String f)
throws InvalidSyntaxException
{
- Filter oldFilter = m_dispatcher.addListener(
- bundle, ServiceListener.class, l, (f == null) ? null : FrameworkUtil.createFilter(f));
+ // Acquire bundle lock.
+ try
+ {
+ acquireBundleLock(bundle, Bundle.STARTING | Bundle.ACTIVE);
+ }
+ catch (IllegalStateException ex)
+ {
+ throw new IllegalStateException(
+ "Can only add listeners while bundle is active or activating.");
+ }
+ Filter oldFilter;
+
+ try
+ {
+ oldFilter = m_dispatcher.addListener(
+ bundle, ServiceListener.class, l,
+ (f == null) ? null : FrameworkUtil.createFilter(f));
+ }
+ finally
+ {
+ releaseBundleLock(bundle);
+ }
+
+ // Invoke ListenerHook.removed() if filter updated.
List listenerHooks = m_registry.getListenerHooks();
if (oldFilter != null)
{
final Collection removed = Collections.singleton(
- new ListenerHookInfoImpl(((BundleImpl) bundle)._getBundleContext(), l, oldFilter.toString(), true));
+ new ListenerHookInfoImpl(
+ ((BundleImpl) bundle)._getBundleContext(), l, oldFilter.toString(), true));
InvokeHookCallback removedCallback = new ListenerHookRemovedCallback(removed);
for (int i = 0; i < listenerHooks.size(); i++)
{
- m_registry.invokeHook((ServiceReference) listenerHooks.get(i), this, removedCallback);
+ m_registry.invokeHook(
+ (ServiceReference) listenerHooks.get(i), this, removedCallback);
}
}
@@ -2776,10 +2836,30 @@
* @param bundle The context bundle of the listener
* @param l The service listener to remove from the listener list.
**/
- void removeServiceListener(Bundle bundle, ServiceListener l)
+ void removeServiceListener(BundleImpl bundle, ServiceListener l)
{
- ListenerHook.ListenerInfo listener =
- m_dispatcher.removeListener(bundle, ServiceListener.class, l);
+ // Acquire bundle lock.
+ try
+ {
+ acquireBundleLock(bundle, Bundle.STOPPING | Bundle.ACTIVE);
+ }
+ catch (IllegalStateException ex)
+ {
+ throw new IllegalStateException(
+ "Can only remove listeners while bundle is active or activating.");
+ }
+
+ ListenerHook.ListenerInfo listener;
+
+ try
+ {
+ listener =
+ m_dispatcher.removeListener(bundle, ServiceListener.class, l);
+ }
+ finally
+ {
+ releaseBundleLock(bundle);
+ }
if (listener != null)
{
@@ -2794,14 +2874,50 @@
}
}
- void addFrameworkListener(Bundle bundle, FrameworkListener l)
+ void addFrameworkListener(BundleImpl bundle, FrameworkListener l)
{
- m_dispatcher.addListener(bundle, FrameworkListener.class, l, null);
+ // Acquire bundle lock.
+ try
+ {
+ acquireBundleLock(bundle, Bundle.STARTING | Bundle.ACTIVE);
+ }
+ catch (IllegalStateException ex)
+ {
+ throw new IllegalStateException(
+ "Can only add listeners while bundle is active or activating.");
+ }
+
+ try
+ {
+ m_dispatcher.addListener(bundle, FrameworkListener.class, l, null);
+ }
+ finally
+ {
+ releaseBundleLock(bundle);
+ }
}
- void removeFrameworkListener(Bundle bundle, FrameworkListener l)
+ void removeFrameworkListener(BundleImpl bundle, FrameworkListener l)
{
- m_dispatcher.removeListener(bundle, FrameworkListener.class, l);
+ // Acquire bundle lock.
+ try
+ {
+ acquireBundleLock(bundle, Bundle.STOPPING | Bundle.ACTIVE);
+ }
+ catch (IllegalStateException ex)
+ {
+ throw new IllegalStateException(
+ "Can only remove listeners while bundle is active or activating.");
+ }
+
+ try
+ {
+ m_dispatcher.removeListener(bundle, FrameworkListener.class, l);
+ }
+ finally
+ {
+ releaseBundleLock(bundle);
+ }
}
/**