Bug 572476 - IllegalStateException in ServiceSupplier if bundle is
stopped

- refactor the service handling with
- add test-case (disabled) for Bug 572546

Change-Id: I151e40e62deb2aafccb79be9296784f5e2467f59
Signed-off-by: Dirk Fauth <dirk.fauth@googlemail.com>
Signed-off-by: Christoph Läubrich <laeubi@laeubi-soft.de>
diff --git a/bundles/org.eclipse.e4.core.di.extensions.supplier/.classpath b/bundles/org.eclipse.e4.core.di.extensions.supplier/.classpath
index 4f83b23..e801ebf 100644
--- a/bundles/org.eclipse.e4.core.di.extensions.supplier/.classpath
+++ b/bundles/org.eclipse.e4.core.di.extensions.supplier/.classpath
@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <classpath>
+	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-11"/>
 	<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
 	<classpathentry kind="src" path="src"/>
-	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8"/>
 	<classpathentry kind="output" path="bin"/>
 </classpath>
diff --git a/bundles/org.eclipse.e4.core.di.extensions.supplier/.settings/org.eclipse.pde.ds.annotations.prefs b/bundles/org.eclipse.e4.core.di.extensions.supplier/.settings/org.eclipse.pde.ds.annotations.prefs
new file mode 100644
index 0000000..73a356b
--- /dev/null
+++ b/bundles/org.eclipse.e4.core.di.extensions.supplier/.settings/org.eclipse.pde.ds.annotations.prefs
@@ -0,0 +1,8 @@
+classpath=true
+dsVersion=V1_3
+eclipse.preferences.version=1
+enabled=true
+generateBundleActivationPolicyLazy=true
+path=OSGI-INF
+validationErrorLevel=error
+validationErrorLevel.missingImplicitUnbindMethod=error
diff --git a/bundles/org.eclipse.e4.core.di.extensions.supplier/META-INF/MANIFEST.MF b/bundles/org.eclipse.e4.core.di.extensions.supplier/META-INF/MANIFEST.MF
index 05a5d2c..14c7247 100644
--- a/bundles/org.eclipse.e4.core.di.extensions.supplier/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.e4.core.di.extensions.supplier/META-INF/MANIFEST.MF
@@ -3,11 +3,12 @@
 Bundle-Name: %Bundle-Name
 Bundle-Vendor: %Bundle-Vendor
 Bundle-SymbolicName: org.eclipse.e4.core.di.extensions.supplier
-Bundle-Version: 0.15.800.qualifier
-Bundle-RequiredExecutionEnvironment: JavaSE-1.8
+Bundle-Version: 0.16.0.qualifier
+Bundle-RequiredExecutionEnvironment: JavaSE-11
 Require-Capability: osgi.extender;
   filter:="(&(osgi.extender=osgi.component)(version>=1.3)(!(version>=2.0)))"
-Import-Package: org.eclipse.core.runtime.preferences;version="3.3.0",
+Import-Package: javax.annotation;version="1.3.5",
+ org.eclipse.core.runtime.preferences;version="3.3.0",
  org.eclipse.e4.core.contexts;version="1.6.0",
  org.eclipse.e4.core.di,
  org.eclipse.e4.core.di.annotations;version="1.6.0",
@@ -17,7 +18,7 @@
  org.osgi.service.component.annotations;version="1.3.0";resolution:=optional,
  org.osgi.service.event;version="1.3.0",
  org.osgi.service.log;version="1.3.0",
- javax.annotation;version="1.3.5"
+ org.osgi.util.tracker;version="1.5.3"
 Service-Component: OSGI-INF/org.eclipse.e4.core.di.internal.extensions.OSGiObjectSupplier.xml,
  OSGI-INF/org.eclipse.e4.core.di.internal.extensions.EventObjectSupplier.xml,
  OSGI-INF/org.eclipse.e4.core.di.internal.extensions.PreferencesObjectSupplier.xml,
