You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by ma...@apache.org on 2010/10/07 14:22:46 UTC
svn commit: r1005430 - in
/felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm:
impl/ComponentImpl.java tracker/ServiceTracker.java
Author: marrs
Date: Thu Oct 7 12:22:46 2010
New Revision: 1005430
URL: http://svn.apache.org/viewvc?rev=1005430&view=rev
Log:
Fixed two issues:
- non-atomic additions of a list of new dependencies could lead to intermittent states becoming visible and being acted upon;
- certain ordering of adding and removing aspects could confuse the ServiceTracker.
Modified:
felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/ComponentImpl.java
felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/tracker/ServiceTracker.java
Modified: felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/ComponentImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/ComponentImpl.java?rev=1005430&r1=1005429&r2=1005430&view=diff
==============================================================================
--- felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/ComponentImpl.java (original)
+++ felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/impl/ComponentImpl.java Thu Oct 7 12:22:46 2010
@@ -130,6 +130,8 @@ public class ComponentImpl implements Co
}
private void calculateStateChanges(final State oldState, final State newState) {
+ System.out.println("SS from " + oldState + "\n to " + newState);
+
if (oldState.isInactive() && (newState.isTrackingOptional())) {
m_executor.enqueue(new Runnable() {
public void run() {
@@ -158,6 +160,11 @@ public class ComponentImpl implements Co
m_executor.enqueue(new Runnable() {
public void run() {
// TODO as far as I can see there is nothing left to do here
+
+
+ //////////unbindService(newState);
+
+
}});
}
if (oldState.isTrackingOptional() && newState.isWaitingForRequired()) {
@@ -222,6 +229,7 @@ public class ComponentImpl implements Co
m_executor.execute();
}
+ // TODO fix code duplication between add(Dependency) and add(List)
public Component add(final Dependency dependency) {
State oldState, newState;
synchronized (m_dependencies) {
@@ -229,36 +237,85 @@ public class ComponentImpl implements Co
m_dependencies.add(dependency);
}
- if (dependency.isInstanceBound()) {
- // At this point: this dependency is added from init(): but we don't want to start it now,
- // because if we start it, and if the required dependency is available, then the service.start()
- // method will be called, and this is a problem if a further
- // required (but unavailable) dependency is then added again from the init() method ...
- // Once the init() method will return, the activateService method will then calculate the state changes,
- // but at this point, all added extra-dependencies will be known.
- return this;
- }
+// if (dependency.isInstanceBound()) {
+// // At this point: this dependency is added from init(): but we don't want to start it now,
+// // because if we start it, and if the required dependency is available, then the service.start()
+// // method will be called, and this is a problem if a further
+// // required (but unavailable) dependency is then added again from the init() method ...
+// // Once the init() method will return, the activateService method will then calculate the state changes,
+// // but at this point, all added extra-dependencies will be known.
+// return this;
+// }
+
+ ///
+
+// if (oldState.isAllRequiredAvailable() || (oldState.isWaitingForRequiredInstantiated() && dependency.isRequired()) || (oldState.isWaitingForRequired() && dependency.isRequired())) {
+// ((DependencyActivation) dependency).start(this);
+// }
- if (oldState.isAllRequiredAvailable() || (oldState.isWaitingForRequiredInstantiated() && dependency.isRequired()) || (oldState.isWaitingForRequired() && dependency.isRequired())) {
- ((DependencyActivation) dependency).start(this);
+ // if we're inactive, don't do anything, otherwise we might want to start
+ // the dependency
+ if (!oldState.isInactive()) {
+ // if the dependency is required, it should be started regardless of the state
+ // we're in
+ if (dependency.isRequired()) {
+ ((DependencyActivation) dependency).start(this);
+ }
+ else {
+ // if the dependency is optional, it should only be started if we're in
+ // bound state
+ if (oldState.isBound()) {
+ ((DependencyActivation) dependency).start(this);
+ }
+ }
}
synchronized (m_dependencies) {
// starting the dependency above might have triggered another state change, so
// we have to fetch the current state again
- oldState = m_state;
newState = new State((List) m_dependencies.clone(), !oldState.isInactive(), m_isInstantiated, m_isBound);
m_state = newState;
}
calculateStateChanges(oldState, newState);
return this;
}
-
+
public Component add(List dependencies) {
- // TODO review if this can be done more smartly
- for (int i = 0; i < dependencies.size(); i++) {
- add((Dependency) dependencies.get(i));
+ State oldState, newState;
+ synchronized (m_dependencies) {
+ oldState = m_state;
+ for (int i = 0; i < dependencies.size(); i++) {
+ m_dependencies.add(dependencies.get(i));
+ }
+ }
+
+ // if we're inactive, don't do anything, otherwise we might want to start
+ // the dependencies
+ if (!oldState.isInactive()) {
+ for (int i = 0; i < dependencies.size(); i++) {
+ Dependency dependency = (Dependency) dependencies.get(i);
+ // if the dependency is required, it should be started regardless of the state
+ // we're in
+ if (dependency.isRequired()) {
+ ((DependencyActivation) dependency).start(this);
+ }
+ else {
+ // if the dependency is optional, it should only be started if we're in
+ // bound state
+ if (oldState.isBound()) {
+ ((DependencyActivation) dependency).start(this);
+ }
+ }
+ }
}
+
+ synchronized (m_dependencies) {
+ // starting the dependency above might have triggered another state change, so
+ // we have to fetch the current state again
+ newState = new State((List) m_dependencies.clone(), !oldState.isInactive(), m_isInstantiated, m_isBound);
+ m_state = newState;
+ }
+ calculateStateChanges(oldState, newState);
return this;
}
@@ -544,8 +601,8 @@ public class ComponentImpl implements Co
// then we invoke the init callback so the service can further initialize
// itself
invoke(init);
- // start extra/required dependencies which might have been added from the init() method.
- startExtraRequiredDependencies();
+// // start extra/required dependencies which might have been added from the init() method.
+// startExtraRequiredDependencies();
// see if any of this caused further state changes
calculateStateChanges();
}
@@ -567,7 +624,7 @@ public class ComponentImpl implements Co
start = m_callbackStart;
}
- // configure service with extra-dependencies which might have been added from init() method.
+// // configure service with extra-dependencies which might have been added from init() method.
configureServiceWithExtraDependencies(state);
// inform the state listeners we're starting
stateListenersStarting();
Modified: felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/tracker/ServiceTracker.java
URL: http://svn.apache.org/viewvc/felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/tracker/ServiceTracker.java?rev=1005430&r1=1005429&r2=1005430&view=diff
==============================================================================
--- felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/tracker/ServiceTracker.java (original)
+++ felix/trunk/dependencymanager/core/src/main/java/org/apache/felix/dm/tracker/ServiceTracker.java Thu Oct 7 12:22:46 2010
@@ -822,22 +822,24 @@ public class ServiceTracker implements S
private ServiceReference highestHidden(long serviceId) {
ServiceReference result = null;
int max = Integer.MIN_VALUE;
- for (int i = 0; i < m_hidden.size(); i++) {
- ServiceReference ref = (ServiceReference) m_hidden.get(i);
- Long sid = (Long) ref.getProperty(Constants.SERVICE_ID);
- Long aid = (Long) ref.getProperty(DependencyManager.ASPECT);
- if ((aid != null && aid.longValue() == serviceId)
- || (aid == null && sid != null && sid.longValue() == serviceId)) {
- Integer ranking = (Integer) ref.getProperty(Constants.SERVICE_RANKING);
- int r = 0;
- if (ranking != null) {
- r = ranking.intValue();
- }
- if (r > max) {
- max = r;
- result = ref;
- }
- }
+ synchronized (m_hidden) {
+ for (int i = 0; i < m_hidden.size(); i++) {
+ ServiceReference ref = (ServiceReference) m_hidden.get(i);
+ Long sid = (Long) ref.getProperty(Constants.SERVICE_ID);
+ Long aid = (Long) ref.getProperty(DependencyManager.ASPECT);
+ if ((aid != null && aid.longValue() == serviceId)
+ || (aid == null && sid != null && sid.longValue() == serviceId)) {
+ Integer ranking = (Integer) ref.getProperty(Constants.SERVICE_RANKING);
+ int r = 0;
+ if (ranking != null) {
+ r = ranking.intValue();
+ }
+ if (r > max) {
+ max = r;
+ result = ref;
+ }
+ }
+ }
}
return result;
}
@@ -879,7 +881,10 @@ public class ServiceTracker implements S
*/
private void hide(ServiceReference ref) {
if (DEBUG) { System.out.println("ServiceTracker.Tracked.hide " + ServiceUtil.toString(ref)); }
- m_hidden.add(ref);
+ synchronized (m_hidden) {
+ if (DEBUG) { if (m_hidden.contains(ref)) { System.out.println("ServiceTracker.Tracked.hide ERROR: " + ServiceUtil.toString(ref)); }};
+ m_hidden.add(ref);
+ }
}
/**
@@ -889,7 +894,10 @@ public class ServiceTracker implements S
*/
private void unhide(ServiceReference ref) {
if (DEBUG) { System.out.println("ServiceTracker.Tracked.unhide " + ServiceUtil.toString(ref)); }
- m_hidden.remove(ref);
+ synchronized (m_hidden) {
+ if (DEBUG) { if (!m_hidden.contains(ref)) { System.out.println("ServiceTracker.Tracked.unhide ERROR: " + ServiceUtil.toString(ref)); }};
+ m_hidden.remove(ref);
+ }
}
/**
@@ -979,7 +987,7 @@ public class ServiceTracker implements S
ServiceReference higher = null;
ServiceReference lower = null;
boolean isAspect = ServiceUtil.isAspect(reference);
- if (isAspect) {
+ if (true /* WAS isAspect */) {
long sid = ServiceUtil.getServiceId(reference);
ServiceReference sr = highestTracked(sid);
if (sr != null) {
@@ -1054,7 +1062,7 @@ public class ServiceTracker implements S
else {
higher = null;
isAspect = ServiceUtil.isAspect(reference);
- if (isAspect) {
+ if (true /* WAS isAspect */) {
long sid = ServiceUtil.getServiceId(reference);
ServiceReference sr = highestHidden(sid);
if (sr != null) {
@@ -1084,7 +1092,7 @@ public class ServiceTracker implements S
case ServiceEvent.UNREGISTERING :
higher = null;
isAspect = ServiceUtil.isAspect(reference);
- if (isAspect) {
+ if (true /* WAS isAspect */) {
long sid = ServiceUtil.getServiceId(reference);
ServiceReference sr = highestHidden(sid);
if (sr != null) {