FELIX-50 Rearranged the handling of dependency changes, resulting service state changes and the resulting (de)activation of the service.
Introduced a new State object (which is a complete snapshot of the state).
Added a serial executor that serially executes tasks in a queue (without creating a new thread(pool)).
Note: the code is still being tested, and contains debug statements, so use it with care!


git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@545630 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ConfigurationDependency.java b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ConfigurationDependency.java
index ed0f537..e6ec17a 100644
--- a/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ConfigurationDependency.java
+++ b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ConfigurationDependency.java
@@ -79,6 +79,7 @@
 	}
 
 	public void updated(Dictionary settings) throws ConfigurationException {
+		System.out.println("Updating " + settings);
 		// if non-null settings come in, we have to instantiate the service and
 		// apply these settings
 		((ServiceImpl) m_service).initService();
@@ -94,6 +95,9 @@
 				// remain in the state we were, assuming that any error causes
 				// the "old" configuration to stay in effect
 			}
+			else {
+				throw new IllegalStateException("Could not invoke updated on implementation");
+			}
 		}
 		else {
 			throw new IllegalStateException("Could not instantiate implementation");
@@ -127,4 +131,8 @@
             throw new IllegalStateException("Cannot modify state while active.");
         }
     }
+    
+    public String toString() {
+    	return "ConfigurationDependency[" + m_pid + "]";
+    }
 }
diff --git a/dependencymanager/src/main/java/org/apache/felix/dependencymanager/DefaultNullObject.java b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/DefaultNullObject.java
index fbd5d7a..79bb240 100644
--- a/dependencymanager/src/main/java/org/apache/felix/dependencymanager/DefaultNullObject.java
+++ b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/DefaultNullObject.java
@@ -28,7 +28,7 @@
  * 
  * @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
  */
-public class DefaultNullObject implements InvocationHandler {
+public final class DefaultNullObject implements InvocationHandler {
     private static final Boolean DEFAULT_BOOLEAN = Boolean.FALSE;
     private static final Byte DEFAULT_BYTE = new Byte((byte) 0);
     private static final Short DEFAULT_SHORT = new Short((short) 0);
diff --git a/dependencymanager/src/main/java/org/apache/felix/dependencymanager/SerialExecutor.java b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/SerialExecutor.java
new file mode 100644
index 0000000..05a99e2
--- /dev/null
+++ b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/SerialExecutor.java
@@ -0,0 +1,148 @@
+package org.apache.felix.dependencymanager;
+
+import java.util.LinkedList;
+import java.util.NoSuchElementException;
+
+/**
+ * Allows you to enqueue tasks from multiple threads and then execute
+ * them on one thread sequentially. It assumes more than one thread will
+ * try to execute the tasks and it will make an effort to pick the first
+ * task that comes along whilst making sure subsequent tasks return
+ * without waiting.
+ * 
+ * @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
+ */
+public final class SerialExecutor {
+    private final LinkedList m_workQueue = new LinkedList();
+    private Runnable m_active;
+    
+    /**
+     * Enqueue a new task for later execution. This method is
+     * thread-safe, so multiple threads can contribute tasks.
+     * 
+     * @param runnable the runnable containing the actual task
+     */
+    public synchronized void enqueue(final Runnable runnable) {
+    	m_workQueue.addLast(new Runnable() {
+			public void run() {
+				try {
+					runnable.run();
+				}
+				finally {
+					scheduleNext();
+				}
+			}
+		});
+    }
+    
+    /**
+     * Execute any pending tasks. This method is thread safe,
+     * so multiple threads can try to execute the pending
+     * tasks, but only the first will be used to actually do
+     * so. Other threads will return immediately.
+     */
+    public void execute() {
+    	Runnable active;
+    	synchronized (this) {
+    		active = m_active;
+    	}
+    	if (active == null) {
+    		scheduleNext();
+    	}
+    }
+
+    private void scheduleNext() {
+    	Runnable active;
+    	synchronized (this) {
+    		try {
+    			m_active = (Runnable) m_workQueue.removeFirst();
+    		}
+    		catch (NoSuchElementException e) {
+    			m_active = null;
+    		}
+    		active = m_active;
+    	}
+    	if (active != null) {
+            active.run();
+        }
+    }
+    
+    /*
+    class SerialExecutor implements Executor {
+        final Queue<Runnable> tasks = new LinkedBlockingQueue<Runnable>();
+        final Executor executor;
+        Runnable active;
+
+        SerialExecutor(Executor executor) {
+            this.executor = executor;
+        }
+
+        public synchronized void execute(final Runnable r) {
+            tasks.offer(new Runnable() {
+                public void run() {
+                    try {
+                        r.run();
+                    } finally {
+                        scheduleNext();
+                    }
+                }
+            });
+            if (active == null) {
+                scheduleNext();
+            }
+        }
+
+        protected synchronized void scheduleNext() {
+            if ((active = tasks.poll()) != null) {
+                executor.execute(active);
+            }
+        }
+    }
+    */
+
+
+    // just some test code ;)
+    public static void main(String[] args) {
+    	final SerialExecutor se = new SerialExecutor();
+    	(new Thread("T1") { public void run() {
+    		for (int i = 0; i < 100; i++) {
+    			final int nr = i;
+	    		se.enqueue(new Runnable() { public void run() {
+	    			System.out.println("A" + nr + ":" + Thread.currentThread().getName());
+	    			if (nr % 10 == 5) {
+	    	    		try { Thread.sleep(10); } catch (InterruptedException ie) {}
+	    			}
+	    		}});
+	    		try { Thread.sleep(1); } catch (InterruptedException ie) {}
+	    		se.execute();
+    		}
+    		System.out.println("A is done");
+    	}}).start();
+		try { Thread.sleep(5); } catch (InterruptedException ie) {}
+    	(new Thread("T2") { public void run() {
+    		for (int i = 0; i < 100; i++) {
+    			final int nr = i;
+	    		se.enqueue(new Runnable() { public void run() {
+	    			System.out.println("B" + nr + ":" + Thread.currentThread().getName());
+	    			if (nr % 19 == 6) {
+	    	    		try { Thread.sleep(20); } catch (InterruptedException ie) {}
+	    			}
+	    		}});
+	    		try { Thread.sleep(1); } catch (InterruptedException ie) {}
+	    		se.execute();
+    		}
+    		System.out.println("B is done");
+    	}}).start();
+		try { Thread.sleep(5); } catch (InterruptedException ie) {}
+    	(new Thread("T3") { public void run() {
+    		for (int i = 0; i < 100; i++) {
+    			final int nr = i;
+	    		se.enqueue(new Runnable() { public void run() {
+	    			System.out.println("C" + nr + ":" + Thread.currentThread().getName());
+	    		}});
+	    		se.execute();
+    		}
+    		System.out.println("C is done");
+    	}}).start();
+    }
+}
diff --git a/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ServiceDependency.java b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ServiceDependency.java
index 11e5c9e..5054e3e 100644
--- a/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ServiceDependency.java
+++ b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ServiceDependency.java
@@ -420,6 +420,6 @@
     }
     
     public String toString() {
-        return "ServiceDependency[" + m_trackedServiceName + " " + m_trackedServiceFilter + " " + m_isRequired + "] for " + m_service;
+        return "ServiceDependency[" + m_trackedServiceName + " " + m_trackedServiceFilter + "]";
     }
 }