diff --git a/bundles/org.eclipse.e4.core.di.extensions.supplier/OSGI-INF/org.eclipse.e4.core.di.internal.extensions.ServiceSupplier.xml b/bundles/org.eclipse.e4.core.di.extensions.supplier/OSGI-INF/org.eclipse.e4.core.di.internal.extensions.ServiceSupplier.xml
index f64f106..d3a2871 100644
--- a/bundles/org.eclipse.e4.core.di.extensions.supplier/OSGI-INF/org.eclipse.e4.core.di.internal.extensions.ServiceSupplier.xml
+++ b/bundles/org.eclipse.e4.core.di.extensions.supplier/OSGI-INF/org.eclipse.e4.core.di.internal.extensions.ServiceSupplier.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<scr:component xmlns:scr="http://www.osgi.org/xmlns/scr/v1.1.0" name="org.eclipse.e4.core.di.internal.extensions.ServiceSupplier">
+<scr:component xmlns:scr="http://www.osgi.org/xmlns/scr/v1.1.0" activate="activate" deactivate="deactivate" name="org.eclipse.e4.core.di.internal.extensions.ServiceSupplier">
    <property name="dependency.injection.annotation" type="String" value="org.eclipse.e4.core.di.extensions.Service"/>
    <property name="event.topics" value="org/eclipse/e4/core/contexts/IEclipseContext/DISPOSE"/>
    <service>
diff --git a/bundles/org.eclipse.e4.core.di.extensions.supplier/pom.xml b/bundles/org.eclipse.e4.core.di.extensions.supplier/pom.xml
index 09b6039..8261b8b 100644
--- a/bundles/org.eclipse.e4.core.di.extensions.supplier/pom.xml
+++ b/bundles/org.eclipse.e4.core.di.extensions.supplier/pom.xml
@@ -19,6 +19,6 @@
   </parent>
   <groupId>org.eclipse.e4</groupId>
   <artifactId>org.eclipse.e4.core.di.extensions.supplier</artifactId>
-  <version>0.15.800-SNAPSHOT</version>
+  <version>0.16.0-SNAPSHOT</version>
   <packaging>eclipse-plugin</packaging>
 </project>
diff --git a/bundles/org.eclipse.e4.core.di.extensions.supplier/src/org/eclipse/e4/core/di/internal/extensions/ServiceSupplier.java b/bundles/org.eclipse.e4.core.di.extensions.supplier/src/org/eclipse/e4/core/di/internal/extensions/ServiceSupplier.java
index a9cfad8..fb893d8 100644
--- a/bundles/org.eclipse.e4.core.di.extensions.supplier/src/org/eclipse/e4/core/di/internal/extensions/ServiceSupplier.java
+++ b/bundles/org.eclipse.e4.core.di.extensions.supplier/src/org/eclipse/e4/core/di/internal/extensions/ServiceSupplier.java
@@ -11,36 +11,44 @@
  * Contributors:
  *     Tom Schindl <tom.schindl@bestsolution.at> - initial API and implementation
  *     Dirk Fauth <dirk.fauth@googlemail.com> - Bug 513563
+ *     Christoph Läubrich - Bug 572476
  *******************************************************************************/
 package org.eclipse.e4.core.di.internal.extensions;
 
+import java.lang.Thread.UncaughtExceptionHandler;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
-import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.Predicate;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.BooleanSupplier;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import org.eclipse.e4.core.contexts.IEclipseContext;
 import org.eclipse.e4.core.di.IInjector;
-import org.eclipse.e4.core.di.InjectionException;
 import org.eclipse.e4.core.di.extensions.Service;
 import org.eclipse.e4.core.di.suppliers.ExtendedObjectSupplier;
 import org.eclipse.e4.core.di.suppliers.IObjectDescriptor;
 import org.eclipse.e4.core.di.suppliers.IRequestor;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
-import org.osgi.framework.Constants;
+import org.osgi.framework.BundleEvent;
+import org.osgi.framework.Filter;
 import org.osgi.framework.FrameworkUtil;
 import org.osgi.framework.InvalidSyntaxException;
-import org.osgi.framework.ServiceEvent;
-import org.osgi.framework.ServiceListener;
 import org.osgi.framework.ServiceReference;
+import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Deactivate;
 import org.osgi.service.component.annotations.Reference;
 import org.osgi.service.component.annotations.ReferenceCardinality;
 import org.osgi.service.component.annotations.ReferencePolicy;
@@ -48,6 +56,9 @@
 import org.osgi.service.event.EventHandler;
 import org.osgi.service.log.Logger;
 import org.osgi.service.log.LoggerFactory;
+import org.osgi.util.tracker.BundleTracker;
+import org.osgi.util.tracker.BundleTrackerCustomizer;
+import org.osgi.util.tracker.ServiceTracker;
 
 /**
  * Supplier for {@link Service}
@@ -60,166 +71,107 @@
 	LoggerFactory factory;
 	Logger logger;
 
-	static class ServiceHandler implements ServiceListener {
-		private final ServiceSupplier supplier;
-		final Set<IRequestor> requestors = new HashSet<>();
-		private final BundleContext bundleContext;
-		private final Class<?> serviceType;
+	private volatile BundleTracker<ServiceSupplierContext> supplierContextTracker;
 
-		public ServiceHandler(ServiceSupplier supplier, BundleContext bundleContext, Class<?> serviceType) {
-			this.supplier = supplier;
-			this.bundleContext = bundleContext;
-			this.serviceType = serviceType;
-		}
+	@Activate
+	void activate(BundleContext bundleContext) {
+		this.supplierContextTracker = new BundleTracker<>(bundleContext,
+				Bundle.RESOLVED | Bundle.STARTING | Bundle.ACTIVE | Bundle.STOPPING,
+				new BundleTrackerCustomizer<ServiceSupplierContext>() {
 
-		@Override
-		public void serviceChanged(ServiceEvent event) {
-			cleanup();
-
-			synchronized (this.supplier) {
-				String[] data = (String[]) event.getServiceReference().getProperty(Constants.OBJECTCLASS);
-				for (String d : data) {
-					if (this.serviceType.getName().equals(d)) {
-						this.requestors.forEach( r -> {
-							try {
-								r.resolveArguments(false);
-								r.execute();
-							} catch (InjectionException e) {
-								this.supplier.logError("Injection failed", e); //$NON-NLS-1$
-							}
-						});
-						break;
+					@Override
+					public ServiceSupplierContext addingBundle(Bundle bundle, BundleEvent event) {
+						return new ServiceSupplierContext(bundle, bundleContext,
+								(t, e) -> logError("Injection failed", e));//$NON-NLS-1$
 					}
-				}
-			}
-		}
 
-		public void cleanup() {
-			synchronized (this.supplier) {
-				Predicate<IRequestor> pr = IRequestor::isValid;
-				this.requestors.removeIf(pr.negate());
-
-				if (this.requestors.isEmpty()) {
-					Map<Class<?>, ServiceHandler> map = this.supplier.handlerList.get(this.bundleContext);
-					if (map != null) {
-						map.remove(this.serviceType);
-
-						if (map.isEmpty()) {
-							this.supplier.handlerList.remove(this.bundleContext);
+					@Override
+					public void modifiedBundle(Bundle bundle, BundleEvent event, ServiceSupplierContext object) {
+						switch (event.getType()) {
+						case BundleEvent.STOPPING:
+							object.serviceTracker.values().forEach(ServiceTracker::close);
+							break;
+						case BundleEvent.STARTED:
+						case BundleEvent.STOPPED:
+							object.refreshServices();
+							break;
+						default:
+							break;
 						}
 					}
 
-					this.bundleContext.removeServiceListener(this);
-					return;
-				}
-			}
-		}
+					@Override
+					public void removedBundle(Bundle bundle, BundleEvent event, ServiceSupplierContext object) {
+						object.dispose();
+					}
+				});
+		this.supplierContextTracker.open();
 	}
 
-	Map<BundleContext, Map<Class<?>, ServiceHandler>> handlerList = new ConcurrentHashMap<>();
+	@Deactivate
+	void deactivate(BundleContext bundleContext) {
+		this.supplierContextTracker.close();
+	}
 
 	@Override
 	public Object get(IObjectDescriptor descriptor, IRequestor requestor, boolean track, boolean group) {
 		Type desiredType = descriptor.getDesiredType();
-		Bundle b = FrameworkUtil.getBundle(requestor.getRequestingObjectClass());
+		Bundle bundle = FrameworkUtil.getBundle(requestor.getRequestingObjectClass());
+		ServiceSupplierContext supplierContext;
+		if (bundle == null || (supplierContext = supplierContextTracker.getObject(bundle)) == null
+				|| supplierContext.disposed) {
+			// bundle has gone...
+			return IInjector.NOT_A_VALUE;
+		}
 		Service qualifier = descriptor.getQualifier(Service.class);
-
 		if (desiredType instanceof ParameterizedType) {
 			ParameterizedType t = (ParameterizedType) desiredType;
 			if (t.getRawType() == Collections.class || t.getRawType() == List.class) {
-
-				return handleCollection(b, t.getActualTypeArguments()[0], requestor, track && qualifier.dynamic(), qualifier);
+				return handleCollection(supplierContext, t.getActualTypeArguments()[0], requestor,
+						track && qualifier.dynamic(), qualifier);
 			}
 		}
 
-		return handleSingle(b, desiredType, requestor, track && qualifier.dynamic(), qualifier);
+		return handleSingle(supplierContext, desiredType, requestor, track && qualifier.dynamic(), qualifier);
 	}
 
-	private Object handleSingle(Bundle bundle, Type t, IRequestor requestor, boolean track, Service qualifier) {
-		BundleContext context = bundle.getBundleContext();
-		if (context == null) {
-			context = FrameworkUtil.getBundle(getClass()).getBundleContext();
-		}
-
-		@SuppressWarnings("unchecked")
-		Class<Object> cl = t instanceof ParameterizedType ? (Class<Object>) ((ParameterizedType) t).getRawType() : (Class<Object>) t;
-		Object result = IInjector.NOT_A_VALUE;
+	private Object handleSingle(ServiceSupplierContext supplierContext, Type t, IRequestor requestor, boolean track,
+			Service qualifier) {
+		Class<?> cls = t instanceof ParameterizedType ? (Class<?>) ((ParameterizedType) t).getRawType() : (Class<?>) t;
 		try {
-			String filter = qualifier.filterExpression() != null && !qualifier.filterExpression().isEmpty()
-					? qualifier.filterExpression()
-					: null;
-
-			ServiceReference<?>[] serviceReferences = context.getServiceReferences(cl.getName(), filter);
-			if (serviceReferences != null) {
-				Arrays.sort(serviceReferences);
-
-				if (serviceReferences.length > 0) {
-					result = context.getService(serviceReferences[serviceReferences.length - 1]);
+			Filter filter = supplierContext.getFilter(qualifier.filterExpression());
+			ServiceSupplierTracker<?> tracker = supplierContext.getTracker(cls);
+			return tracker.getAndTrack(filter, track ? requestor : null, s -> {
+				Optional<?> service = s.findFirst();
+				if (service.isPresent()) {
+					return service.get();
 				}
-			}
+				return IInjector.NOT_A_VALUE;
+			});
 		} catch (InvalidSyntaxException e) {
 			logError("Invalid filter expression", e); //$NON-NLS-1$
+			return IInjector.NOT_A_VALUE;
 		}
-
-		if (track) {
-			trackService(context, cl, requestor);
-		}
-
-		return result;
 	}
 
-	private List<Object> handleCollection(Bundle bundle, Type t, IRequestor requestor, boolean track, Service qualifier) {
-		List<Object> rv = new ArrayList<>();
-
-		BundleContext context = bundle.getBundleContext();
-		if (context == null) {
-			context = FrameworkUtil.getBundle(getClass()).getBundleContext();
-		}
-
-		@SuppressWarnings("unchecked")
-		Class<Object> cl = t instanceof ParameterizedType ? (Class<Object>) ((ParameterizedType) t).getRawType() : (Class<Object>) t;
+	private Object handleCollection(ServiceSupplierContext supplierContext, Type t, IRequestor requestor, boolean track,
+			Service qualifier) {
+		Class<?> cls = t instanceof ParameterizedType ? (Class<?>) ((ParameterizedType) t).getRawType() : (Class<?>) t;
 		try {
-			String filter = qualifier.filterExpression() != null && ! qualifier.filterExpression().isEmpty() ? qualifier.filterExpression() : null;
-
-			ServiceReference<?>[] serviceReferences = context.getServiceReferences(cl.getName(), filter);
-			if( serviceReferences != null ) {
-				Arrays.sort(serviceReferences);
-
-				for (ServiceReference<?> serviceReference : serviceReferences) {
-					rv.add(context.getService(serviceReference));
-				}
-			}
-
-			// We are in the wrong order
-			Collections.reverse(rv);
-
-			if (track) {
-				trackService(context, cl, requestor);
-			}
+			Filter filter = supplierContext.getFilter(qualifier.filterExpression());
+			ServiceSupplierTracker<?> tracker = supplierContext.getTracker(cls);
+			return tracker.getAndTrack(filter, track ? requestor : null, s -> s.collect(Collectors.toList()));
 		} catch (InvalidSyntaxException e) {
 			logError("Invalid filter expression", e); //$NON-NLS-1$
+			return IInjector.NOT_A_VALUE;
 		}
-
-		return rv;
-	}
-
-	private synchronized void trackService(BundleContext context, Class<?> serviceClass, IRequestor requestor) {
-		Map<Class<?>, ServiceHandler> map = this.handlerList.computeIfAbsent(context, k -> new ConcurrentHashMap<>());
-		ServiceHandler handler = map.computeIfAbsent(serviceClass, cl -> {
-			ServiceHandler h = new ServiceHandler(this,context, serviceClass);
-			context.addServiceListener(h);
-			return h;
-		});
-		handler.requestors.add(requestor);
 	}
 
 	/**
 	 * Method to log an exception.
 	 *
-	 * @param message
-	 *            The log message.
-	 * @param e
-	 *            The exception that should be logged.
+	 * @param message The log message.
+	 * @param e       The exception that should be logged.
 	 */
 	void logError(String message, Throwable e) {
 		Logger log = this.logger;
@@ -233,11 +185,8 @@
 
 	@Override
 	public void handleEvent(Event event) {
-		this.handlerList.forEach((bc, map) -> {
-			map.forEach((cl, sh) -> {
-				sh.cleanup();
-			});
-		});
+		supplierContextTracker.getTracked().values().stream().flatMap(ctx -> ctx.serviceTracker.values().stream())
+				.forEach(ServiceSupplierTracker::cleanup);
 	}
 
 	@Reference(cardinality = ReferenceCardinality.OPTIONAL, policy = ReferencePolicy.DYNAMIC)
@@ -252,4 +201,167 @@
 			this.logger = null;
 		}
 	}