diff --git a/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ServiceImpl.java b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ServiceImpl.java
index 3b1be60..666c781 100644
--- a/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ServiceImpl.java
+++ b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ServiceImpl.java
@@ -38,99 +38,197 @@
  */
 public class ServiceImpl implements Service {
     private static final ServiceRegistration NULL_REGISTRATION;
-    private static final int STARTING = 1;
-    private static final int WAITING_FOR_REQUIRED = 2;
-    private static final int TRACKING_OPTIONAL = 3;
-    private static final int STOPPING = 4;
-    private static final String[] STATE_NAMES = {
-        "(unknown)", 
-        "starting", 
-        "waiting for required dependencies", 
-        "tracking optional dependencies", 
-        "stopping"};
+//    private static final int STARTING = 1;
+//    private static final int WAITING_FOR_REQUIRED = 2;
+//    private static final int TRACKING_OPTIONAL = 3;
+//    private static final int STOPPING = 4;
+//    private static final String[] STATE_NAMES = {
+//        "(unknown)", 
+//        "starting", 
+//        "waiting for required dependencies", 
+//        "tracking optional dependencies", 
+//        "stopping"};
+    private static final ServiceStateListener[] SERVICE_STATE_LISTENER_TYPE = new ServiceStateListener[] {};
     
-    private BundleContext m_context;
-    private ServiceRegistration m_registration;
-    
+    private final BundleContext m_context;
+
+    // configuration (static)
     private String m_callbackInit;
     private String m_callbackStart;
     private String m_callbackStop;
     private String m_callbackDestroy;
-    
-    private List m_listeners = new ArrayList();
-    private ArrayList m_dependencies = new ArrayList();
-    private int m_state;
-    
-    private Object m_serviceInstance;
-    private Object m_implementation;
     private Object m_serviceName;
+    private Object m_implementation;
+    
+    // configuration (dynamic, but does not affect state)
     private Dictionary m_serviceProperties;
+    
+    // configuration (dynamic, and affects state)
+    private ArrayList m_dependencies = new ArrayList();
+    
+    // runtime state (calculated from dependencies)
+//    private int m_state;
+    private State m_state;
+    
+    // runtime state (changes because of state changes)
+    private Object m_serviceInstance;
+    private ServiceRegistration m_registration;
 
+    // service state listeners
+    private final List m_stateListeners = new ArrayList();
+
+    // work queue
+    private final SerialExecutor m_executor = new SerialExecutor();
+//    final LinkedList m_workQueue = new LinkedList();
+//    Runnable m_active;
+    
     public ServiceImpl(BundleContext context) {
-        m_state = STARTING;
+//        m_state = STARTING;
+    	m_state = new State((List) m_dependencies.clone(), false);
         m_context = context;
         m_callbackInit = "init";
         m_callbackStart = "start";
         m_callbackStop = "stop";
         m_callbackDestroy = "destroy";
+        m_implementation = null;
     }
     
-    public Service add(Dependency dependency) {
+    private void calculateStateChanges(final State oldState, final State newState) {
+    	if (oldState.isWaitingForRequired() && newState.isTrackingOptional()) {
+    		// TODO service needs to be activated
+    		// activateService(newState);
+        	m_executor.enqueue(new Runnable() {
+				public void run() {
+					activateService(newState);
+				}});
+    	}
+    	if (oldState.isTrackingOptional() && newState.isWaitingForRequired()) {
+    		// TODO service needs to be deactivated
+    		// deactivateService(oldState);
+    		m_executor.enqueue(new Runnable() {
+				public void run() {
+					deactivateService(oldState);
+				}});
+    	}
+    	
+    	if (oldState.isInactive() && (newState.isTrackingOptional())) {
+    		m_executor.enqueue(new Runnable() {
+				public void run() {
+					activateService(newState);
+				}});
+    	}
+    	if (oldState.isInactive() && (newState.isWaitingForRequired())) {
+    		m_executor.enqueue(new Runnable() {
+				public void run() {
+					startTrackingRequired(newState);
+				}});
+    	}
+    	if ((oldState.isWaitingForRequired()) && newState.isInactive()) {
+    		m_executor.enqueue(new Runnable() {
+				public void run() {
+					stopTrackingRequired(oldState);
+				}});
+    	}
+    	if ((oldState.isTrackingOptional()) && newState.isInactive()) {
+    		m_executor.enqueue(new Runnable() {
+				public void run() {
+					deactivateService(oldState);
+					stopTrackingRequired(oldState);
+				}});
+    	}
+    	m_executor.execute();
+    }
+    
+    public Service add(final Dependency dependency) {
+    	State oldState, newState;
         synchronized (m_dependencies) {
+        	oldState = m_state;
             m_dependencies.add(dependency);
         }
-        if (m_state == WAITING_FOR_REQUIRED) {
-            // if we're waiting for required dependencies, and
-            // this is a required dependency, start tracking it
-            // ...otherwise, we don't need to do anything yet
-            if (dependency.isRequired()) {
-                dependency.start(this);
-            }
+        if (oldState.isTrackingOptional() || (oldState.isWaitingForRequired() && dependency.isRequired())) {
+        	dependency.start(this);
         }
-        else if (m_state == TRACKING_OPTIONAL) {
-            // start tracking the dependency
-            dependency.start(this);
-            if (dependency.isRequired() && !dependency.isAvailable()) {
-                // if this is a required dependency and it can not
-                // be resolved right away, then we need to go back to 
-                // the waiting for required state, until this
-                // dependency is available
-                deactivateService();
-            }
+        synchronized (m_dependencies) {
+            newState = new State((List) m_dependencies.clone(), !oldState.isInactive());
+            m_state = newState;
+            calculateStateChanges(oldState, newState);
         }
+//        executeWorkInQueue();
+//            if ((state == WAITING_FOR_REQUIRED) && dependency.isRequired()) {
+//            	// if we're waiting for required dependencies, and
+//            	// this is a required dependency, start tracking it
+//            	// ...otherwise, we don't need to do anything yet
+//            	final ServiceImpl impl = this;
+//            	m_workQueue.addLast(new Runnable() {
+//					public void run() {
+//						dependency.start(impl);
+//					}});
+//            }
+//            if (state == TRACKING_OPTIONAL) {
+//            	// start tracking the dependency
+//            	final ServiceImpl impl = this;
+//            	m_workQueue.addLast(new Runnable() {
+//					public void run() {
+//						dependency.start(impl);
+//					}});
+//            	// TODO review this logic
+//            	if (dependency.isRequired() && !dependency.isAvailable()) {
+//            		// if this is a required dependency and it can not
+//            		// be resolved right away, then we need to go back to 
+//            		// the waiting for required state, until this
+//            		// dependency is available
+//                	m_workQueue.addLast(new Runnable() {
+//    					public void run() {
+//    						deactivateService();
+//    					}});
+//            	}
+//        }
+//        executeWorkInQueue();
         return this;
     }
 
     public Service remove(Dependency dependency) {
+    	State oldState, newState;
         synchronized (m_dependencies) {
+        	oldState = m_state;
             m_dependencies.remove(dependency);
         }
-        if (m_state == TRACKING_OPTIONAL) {
-            // if we're tracking optional dependencies, then any
-            // dependency that is removed can be stopped without
-            // causing state changes
-            dependency.stop(this);
+        if (oldState.isTrackingOptional() || (oldState.isWaitingForRequired() && dependency.isRequired())) {
+        	dependency.stop(this);
         }
-        else if (m_state == WAITING_FOR_REQUIRED) {
-            // if we're waiting for required dependencies, then
-            // we only need to stop tracking the dependency if it
-            // too is required; this might trigger a state change
-            if (dependency.isRequired()) {
-                dependency.stop(this);
-                if (allRequiredDependenciesAvailable()) {
-                    activateService();
-                }
-            }
+        synchronized (m_dependencies) {
+            newState = new State((List) m_dependencies.clone(), !oldState.isInactive());
+            m_state = newState;
         }
+        calculateStateChanges(oldState, newState);
+//    	int state;
+//        synchronized (m_dependencies) {
+//        	state = m_state;
+//            m_dependencies.remove(dependency);
+//        }
+//        if (state == TRACKING_OPTIONAL) {
+//            // if we're tracking optional dependencies, then any
+//            // dependency that is removed can be stopped without
+//            // causing state changes
+//            dependency.stop(this);
+//        }
+//        if ((state == WAITING_FOR_REQUIRED) && dependency.isRequired()) {
+//            // if we're waiting for required dependencies, then
+//            // we only need to stop tracking the dependency if it
+//            // too is required; this might trigger a state change
+//            dependency.stop(this);
+//            // TODO review this logic
+//            if (allRequiredDependenciesAvailable()) {
+//                activateService();
+//            }
+//        }
         return this;
     }
 
     public List getDependencies() {
-        List list;
         synchronized (m_dependencies) {
-            list = (List) m_dependencies.clone();
+            return (List) m_dependencies.clone();
         }
-        return list;
     }
     
     public ServiceRegistration getServiceRegistration() {
@@ -141,100 +239,285 @@
         return m_serviceInstance;
     }
     
-    public void dependencyAvailable(Dependency dependency) {
-        if ((dependency.isRequired()) 
-            && (m_state == WAITING_FOR_REQUIRED) 
-            && (allRequiredDependenciesAvailable())) {
-            activateService();
+    public void dependencyAvailable(final Dependency dependency) {
+    	State oldState, newState;
+        synchronized (m_dependencies) {
+        	oldState = m_state;
+            newState = new State((List) m_dependencies.clone(), !oldState.isInactive());
+            m_state = newState;
         }
-        if ((!dependency.isRequired()) && (m_state == TRACKING_OPTIONAL)) {
-            updateInstance(dependency);
+        calculateStateChanges(oldState, newState);
+        if (newState.isTrackingOptional()) {
+        	m_executor.enqueue(new Runnable() {
+        		public void run() {
+        			updateInstance(dependency);
+        		}
+        	});
+        	m_executor.execute();
         }
+
+//    	int oldState, newState;
+//    	synchronized (m_dependencies) {
+//    		oldState = m_state;
+//    		newState = calculateState();
+//    	}
+//    	if (dependency.isRequired() && (oldState == WAITING_FOR_REQUIRED) && (newState == TRACKING_OPTIONAL)) {
+//    		activateService();
+//    	}
+//    	// TODO review, only update optional deps here??? nonono probably not
+//        if ((!dependency.isRequired()) && (oldState == TRACKING_OPTIONAL)) {
+//            updateInstance(dependency);
+//        }
     }
 
-    public void dependencyChanged(Dependency dependency) {
-        if (m_state == TRACKING_OPTIONAL) {
-            updateInstance(dependency);
+    public void dependencyChanged(final Dependency dependency) {
+    	State state;
+        synchronized (m_dependencies) {
+        	state = m_state;
         }
+        if (state.isTrackingOptional()) {
+        	m_executor.enqueue(new Runnable() {
+        		public void run() {
+        			updateInstance(dependency);
+        		}
+        	});
+        	m_executor.execute();
+        }
+//    	int state;
+//    	synchronized (m_dependencies) {
+//    		state = m_state;
+//    	}
+//        if (state == TRACKING_OPTIONAL) {
+//            updateInstance(dependency);
+//        }
     }
     
-    public void dependencyUnavailable(Dependency dependency) {
-        if (dependency.isRequired()) {
-            if (m_state == TRACKING_OPTIONAL) {
-                if (!allRequiredDependenciesAvailable()) {
-                    deactivateService();
-                }
-            }
+    public void dependencyUnavailable(final Dependency dependency) {
+    	State oldState, newState;
+        synchronized (m_dependencies) {
+        	oldState = m_state;
+            newState = new State((List) m_dependencies.clone(), !oldState.isInactive());
+            m_state = newState;
         }
-        else {
-            // optional dependency
+        calculateStateChanges(oldState, newState);
+        if (newState.isTrackingOptional()) {
+        	m_executor.enqueue(new Runnable() {
+        		public void run() {
+        			updateInstance(dependency);
+        		}
+        	});
+        	m_executor.execute();
         }
-        if (m_state == TRACKING_OPTIONAL) {
-            updateInstance(dependency);
-        }
+//    	int oldState, newState;
+//    	synchronized (m_dependencies) {
+//    		oldState = m_state;
+//    		newState = calculateState();
+//    	}
+//        if (dependency.isRequired() && (oldState == TRACKING_OPTIONAL) && (newState == WAITING_FOR_REQUIRED)) {
+//            deactivateService();
+//        }
+//        if (oldState == TRACKING_OPTIONAL) {
+//            updateInstance(dependency);
+//        }
+//        // TODO note the slight asymmmetry with dependencyAvailable()
     }
 
     public synchronized void start() {
-        if ((m_state != STARTING) && (m_state != STOPPING)) {
-            throw new IllegalStateException("Cannot start from state " + STATE_NAMES[m_state]);
+    	State oldState, newState;
+        synchronized (m_dependencies) {
+        	oldState = m_state;
+            newState = new State((List) m_dependencies.clone(), true);
+            m_state = newState;
         }
-        startTrackingRequired();
-        if (allRequiredDependenciesAvailable() && (m_state == WAITING_FOR_REQUIRED)) {
-            activateService();
-        }
+        calculateStateChanges(oldState, newState);
+//        if ((m_state != STARTING) && (m_state != STOPPING)) {
+//            throw new IllegalStateException("Cannot start from state " + STATE_NAMES[m_state]);
+//        }
+//        startTrackingRequired();
+//        if (allRequiredDependenciesAvailable() && (m_state == WAITING_FOR_REQUIRED)) {
+//            activateService();
+//        }
     }
 
     public synchronized void stop() {
-        if ((m_state != WAITING_FOR_REQUIRED) && (m_state != TRACKING_OPTIONAL)) {
-            if ((m_state > 0) && (m_state < STATE_NAMES.length)) {
-                throw new IllegalStateException("Cannot stop from state " + STATE_NAMES[m_state]);
-            }
-            throw new IllegalStateException("Cannot stop from unknown state.");
+    	State oldState, newState;
+        synchronized (m_dependencies) {
+        	oldState = m_state;
+            newState = new State((List) m_dependencies.clone(), false);
+            m_state = newState;
         }
-        if (m_state == TRACKING_OPTIONAL) {
-            deactivateService();
-        }
-        stopTrackingRequired();
+        calculateStateChanges(oldState, newState);
+//        if ((m_state != WAITING_FOR_REQUIRED) && (m_state != TRACKING_OPTIONAL)) {
+//            if ((m_state > 0) && (m_state < STATE_NAMES.length)) {
+//                throw new IllegalStateException("Cannot stop from state " + STATE_NAMES[m_state]);
+//            }
+//            throw new IllegalStateException("Cannot stop from unknown state.");
+//        }
+//        if (m_state == TRACKING_OPTIONAL) {
+//            deactivateService();
+//        }
+//        stopTrackingRequired();
     }
 
-    private void activateService() {
+    public synchronized Service setInterface(String serviceName, Dictionary properties) {
+	    ensureNotActive();
+	    m_serviceName = serviceName;
+	    m_serviceProperties = properties;
+	    return this;
+	}
+
+	public synchronized Service setInterface(String[] serviceName, Dictionary properties) {
+	    ensureNotActive();
+	    m_serviceName = serviceName;
+	    m_serviceProperties = properties;
+	    return this;
+	}
+
+	public synchronized Service setCallbacks(String init, String start, String stop, String destroy) {
+	    ensureNotActive();
+	    m_callbackInit = init;
+	    m_callbackStart = start;
+	    m_callbackStop = stop;
+	    m_callbackDestroy = destroy;
+	    return this;
+	}
+
+	public synchronized Service setImplementation(Object implementation) {
+	    ensureNotActive();
+	    m_implementation = implementation;
+	    return this;
+	}
+
+	public String toString() {
+	    return "ServiceImpl[" + m_serviceName + " " + m_implementation + "]";
+	}
+
+	public synchronized Dictionary getServiceProperties() {
+	    if (m_serviceProperties != null) {
+	        return (Dictionary) ((Hashtable) m_serviceProperties).clone();
+	    }
+	    return null;
+	}
+
+	public synchronized void setServiceProperties(Dictionary serviceProperties) {
+	    m_serviceProperties = serviceProperties;
+	    if (isRegistered() && (m_serviceName != null) && (m_serviceProperties != null)) {
+	        m_registration.setProperties(m_serviceProperties);
+	    }
+	}
+
+	// service state listener methods
+	public void addStateListener(ServiceStateListener listener) {
+    	synchronized (m_stateListeners) {
+		    m_stateListeners.add(listener);
+    	}
+    	// when we register as a listener and the service is already started
+    	// make sure we invoke the right callbacks so the listener knows
+    	State state;
+    	synchronized (m_dependencies) {
+    		state = m_state;
+    	}
+    	if (state.isTrackingOptional()) {
+    		listener.starting(this);
+    		listener.started(this);
+    	}
+	}
+
+	public void removeStateListener(ServiceStateListener listener) {
+    	synchronized (m_stateListeners) {
+    		m_stateListeners.remove(listener);
+    	}
+	}
+
+	void removeStateListeners() {
+    	synchronized (m_stateListeners) {
+    		m_stateListeners.clear();
+    	}
+	}
+
+	private void stateListenersStarting() {
+		ServiceStateListener[] list = getListeners();
+		for (int i = 0; i < list.length; i++) {
+			list[i].starting(this);
+		}
+	}
+
+	private void stateListenersStarted() {
+		ServiceStateListener[] list = getListeners();
+		for (int i = 0; i < list.length; i++) {
+			list[i].started(this);
+		}
+	}
+
+	private void stateListenersStopping() {
+		ServiceStateListener[] list = getListeners();
+		for (int i = 0; i < list.length; i++) {
+			list[i].stopping(this);
+		}
+	}
+
+	private void stateListenersStopped() {
+		ServiceStateListener[] list = getListeners();
+		for (int i = 0; i < list.length; i++) {
+			list[i].stopped(this);
+		}
+	}
+
+	private ServiceStateListener[] getListeners() {
+		synchronized (m_stateListeners) {
+			return (ServiceStateListener[]) m_stateListeners.toArray(SERVICE_STATE_LISTENER_TYPE);
+		}
+	}
+
+	private void activateService(State state) {
+		System.out.println("!!!!! activateService: " + this + " " + state);
+		String init, start;
+		synchronized (this) {
+			init = m_callbackInit;
+			start = m_callbackStart;
+		}
         // service activation logic, first we initialize the service instance itself
         // meaning it is created if necessary and the bundle context is set
         initService();
         // then we invoke the init callback so the service can further initialize
         // itself
-        invoke(m_callbackInit);
+        invoke(init);
         // now is the time to configure the service, meaning all required
         // dependencies will be set and any callbacks called
-        configureService();
+        configureService(state);
         // inform the state listeners we're starting
         stateListenersStarting();
         // start tracking optional services
-        startTrackingOptional();
+        startTrackingOptional(state);
         // invoke the start callback, since we're now ready to be used
-        invoke(m_callbackStart);
+        invoke(start);
         // register the service in the framework's service registry
         registerService();
         // inform the state listeners we've started
         stateListenersStarted();
     }
 
-    private void deactivateService() {
+    private void deactivateService(State state) {
+    	String stop, destroy;
+    	synchronized (this) {
+    		stop = m_callbackStop;
+    		destroy = m_callbackDestroy;
+    	}
         // service deactivation logic, first inform the state listeners
         // we're stopping
         stateListenersStopping();
         // then, unregister the service from the framework
         unregisterService();
         // invoke the stop callback
-        invoke(m_callbackStop);
+        invoke(stop);
         // stop tracking optional services
-        stopTrackingOptional();
+        stopTrackingOptional(state);
         // inform the state listeners we've stopped
         stateListenersStopped();
         // invoke the destroy callback
-        invoke(m_callbackDestroy);
+        invoke(destroy);
         // destroy the service instance
-        destroyService();
+        destroyService(state);
     }
 
     private void invoke(String name) {
@@ -266,52 +549,19 @@
         }
     }
     
-    private synchronized void stateListenersStarting() {
-        Iterator i = m_listeners.iterator();
-        while (i.hasNext()) {
-            ServiceStateListener ssl = (ServiceStateListener) i.next();
-            ssl.starting(this);
-        }
-    }
-
-    private synchronized void stateListenersStarted() {
-        Iterator i = m_listeners.iterator();
-        while (i.hasNext()) {
-            ServiceStateListener ssl = (ServiceStateListener) i.next();
-            ssl.started(this);
-        }
-    }
-
-    private synchronized void stateListenersStopping() {
-        Iterator i = m_listeners.iterator();
-        while (i.hasNext()) {
-            ServiceStateListener ssl = (ServiceStateListener) i.next();
-            ssl.stopping(this);
-        }
-    }
-
-    private synchronized void stateListenersStopped() {
-        Iterator i = m_listeners.iterator();
-        while (i.hasNext()) {
-            ServiceStateListener ssl = (ServiceStateListener) i.next();
-            ssl.stopped(this);
-        }
-    }
-
-    private boolean allRequiredDependenciesAvailable() {
-        Iterator i = getDependencies().iterator();
-        while (i.hasNext()) {
-            Dependency dependency = (Dependency) i.next();
-            if (dependency.isRequired() && !dependency.isAvailable()) {
-                return false;
-            }
-        }
-        return true;
-    }
+//    private boolean allRequiredDependenciesAvailable() {
+//        Iterator i = getDependencies().iterator();
+//        while (i.hasNext()) {
+//            Dependency dependency = (Dependency) i.next();
+//            if (dependency.isRequired() && !dependency.isAvailable()) {
+//                return false;
+//            }
+//        }
+//        return true;
+//    }
     
-    private void startTrackingOptional() {
-        m_state = TRACKING_OPTIONAL;
-        Iterator i = getDependencies().iterator();
+    private void startTrackingOptional(State state) {
+        Iterator i = state.getDependencies().iterator();
         while (i.hasNext()) {
             Dependency dependency = (Dependency) i.next();
             if (!dependency.isRequired()) {
@@ -320,9 +570,8 @@
         }
     }
 
-    private void stopTrackingOptional() {
-        m_state = WAITING_FOR_REQUIRED;
-        Iterator i = getDependencies().iterator();
+    private void stopTrackingOptional(State state) {
+        Iterator i = state.getDependencies().iterator();
         while (i.hasNext()) {
             Dependency dependency = (Dependency) i.next();
             if (!dependency.isRequired()) {
@@ -331,9 +580,8 @@
         }
     }
 
-    private void startTrackingRequired() {
-        m_state = WAITING_FOR_REQUIRED;
-        Iterator i = getDependencies().iterator();
+    private void startTrackingRequired(State state) {
+        Iterator i = state.getDependencies().iterator();
         while (i.hasNext()) {
             Dependency dependency = (Dependency) i.next();
             if (dependency.isRequired()) {
@@ -342,9 +590,8 @@
         }
     }
 
-    private void stopTrackingRequired() {
-        m_state = STOPPING;
-        Iterator i = getDependencies().iterator();
+    private void stopTrackingRequired(State state) {
+        Iterator i = state.getDependencies().iterator();
         while (i.hasNext()) {
             Dependency dependency = (Dependency) i.next();
             if (dependency.isRequired()) {
@@ -368,6 +615,9 @@
 	            }
 	        }
 	        else {
+	        	if (m_implementation == null) {
+	        		throw new IllegalStateException("Implementation cannot be null");
+	        	}
 	            m_serviceInstance = m_implementation;
 	        }
 	        // configure the bundle context
@@ -376,14 +626,14 @@
     	}        
     }
     
-    private void configureService() {
+    private void configureService(State state) {
         // configure all services (the optional dependencies might be configured
         // as null objects but that's what we want at this point)
-        configureServices();
+        configureServices(state);
     }
 
-    private void destroyService() {
-        unconfigureServices();
+    private void destroyService(State state) {
+        unconfigureServices(state);
         m_serviceInstance = null;
     }
     
@@ -406,7 +656,7 @@
             catch (IllegalArgumentException iae) {
                 // set the registration to an illegal state object, which will make all invocations on this
                 // wrapper fail with an ISE (which also occurs when the SR becomes invalid)
-                wrapper.setServiceRegistration(ServiceRegistrationImpl.ILLEGAL_STATE);
+                wrapper.setIllegalState();
             }
         }
     }
@@ -481,8 +731,8 @@
         }
     }
 
-    private void configureServices() {
-        Iterator i = getDependencies().iterator();
+    private void configureServices(State state) {
+        Iterator i = state.getDependencies().iterator();
         while (i.hasNext()) {
             Dependency dependency = (Dependency) i.next();
             if (dependency instanceof ServiceDependency) {
@@ -498,8 +748,8 @@
         }
     }
     
-    private void unconfigureServices() {
-        Iterator i = getDependencies().iterator();
+    private void unconfigureServices(State state) {
+        Iterator i = state.getDependencies().iterator();
         while (i.hasNext()) {
             Dependency dependency = (Dependency) i.next();
             if (dependency instanceof ServiceDependency) {
@@ -512,76 +762,21 @@
         }
     }
 
-    public synchronized void addStateListener(ServiceStateListener listener) {
-        m_listeners.add(listener);
-        if (m_state == TRACKING_OPTIONAL) {
-        	listener.starting(this);
-        	listener.started(this);
-        }
-    }
-
-    public synchronized void removeStateListener(ServiceStateListener listener) {
-        m_listeners.remove(listener);
-    }
-
-    synchronized void removeStateListeners() {
-        m_listeners.clear();
-    }
-    
-    public synchronized Service setInterface(String serviceName, Dictionary properties) {
-        ensureNotActive();
-        m_serviceName = serviceName;
-        m_serviceProperties = properties;
-        return this;
-    }
-
-    public synchronized Service setInterface(String[] serviceName, Dictionary properties) {
-        ensureNotActive();
-        m_serviceName = serviceName;
-        m_serviceProperties = properties;
-        return this;
-    }
-    
-    public synchronized Service setCallbacks(String init, String start, String stop, String destroy) {
-        ensureNotActive();
-        m_callbackInit = init;
-        m_callbackStart = start;
-        m_callbackStop = stop;
-        m_callbackDestroy = destroy;
-        return this;
-    }
-    
-    public synchronized Service setImplementation(Object implementation) {
-        ensureNotActive();
-        m_implementation = implementation;
-        return this;
-    }
-    
     private void ensureNotActive() {
-        if ((m_state == TRACKING_OPTIONAL) || (m_state == WAITING_FOR_REQUIRED)) {
+    	State state;
+    	synchronized (m_dependencies) {
+    		state = m_state;
+    	}
+    	if (!state.isInactive()) {
             throw new IllegalStateException("Cannot modify state while active.");
         }
     }
     boolean isRegistered() {
-        return (m_state == TRACKING_OPTIONAL);
-    }
-    
-    public String toString() {
-        return "ServiceImpl[" + m_serviceName + " " + m_implementation + "]";
-    }
-
-    public synchronized Dictionary getServiceProperties() {
-        if (m_serviceProperties != null) {
-            return (Dictionary) ((Hashtable) m_serviceProperties).clone();
-        }
-        return null;
-    }
-
-    public synchronized void setServiceProperties(Dictionary serviceProperties) {
-        m_serviceProperties = serviceProperties;
-        if (isRegistered() && (m_serviceName != null) && (m_serviceProperties != null)) {
-            m_registration.setProperties(m_serviceProperties);
-        }
+    	State state;
+    	synchronized (m_dependencies) {
+    		state = m_state;
+    	}
+        return (state.isTrackingOptional());
     }
     
     static {
diff --git a/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ServiceRegistrationImpl.java b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ServiceRegistrationImpl.java
index e86a0fb..0e00202 100644
--- a/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ServiceRegistrationImpl.java
+++ b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/ServiceRegistrationImpl.java
@@ -29,7 +29,7 @@
  * 
  * @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
  */
-public class ServiceRegistrationImpl implements ServiceRegistration {
+public final class ServiceRegistrationImpl implements ServiceRegistration {
     public static final ServiceRegistrationImpl ILLEGAL_STATE = new ServiceRegistrationImpl();
     private ServiceRegistration m_registration;
 
@@ -38,36 +38,30 @@
     }
     
     public ServiceReference getReference() {
-        ensureRegistration();
-        return m_registration.getReference();
+        return ensureRegistration().getReference();
     }
 
     public void setProperties(Dictionary dictionary) {
-        ensureRegistration();
-        m_registration.setProperties(dictionary);
+        ensureRegistration().setProperties(dictionary);
     }
 
     public void unregister() {
-        ensureRegistration();
-        m_registration.unregister();
+        ensureRegistration().unregister();
     }
 
     public boolean equals(Object obj) {
-        ensureRegistration();
-        return m_registration.equals(obj);
+        return ensureRegistration().equals(obj);
     }
 
     public int hashCode() {
-        ensureRegistration();
-        return m_registration.hashCode();
+        return ensureRegistration().hashCode();
     }
 
     public String toString() {
-        ensureRegistration();
-        return m_registration.toString();
+        return ensureRegistration().toString();
     }
     
-    private synchronized void ensureRegistration() {
+    private synchronized ServiceRegistration ensureRegistration() {
         while (m_registration == null) {
             try {
                 wait();
@@ -77,15 +71,28 @@
                 // service registration ready; if not we wait again
             }
         }
+        // check if we're in an illegal state and throw an exception
         if (ILLEGAL_STATE == m_registration) {
             throw new IllegalStateException("Service is not registered.");
         }
+        return m_registration;
     }
 
+    /**
+     * Sets the service registration and notifies all waiting parties.
+     */
     void setServiceRegistration(ServiceRegistration registration) {
-        m_registration = registration;
         synchronized (this) {
+        	m_registration = registration;
             notifyAll();
         }
     }
+
+    /**
+     * Sets this wrapper to an illegal state, which will cause all threads
+     * that are waiting for this service registration to fail.
+     */
+	void setIllegalState() {
+        setServiceRegistration(ServiceRegistrationImpl.ILLEGAL_STATE);
+	}
 }
diff --git a/dependencymanager/src/main/java/org/apache/felix/dependencymanager/State.java b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/State.java
new file mode 100644
index 0000000..986f70c
--- /dev/null
+++ b/dependencymanager/src/main/java/org/apache/felix/dependencymanager/State.java
@@ -0,0 +1,79 @@
+package org.apache.felix.dependencymanager;
+
+import java.util.List;
+
+/**
+ * Encapsulates the current state of the dependencies of a service.
+ * 
+ * @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
+ */
+public final class State {
+	public static final String[] STATES = { "?", "inactive", "waiting for required", "tracking optional" };
+	public static final int INACTIVE = 1;
+	public static final int WAITING_FOR_REQUIRED = 2;
+	public static final int TRACKING_OPTIONAL = 3;
+	private final List m_deps;
+	private final int m_state;
+	
+	/**
+	 * Creates a new state instance.
+	 * 
+	 * @param deps the dependencies that determine the state
+	 * @param isActive <code>true</code> if the service is active (started)
+	 */
+	public State(List deps, boolean isActive) {
+		m_deps = deps;
+		// only bother calculating dependencies if we're active
+    	if (isActive) {
+    		boolean allRequiredAvailable = true;
+    		for (int i = 0; i < deps.size(); i++) {
+    			Dependency dep = (Dependency) deps.get(i);
+    			if (dep.isRequired()) {
+    				if (!dep.isAvailable()) {
+    					allRequiredAvailable = false;
+    				}
+    			}
+    		}
+	    	if (allRequiredAvailable) {
+	    		m_state = TRACKING_OPTIONAL;
+	    	}
+	    	else {
+	    		m_state = WAITING_FOR_REQUIRED;
+	    	}
+    	}
+    	else {
+    		m_state = INACTIVE;
+    	}
+	}
+	
+	public int getState() {
+		return m_state;
+	}
+	
+	public boolean isInactive() {
+		return m_state == INACTIVE;
+	}
+	
+	public boolean isWaitingForRequired() {
+		return m_state == WAITING_FOR_REQUIRED;
+	}
+	
+	public boolean isTrackingOptional() {
+		return m_state == TRACKING_OPTIONAL;
+	}
+	
+	public List getDependencies() {
+		return m_deps;
+	}
+	
+	public String toString() {
+		StringBuffer buf = new StringBuffer();
+		buf.append("State[" + STATES[m_state] + "|");
+		List deps = m_deps;
+    	for (int i = 0; i < deps.size(); i++) {
+    		Dependency dep = (Dependency) deps.get(i);
+    		buf.append("(" + dep + (dep.isRequired() ? " R" : " O") + (dep.isAvailable() ? " +" : " -") + ")");
+    	}
+    	return buf.toString();
+	}
+}
diff --git a/dependencymanager/src/test/java/org/apache/felix/dependencymanager/ServiceDependencyTest.java b/dependencymanager/src/test/java/org/apache/felix/dependencymanager/ServiceDependencyTest.java
index b6da1a2..fae1491 100644
--- a/dependencymanager/src/test/java/org/apache/felix/dependencymanager/ServiceDependencyTest.java
+++ b/dependencymanager/src/test/java/org/apache/felix/dependencymanager/ServiceDependencyTest.java
@@ -75,7 +75,7 @@
 	 * Defines a service with one required dependency that is
 	 * available. Makes sure the service is started.
 	 */
-	public void testRequiredAvailableDependency() throws Exception {
+//	public void testRequiredAvailableDependency() throws Exception {
 //		// setup the mock objects
 //		IMocksControl ctrl = EasyMock.createControl();
 //		BundleContext context = ctrl.createMock(BundleContext.class);
@@ -100,13 +100,13 @@
 //		dm.remove(service);
 //		// verify the results
 //		ctrl.verify();
-	}
+//	}
 	
 	/**
 	 * Defines a service with an optional dependency that is not available.
 	 * Makes sure the service is started.
 	 */
-	public void testOptionalDependency() throws Exception {
+//	public void testOptionalDependency() throws Exception {
 //		// setup the mock objects
 //		IMocksControl ctrl = EasyMock.createControl();
 //		BundleContext context = ctrl.createMock(BundleContext.class);
@@ -131,7 +131,7 @@
 //		dm.remove(service);
 //		// verify the results
 //		ctrl.verify();
-	}
+//	}
 	
 	public void XtestRequiredAvailableServiceDependency() throws Exception {
 //		// setup the mock objects