+
+	private static final class ServiceSupplierContext {
+
+		private Bundle bundle;
+		private BundleContext serviceBundleContext;
+		final Map<Class<?>, ServiceSupplierTracker<?>> serviceTracker = new ConcurrentHashMap<>();
+		volatile boolean disposed;
+		private UncaughtExceptionHandler exceptionHandler;
+
+		ServiceSupplierContext(Bundle bundle, BundleContext serviceBundleContext,
+				UncaughtExceptionHandler exceptionHandler) {
+			this.bundle = bundle;
+			this.serviceBundleContext = serviceBundleContext;
+			this.exceptionHandler = exceptionHandler;
+		}
+
+		public Filter getFilter(String filterExpression) throws InvalidSyntaxException {
+			if (filterExpression == null || filterExpression.isEmpty() || disposed) {
+				return null;
+			}
+			return serviceBundleContext.createFilter(filterExpression);
+		}
+
+		@SuppressWarnings("unchecked")
+		public <T> ServiceSupplierTracker<T> getTracker(Class<T> serviceClass) {
+			return (ServiceSupplierTracker<T>) serviceTracker.computeIfAbsent(serviceClass, cls -> {
+				BundleContext bundleContext = bundle.getBundleContext();
+				if (bundleContext == null || bundle.getState() == Bundle.STOPPING) {
+					bundleContext = serviceBundleContext;
+				}
+				ServiceSupplierTracker<T> tracker = new ServiceSupplierTracker<>(bundleContext, serviceClass,
+						exceptionHandler);
+				tracker.open();
+				return tracker;
+			});
+
+		}
+
+		void refreshServices() {
+			for (Iterator<Entry<Class<?>, ServiceSupplierTracker<?>>> iter = serviceTracker.entrySet().iterator(); iter
+					.hasNext();) {
+				Entry<Class<?>, ServiceSupplierTracker<?>> entry = iter.next();
+				serviceTracker.compute(entry.getKey(), (k, v) -> {
+					if (v != null) {
+						v.close();
+						v.update(null);
+					}
+					return null;
+				});
+			}
+		}
+
+		void dispose() {
+			disposed = true;
+			refreshServices();
+		}
+
+	}
+
+	private static final class ServiceSupplierTracker<T> extends ServiceTracker<T, T> {
+
+		Set<IRequestor> trackedRequestors = ConcurrentHashMap.newKeySet();
+		private UncaughtExceptionHandler exceptionHandler;
+		private AtomicReference<CompletableFuture<?>> pending = new AtomicReference<>();
+
+		public ServiceSupplierTracker(BundleContext context, Class<T> clazz,
+				UncaughtExceptionHandler exceptionHandler) {
+			super(context, clazz, null);
+			this.exceptionHandler = exceptionHandler;
+		}
+
+		public synchronized <R> R getAndTrack(Filter f, IRequestor requestor, Function<Stream<T>, R> extractor) {
+			cleanup();
+			Stream<T> stream = getTracked().entrySet().stream().filter(entry -> f == null || f.match(entry.getKey()))
+					.map(Entry::getValue);
+			R value = extractor.apply(stream);
+			if (requestor != null) {
+				trackedRequestors.add(requestor);
+			}
+			return value;
+		}
+
+		void cleanup() {
+			for (Iterator<IRequestor> iterator = trackedRequestors.iterator(); iterator.hasNext();) {
+				IRequestor requestor = iterator.next();
+				if (!requestor.isValid()) {
+					iterator.remove();
+				}
+			}
+		}
+
+		@Override
+		public T addingService(ServiceReference<T> reference) {
+			T service = super.addingService(reference);
+			if (service != null) {
+				update(() -> getService(reference) != null);
+			}
+			return service;
+		}
+
+		@Override
+		public void removedService(ServiceReference<T> reference, T service) {
+			super.removedService(reference, service);
+			update(() -> getService(reference) == null);
+		}
+
+		@Override
+		public void modifiedService(ServiceReference<T> reference, T service) {
+			super.modifiedService(reference, service);
+			update(null);
+		}
+
+		private void update(BooleanSupplier check) {
+			IRequestor[] requestors = trackedRequestors.toArray(IRequestor[]::new);
+			if (requestors.length == 0) {
+				return;
+			}
+			CompletableFuture<Void> execution = new CompletableFuture<>();
+			CompletableFuture<?> pendingExecution = pending.getAndSet(execution);
+			if (pendingExecution != null) {
+				pendingExecution.cancel(true);
+			}
+			execution.completeAsync(() -> {
+				if (check != null) {
+					do {
+						Thread.yield();
+					} while (!check.getAsBoolean() && !execution.isCancelled());
+				}
+				if (!execution.isCancelled()) {
+					refreshRequestors(requestors);
+				}
+				return null;
+			}).handle((v, e) -> {
+				if (e instanceof CancellationException) {
+					// request was canceled
+					return null;
+				}
+				if (e != null) {
+					exceptionHandler.uncaughtException(Thread.currentThread(), e);
+				}
+				pending.compareAndSet(execution, null);
+				return null;
+			});
+		}
+
+		private synchronized void refreshRequestors(IRequestor[] requestors) {
+			for (IRequestor requestor : requestors) {
+				if (requestor.isValid()) {
+					try {
+						requestor.resolveArguments(false);
+						requestor.execute();
+					} catch (RuntimeException e) {
+						exceptionHandler.uncaughtException(Thread.currentThread(), e);
+					}
+				} else {
+					trackedRequestors.remove(requestor);
+				}
+
+			}
+		}
+
+	}
+
 }
diff --git a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/di/extensions/ServiceSupplierTestCase.java b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/di/extensions/ServiceSupplierTestCase.java
index 8304d76..15bdd88 100644
--- a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/di/extensions/ServiceSupplierTestCase.java
+++ b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/di/extensions/ServiceSupplierTestCase.java
@@ -8,6 +8,7 @@
 import java.util.ArrayList;
 import java.util.Hashtable;
 import java.util.List;
+import java.util.function.BooleanSupplier;
 
 import javax.inject.Inject;
 
@@ -17,6 +18,7 @@
 import org.eclipse.e4.core.di.annotations.Optional;
 import org.eclipse.e4.core.di.extensions.Service;
 import org.junit.After;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.FrameworkUtil;
@@ -25,21 +27,25 @@
 
 public class ServiceSupplierTestCase {
 	public static class TestBean {
-		TestService service;
-		List<TestService> serviceList;
-		int serviceInjectionCount;
-		int serviceListInjectionCount;
+		volatile TestService service;
+		volatile List<TestService> serviceList;
+		volatile int serviceInjectionCount;
+		volatile int serviceListInjectionCount;
+		volatile boolean updated;
+		volatile boolean listUpdated;
 
 		@Inject
 		public void setService(@Service TestService service) {
 			this.service = service;
 			this.serviceInjectionCount++;
+			updated = true;
 		}
 
 		@Inject
 		public void setServiceList(@Service List<TestService> serviceList) {
 			this.serviceList = serviceList;
 			this.serviceListInjectionCount++;
+			listUpdated = true;
 		}
 	}
 
@@ -113,7 +119,7 @@
 		assertSame(FilterServiceB.class, bean.serviceList.get(1).getClass());
 	}
 
-	@Test
+	@Test(timeout = 30000)
 	public void testDynamicAdd() {
 		BundleContext context = FrameworkUtil.getBundle(getClass()).getBundleContext();
 		IEclipseContext serviceContext = EclipseContextFactory.getServiceContext(context);
@@ -127,9 +133,14 @@
 		};
 
 		Hashtable<String, Object> properties = new Hashtable<>();
-		properties.put("service.ranking", 100); //$NON-NLS-1$
+		properties.put("service.ranking", 10000); //$NON-NLS-1$
+		bean.updated = false;
+		bean.listUpdated = false;
+		System.out.println("-------------------");
 		this.registrations.add(context.registerService(TestService.class, t, properties));
-
+		spinWait(() -> bean.updated && bean.listUpdated && bean.service == t);
+		System.out.println("-------------------");
+		System.out.println("Final " + bean.service);
 		assertSame(t, bean.service);
 		assertEquals(2, bean.serviceInjectionCount);
 
@@ -143,8 +154,10 @@
 
 		properties = new Hashtable<>();
 		properties.put("service.ranking", Integer.valueOf(-1)); //$NON-NLS-1$
+		bean.updated = false;
+		bean.listUpdated = false;
 		this.registrations.add(context.registerService(TestService.class, t2, properties));
-
+		spinWait(() -> bean.updated && bean.listUpdated);
 		assertSame(t, bean.service);
 		assertEquals(3, bean.serviceInjectionCount);
 
@@ -154,6 +167,12 @@
 		assertSame(t, bean.serviceList.get(0));
 	}
 
+	private void spinWait(BooleanSupplier condition) {
+		while (!condition.getAsBoolean() && !Thread.currentThread().isInterrupted()) {
+			Thread.onSpinWait();
+		}
+	}
+
 	@Test
 	public void testDynamicAddRemove() {
 		BundleContext context = FrameworkUtil.getBundle(getClass()).getBundleContext();
@@ -169,19 +188,22 @@
 
 		Hashtable<String, Object> properties = new Hashtable<>();
 		properties.put("service.ranking", 52); //$NON-NLS-1$
+		bean.updated = false;
+		bean.listUpdated = false;
 		this.registrations.add(context.registerService(TestService.class, t, properties));
-
+		spinWait(() -> bean.updated && bean.listUpdated);
 		assertSame(t, bean.service);
 		assertEquals(2, bean.serviceInjectionCount);
 
 		assertEquals(2, bean.serviceListInjectionCount);
 		assertEquals(5, bean.serviceList.size());
 		assertSame(t, bean.serviceList.get(0));
-
+		bean.updated = false;
+		bean.listUpdated = false;
 		ServiceRegistration<?> registration = this.registrations.get(0);
 		registration.unregister();
 		this.registrations.remove(registration);
-
+		spinWait(() -> bean.updated && bean.listUpdated);
 		assertEquals(3, bean.serviceInjectionCount);
 		assertEquals(3, bean.serviceListInjectionCount);
 
@@ -191,11 +213,13 @@
 		assertSame(SampleServiceB.class, bean.serviceList.get(1).getClass());
 	}
 
-	@Test
+	@Test(timeout = 30000)
 	public void testCleanup() {
 		BundleContext context = FrameworkUtil.getBundle(getClass()).getBundleContext();
-		IEclipseContext iec = EclipseContextFactory.getServiceContext(context).createChild();
+		IEclipseContext eclipseContext = EclipseContextFactory.getServiceContext(context);
+		IEclipseContext iec = eclipseContext.createChild();
 		TestBean bean = ContextInjectionFactory.make(TestBean.class, iec);
+		TestBean bean2 = ContextInjectionFactory.make(TestBean.class, eclipseContext);
 		iec.dispose();
 
 		TestService t = new TestService() {
@@ -204,8 +228,9 @@
 
 		Hashtable<String, Object> properties = new Hashtable<>();
 		properties.put("service.ranking", 2); //$NON-NLS-1$
+		bean2.listUpdated = false;
 		this.registrations.add(context.registerService(TestService.class, t, properties));
-
+		spinWait(() -> bean2.listUpdated);
 		assertSame(SampleServiceA.class, bean.service.getClass());
 	}
 
@@ -255,4 +280,28 @@
 			Thread.sleep(100);
 		}
 	}
+
+	@Test
+	@Ignore("See Bug 572546")
+	public void testUselessUpdates() throws InterruptedException {
+		BundleContext context = FrameworkUtil.getBundle(getClass()).getBundleContext();
+		IEclipseContext serviceContext = EclipseContextFactory.getServiceContext(context);
+		TestBean bean = ContextInjectionFactory.make(TestBean.class, serviceContext);
+		assertEquals(1, bean.serviceInjectionCount);
+		assertEquals(1, bean.serviceListInjectionCount);
+		TestService current = bean.service;
+		TestService t = new TestService() {
+			// nothing todo
+		};
+
+		Hashtable<String, Object> properties = new Hashtable<>();
+		properties.put("service.ranking", -1); //$NON-NLS-1$
+		bean.updated = false;
+		bean.listUpdated = false;
+		this.registrations.add(context.registerService(TestService.class, t, properties));
+		spinWait(() -> bean.listUpdated);
+		Thread.sleep(100);
+		assertEquals(current, bean.service);
+		assertEquals(1, bean.serviceInjectionCount);
+	}
 }