big 199103: release fix to HEAD

6 files changed, 415 insertions, 278 deletions

diff --git a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/BundleContextImpl.java b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/BundleContextImpl.java
index c5dce185b..ad7fd9477 100644
--- a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/BundleContextImpl.java
+++ b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/BundleContextImpl.java
@@ -34,20 +34,21 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 	public static final String PROP_SCOPE_SERVICE_EVENTS = "osgi.scopeServiceEvents"; //$NON-NLS-1$
 	public static final boolean scopeEvents = Boolean.valueOf(FrameworkProperties.getProperty(PROP_SCOPE_SERVICE_EVENTS, "true")).booleanValue(); //$NON-NLS-1$
 	/** true if the bundle context is still valid */
-	private boolean valid;
+	private volatile boolean valid;
 
 	/** Bundle object this context is associated with. */
 	// This slot is accessed directly by the Framework instead of using
 	// the getBundle() method because the Framework needs access to the bundle
 	// even when the context is invalid while the close method is being called.
-	protected BundleHost bundle;
+	final BundleHost bundle;
 
 	/** Internal framework object. */
-	protected Framework framework;
+	final Framework framework;
 
-	/** Services that bundle has used. Key is ServiceReference,
+	/** Services that bundle has used. Key is ServiceRegistrationImpl,
 	 Value is ServiceUse */
-	protected Hashtable servicesInUse;
+	/* @GuardedBy("contextLock") */
+	private HashMap/*<ServiceRegistrationImpl, ServiceUse>*/servicesInUse;
 
 	/** Listener list for bundle's BundleListeners */
 	protected EventListeners bundleEvent;
@@ -65,7 +66,7 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 	protected BundleActivator activator;
 
 	/** private object for locking */
-	protected Object contextLock = new Object();
+	private final Object contextLock = new Object();
 
 	/**
 	 * Construct a BundleContext which wrappers the framework for a
@@ -81,7 +82,9 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 		bundleEventSync = null;
 		serviceEvent = null;
 		frameworkEvent = null;
-		servicesInUse = null;
+		synchronized (contextLock) {
+			servicesInUse = null;
+		}
 		activator = null;
 	}
 
@@ -134,10 +137,12 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 		}
 
 		/* service's used by the bundle, if any, are released. */
-		if (servicesInUse != null) {
-			int usedSize;
-			ServiceReference[] usedRefs = null;
-
+		int usedSize;
+		ServiceRegistrationImpl[] usedServices = null;
+		synchronized (contextLock) {
+			if (servicesInUse == null) {
+				return;
+			}
 			synchronized (servicesInUse) {
 				usedSize = servicesInUse.size();
 
@@ -146,23 +151,23 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 						Debug.println("Releasing services"); //$NON-NLS-1$
 					}
 
-					usedRefs = new ServiceReference[usedSize];
+					usedServices = new ServiceRegistrationImpl[usedSize];
 
-					Enumeration refsEnum = servicesInUse.keys();
+					Iterator regsIter = servicesInUse.keySet().iterator();
 					for (int i = 0; i < usedSize; i++) {
-						usedRefs[i] = (ServiceReference) refsEnum.nextElement();
+						usedServices[i] = (ServiceRegistrationImpl) regsIter.next();
 					}
 				}
 			}
+		}
 
-			for (int i = 0; i < usedSize; i++) {
-				((ServiceReferenceImpl) usedRefs[i]).registration.releaseService(this);
-			}
+		for (int i = 0; i < usedSize; i++) {
+			usedServices[i].releaseService(this);
+		}
 
+		synchronized (contextLock) {
 			servicesInUse = null;
 		}
-
-		bundle = null;
 	}
 
 	/**
@@ -306,6 +311,7 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 		ServiceListener filteredListener = new FilteredServiceListener(filter, listener, this);
 
 		synchronized (framework.serviceEvent) {
+			checkValid();
 			if (serviceEvent == null) {
 				serviceEvent = new EventListeners();
 				framework.serviceEvent.addListener(this, this);
@@ -379,7 +385,6 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 	 */
 	public void addBundleListener(BundleListener listener) {
 		checkValid();
-
 		if (Debug.DEBUG && Debug.DEBUG_EVENTS) {
 			String listenerName = listener.getClass().getName() + "@" + Integer.toHexString(System.identityHashCode(listener)); //$NON-NLS-1$
 			Debug.println("addBundleListener[" + bundle + "](" + listenerName + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
@@ -388,6 +393,7 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 		if (listener instanceof SynchronousBundleListener) {
 			framework.checkAdminPermission(getBundle(), AdminPermission.LISTENER);
 			synchronized (framework.bundleEventSync) {
+				checkValid();
 				if (bundleEventSync == null) {
 					bundleEventSync = new EventListeners();
 					framework.bundleEventSync.addListener(this, this);
@@ -397,6 +403,7 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 			}
 		} else {
 			synchronized (framework.bundleEvent) {
+				checkValid();
 				if (bundleEvent == null) {
 					bundleEvent = new EventListeners();
 					framework.bundleEvent.addListener(this, this);
@@ -467,6 +474,7 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 		}
 
 		synchronized (framework.frameworkEvent) {
+			checkValid();
 			if (frameworkEvent == null) {
 				frameworkEvent = new EventListeners();
 				framework.frameworkEvent.addListener(this, this);
@@ -654,7 +662,9 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 	 * @return A {@link ServiceRegistrationImpl} object for use by the bundle.
 	 */
 	protected ServiceRegistrationImpl createServiceRegistration(String[] clazzes, Object service, Dictionary properties) {
-		return (new ServiceRegistrationImpl(this, clazzes, service, properties));
+		ServiceRegistrationImpl registration = new ServiceRegistrationImpl(this, clazzes, service);
+		registration.register(properties);
+		return registration;
 	}
 
 	/**
@@ -874,7 +884,7 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 		synchronized (contextLock) {
 			if (servicesInUse == null)
 				// Cannot predict how many services a bundle will use, start with a small table.
-				servicesInUse = new Hashtable(10);
+				servicesInUse = new HashMap(10);
 		}
 
 		ServiceRegistrationImpl registration = ((ServiceReferenceImpl) reference).registration;
@@ -1126,47 +1136,61 @@ public class BundleContextImpl implements BundleContext, EventDispatcher {
 	 * @see ServiceReferenceImpl
 	 */
 	protected ServiceReferenceImpl[] getServicesInUse() {
-		if (servicesInUse == null) {
-			return (null);
-		}
+		synchronized (contextLock) {
+			if (servicesInUse == null) {
+				return null;
+			}
 
-		synchronized (servicesInUse) {
-			int size = servicesInUse.size();
+			synchronized (servicesInUse) {
+				int size = servicesInUse.size();
 
-			if (size == 0) {
-				return (null);
-			}
+				if (size == 0) {
+					return null;
+				}
 
-			ServiceReferenceImpl[] references = new ServiceReferenceImpl[size];
-			int refcount = 0;
+				ServiceReferenceImpl[] references = new ServiceReferenceImpl[size];
+				int refcount = 0;
 
-			Enumeration refsEnum = servicesInUse.keys();
+				Iterator regsIter = servicesInUse.keySet().iterator();
 
-			for (int i = 0; i < size; i++) {
-				ServiceReferenceImpl reference = (ServiceReferenceImpl) refsEnum.nextElement();
+				for (int i = 0; i < size; i++) {
+					ServiceRegistrationImpl service = (ServiceRegistrationImpl) regsIter.next();
 
-				try {
-					framework.checkGetServicePermission(reference.registration.clazzes);
-				} catch (SecurityException se) {
-					continue;
+					try {
+						framework.checkGetServicePermission(service.clazzes);
+					} catch (SecurityException se) {
+						continue;
+					}
+
+					references[refcount] = (ServiceReferenceImpl) service.getReference();
+					refcount++;
 				}
 
-				references[refcount] = reference;
-				refcount++;
-			}
+				if (refcount < size) {
+					if (refcount == 0) {
+						return null;
+					}
 
-			if (refcount < size) {
-				if (refcount == 0) {
-					return (null);
-				}
+					ServiceReferenceImpl[] refs = references;
+					references = new ServiceReferenceImpl[refcount];
 
-				ServiceReferenceImpl[] refs = references;
-				references = new ServiceReferenceImpl[refcount];
+					System.arraycopy(refs, 0, references, 0, refcount);
+				}
 
-				System.arraycopy(refs, 0, references, 0, refcount);
+				return references;
 			}
+		}
+	}
 
-			return (references);
+	/** 
+	 * Return the map of ServiceRegistrationImpl to ServiceUse for services being
+	 * used by this context.
+	 * @return A map of ServiceRegistrationImpl to ServiceUse for services in use by 
+	 * this context.
+	 */
+	Map getServicesInUseMap() {
+		synchronized (contextLock) {
+			return servicesInUse;
 		}
 	}
 
diff --git a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/FilterImpl.java b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/FilterImpl.java
index c6691bba8..b8d8ea0ad 100644
--- a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/FilterImpl.java
+++ b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/FilterImpl.java
@@ -159,7 +159,7 @@ public class FilterImpl implements Filter /* since Framework 1.1 */{
 	 * <code>false</code> otherwise.
 	 */
 	public boolean match(org.osgi.framework.ServiceReference reference) {
-		return match0(((ServiceReferenceImpl) reference).registration.properties);
+		return match0(((ServiceReferenceImpl) reference).registration.getProperties());
 	}
 
 	/**
@@ -386,7 +386,7 @@ public class FilterImpl implements Filter /* since Framework 1.1 */{
 	 * return <code>true</code>. Otherwise, return <code>false</code>.
 	 */
 	protected boolean match(ServiceReferenceImpl reference) {
-		return match0(reference.registration.properties);
+		return match0(reference.registration.getProperties());
 	}
 
 	/**
diff --git a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/FilteredServiceListener.java b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/FilteredServiceListener.java
index fe4b115df..60c8c478b 100644
--- a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/FilteredServiceListener.java
+++ b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/FilteredServiceListener.java
@@ -79,7 +79,7 @@ public class FilteredServiceListener implements ServiceListener {
 
 		if (Debug.DEBUG && Debug.DEBUG_EVENTS) {
 			String listenerName = this.getClass().getName() + "@" + Integer.toHexString(System.identityHashCode(this)); //$NON-NLS-1$
-			Debug.println("filterServiceEvent(" + listenerName + ", \"" + filter + "\", " + reference.registration.properties + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
+			Debug.println("filterServiceEvent(" + listenerName + ", \"" + filter + "\", " + reference.registration.getProperties() + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
 		}
 
 		if ((filter == null || filter.match(reference)) && (allservices || context.isAssignableTo(reference))) {
diff --git a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/ServiceReferenceImpl.java b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/ServiceReferenceImpl.java
index fc6be6f04..d4698b768 100644
--- a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/ServiceReferenceImpl.java
+++ b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/ServiceReferenceImpl.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2003, 2005 IBM Corporation and others.
+ * Copyright (c) 2003, 2008 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -33,7 +33,7 @@ import org.osgi.framework.ServiceReference;
 
 public class ServiceReferenceImpl implements ServiceReference, Comparable {
 	/** Registered Service object. */
-	protected ServiceRegistrationImpl registration;
+	final ServiceRegistrationImpl registration;
 
 	/**
 	 * Construct a reference.
@@ -41,6 +41,10 @@ public class ServiceReferenceImpl implements ServiceReference, Comparable {
 	 */
 	protected ServiceReferenceImpl(ServiceRegistrationImpl registration) {
 		this.registration = registration;
+		/* We must not dereference registration in the constructor
+		 * since it is "leaked" to us in the ServiceRegistrationImpl
+		 * constructor.
+		 */
 	}
 
 	/**
@@ -110,21 +114,21 @@ public class ServiceReferenceImpl implements ServiceReference, Comparable {
 	}
 
 	/**
-	 * Return the serviceid of the ServiceRegistration.
+	 * Return the service id of the ServiceRegistration.
 	 *
 	 * @return service.id of the service
 	 */
 	protected long getId() {
-		return (registration.serviceid);
+		return registration.getId();
 	}
 
 	/**
-	 * Return the serviceranking of the ServiceRegistration.
+	 * Return the service ranking of the ServiceRegistration.
 	 *
 	 * @return service.ranking of the service
 	 */
 	protected int getRanking() {
-		return (registration.serviceranking);
+		return registration.getRanking();
 	}
 
 	/**
diff --git a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/ServiceRegistrationImpl.java b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/ServiceRegistrationImpl.java
index d7a7672aa..9ee488af8 100644
--- a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/ServiceRegistrationImpl.java
+++ b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/ServiceRegistrationImpl.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2003, 2007 IBM Corporation and others.
+ * Copyright (c) 2003, 2008 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -32,75 +32,94 @@ import org.osgi.framework.*;
  * bundle wants to keep its service registered, it should keep the
  * ServiceRegistration object referenced.
  */
-//TODO That's kind of big
 public class ServiceRegistrationImpl implements ServiceRegistration {
 	/** Reference to this registration. */
-	protected ServiceReferenceImpl reference;
+	/* @GuardedBy("registrationLock") */
+	private ServiceReferenceImpl reference;
 
 	/** Internal framework object. */
-	protected Framework framework;
+	final Framework framework;
 
 	/** context which registered this service. */
-	protected BundleContextImpl context;
+	final BundleContextImpl context;
 
 	/** bundle which registered this service. */
-	protected AbstractBundle bundle;
+	final AbstractBundle bundle;
 
 	/** list of contexts using the service.
-	 * Access to this should be protected by the registrationLock */
-	protected ArrayList contextsUsing;
+	/* @GuardedBy("registrationLock") */
+	private ArrayList contextsUsing;
 
 	/** service classes for this registration. */
-	protected String[] clazzes;
+	final String[] clazzes;
 
 	/** service object for this registration. */
-	protected Object service;
+	final Object service;
 
 	/** properties for this registration. */
-	protected Properties properties;
+	/* @GuardedBy("registrationLock") */
+	private Properties properties;
 
 	/** service id. */
-	protected long serviceid;
+	/* @GuardedBy("registrationLock") */
+	private long serviceid;
 
 	/** service ranking. */
-	protected int serviceranking;
+	/* @GuardedBy("registrationLock") */
+	private int serviceranking;
 
 	/* internal object to use for synchronization */
-	protected Object registrationLock = new Object();
+	private final Object registrationLock = new Object();
 
 	/** The registration state */
-	protected int state = REGISTERED;
-	public static final int REGISTERED = 0x00;
-	public static final int UNREGISTERING = 0x01;
-	public static final int UNREGISTERED = 0x02;
+	/* @GuardedBy("registrationLock") */
+	private int state = REGISTERED;
+	private static final int REGISTERED = 0x00;
+	private static final int UNREGISTERING = 0x01;
+	private static final int UNREGISTERED = 0x02;
 
 	/**
 	 * Construct a ServiceRegistration and register the service
 	 * in the framework's service registry.
 	 *
 	 */
-	protected ServiceRegistrationImpl(BundleContextImpl context, String[] clazzes, Object service, Dictionary properties) {
+	protected ServiceRegistrationImpl(BundleContextImpl context, String[] clazzes, Object service) {
 		this.context = context;
 		this.bundle = context.bundle;
 		this.framework = context.framework;
 		this.clazzes = clazzes; /* must be set before calling createProperties. */
 		this.service = service;
-		this.contextsUsing = null;
-		this.reference = new ServiceReferenceImpl(this);
+		synchronized (registrationLock) {
+			this.contextsUsing = null;
+			/* We leak this from the constructor here, but it is ok
+			 * because the ServiceReferenceImpl constructor only
+			 * stores the value in a final field without
+			 * otherwise using it.
+			 */
+			this.reference = new ServiceReferenceImpl(this);
+		}
+	}
 
+	/**
+	 * Call after constructing this object to complete the registration.
+	 */
+	void register(Dictionary props) {
+		final ServiceReference ref;
 		synchronized (framework.serviceRegistry) {
-			serviceid = framework.getNextServiceId(); /* must be set before calling createProperties. */
-			this.properties = createProperties(properties); /* must be valid after unregister is called. */
-
+			context.checkValid();
+			synchronized (registrationLock) {
+				ref = reference; /* used to publish event outside sync */
+				serviceid = framework.getNextServiceId(); /* must be set before calling createProperties. */
+				this.properties = createProperties(props); /* must be valid after unregister is called. */
+			}
 			if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
 				Debug.println("registerService[" + bundle + "](" + this + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
 			}
-
 			framework.serviceRegistry.publishService(context, this);
 		}
 
 		/* must not hold the registrations lock when this event is published */
-		framework.publishServiceEvent(ServiceEvent.REGISTERED, reference);
+		framework.publishServiceEvent(ServiceEvent.REGISTERED, ref);
 	}
 
 	/**
@@ -134,35 +153,36 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 	 * @see BundleContextImpl#ungetService
 	 */
 	public void unregister() {
-		synchronized (registrationLock) {
-			if (state != REGISTERED) /* in the process of unregisterING */
-			{
-				throw new IllegalStateException(Msg.SERVICE_ALREADY_UNREGISTERED_EXCEPTION);
-			}
+		final ServiceReference ref;
+		synchronized (framework.serviceRegistry) {
+			synchronized (registrationLock) {
+				if (state != REGISTERED) /* in the process of unregisterING */
+				{
+					throw new IllegalStateException(Msg.SERVICE_ALREADY_UNREGISTERED_EXCEPTION);
+				}
 
-			/* remove this object from the service registry */
-			if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
-				Debug.println("unregisterService[" + bundle + "](" + this + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
-			}
+				/* remove this object from the service registry */
+				if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
+					Debug.println("unregisterService[" + bundle + "](" + this + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+				}
 
-			synchronized (framework.serviceRegistry) {
 				framework.serviceRegistry.unpublishService(context, this);
-			}
 
-			state = UNREGISTERING; /* mark unregisterING */
+				state = UNREGISTERING; /* mark unregisterING */
+				ref = reference; /* used to publish event outside sync */
+			}
 		}
 
 		/* must not hold the registrationLock when this event is published */
-		framework.publishServiceEvent(ServiceEvent.UNREGISTERING, reference);
-
-		/* we have published the ServiceEvent, now mark the service fully unregistered */
-		service = null;
-		state = UNREGISTERED;
+		framework.publishServiceEvent(ServiceEvent.UNREGISTERING, ref);
 
 		int size = 0;
 		BundleContextImpl[] users = null;
 
 		synchronized (registrationLock) {
+			/* we have published the ServiceEvent, now mark the service fully unregistered */
+			state = UNREGISTERED;
+
 			if (contextsUsing != null) {
 				size = contextsUsing.size();
 
@@ -180,14 +200,13 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 			releaseService(users[i]);
 		}
 
-		contextsUsing = null;
+		synchronized (registrationLock) {
+			contextsUsing = null;
 
-		reference = null; /* mark registration dead */
-		context = null;
+			reference = null; /* mark registration dead */
+		}
 
-		/* These fields must be valid after unregister is called:
-		 *   properties
-		 */
+		/* The properties field must remain valid after unregister completes. */
 	}
 
 	/**
@@ -201,14 +220,16 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 	public org.osgi.framework.ServiceReference getReference() {
 		/* use reference instead of unregistered so that ServiceFactorys, called
 		 * by releaseService after the registration is unregistered, can
-		 * get the ServiceReference. Note this technically may voilate the spec
+		 * get the ServiceReference. Note this technically may violate the spec
 		 * but makes more sense.
 		 */
-		if (reference == null) {
-			throw new IllegalStateException(Msg.SERVICE_ALREADY_UNREGISTERED_EXCEPTION);
-		}
+		synchronized (registrationLock) {
+			if (reference == null) {
+				throw new IllegalStateException(Msg.SERVICE_ALREADY_UNREGISTERED_EXCEPTION);
+			}
 
-		return (reference);
+			return reference;
+		}
 	}
 
 	/**
@@ -234,17 +255,19 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 	 * parameter contains case variants of the same key name.
 	 */
 	public void setProperties(Dictionary props) {
+		final ServiceReference ref;
 		synchronized (registrationLock) {
 			if (state != REGISTERED) /* in the process of unregistering */
 			{
 				throw new IllegalStateException(Msg.SERVICE_ALREADY_UNREGISTERED_EXCEPTION);
 			}
 
+			ref = reference; /* used to publish event outside sync */
 			this.properties = createProperties(props);
 		}
 
 		/* must not hold the registrationLock when this event is published */
-		framework.publishServiceEvent(ServiceEvent.MODIFIED, reference);
+		framework.publishServiceEvent(ServiceEvent.MODIFIED, ref);
 	}
 
 	/**
@@ -254,7 +277,8 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 	 * @param props The properties for this service.
 	 * @return A Properties object for this ServiceRegistration.
 	 */
-	protected Properties createProperties(Dictionary props) {
+	/* @GuardedBy("registrationLock") */
+	private Properties createProperties(Dictionary props) {
 		Properties properties = new Properties(props);
 
 		properties.set(Constants.OBJECTCLASS, clazzes, true);
@@ -264,7 +288,17 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 
 		serviceranking = (ranking instanceof Integer) ? ((Integer) ranking).intValue() : 0;
 
-		return (properties);
+		return properties;
+	}
+
+	/**
+	 * Return the properties object. This is for framework internal use only.
+	 * @return The service registration's properties.
+	 */
+	Properties getProperties() {
+		synchronized (registrationLock) {
+			return properties;
+		}
 	}
 
 	/**
@@ -281,7 +315,7 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 	 */
 	protected Object getProperty(String key) {
 		synchronized (registrationLock) {
-			return (properties.getProperty(key));
+			return properties.getProperty(key);
 		}
 	}
 
@@ -297,7 +331,27 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 	 */
 	protected String[] getPropertyKeys() {
 		synchronized (registrationLock) {
-			return (properties.getPropertyKeys());
+			return properties.getPropertyKeys();
+		}
+	}
+
+	/**
+	 * Return the service id for this service.
+	 * @return The service id for this service.
+	 */
+	long getId() {
+		synchronized (registrationLock) {
+			return serviceid;
+		}
+	}
+
+	/**
+	 * Return the service ranking for this service.
+	 * @return The service ranking for this service.
+	 */
+	int getRanking() {
+		synchronized (registrationLock) {
+			return serviceranking;
 		}
 	}
 
@@ -311,11 +365,13 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 	 * @return The bundle which registered the service.
 	 */
 	protected AbstractBundle getBundle() {
-		if (reference == null) {
-			return (null);
-		}
+		synchronized (registrationLock) {
+			if (reference == null) {
+				return null;
+			}
 
-		return (bundle);
+			return bundle;
+		}
 	}
 
 	/**
@@ -326,37 +382,64 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 	 */
 	protected Object getService(BundleContextImpl user) {
 		synchronized (registrationLock) {
-			if (state == UNREGISTERED) /* service unregistered */
-			{
-				return (null);
+			if (state == UNREGISTERED) { /* service unregistered */
+				return null;
 			}
+		}
+		if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
+			Debug.println("getService[" + user.bundle + "](" + this + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+		}
 
-			if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
-				Debug.println("getService[" + user.bundle + "](" + this + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+		Map servicesInUse = user.getServicesInUseMap();
+		if (servicesInUse == null) { /* user is closed */
+			user.checkValid(); /* throw exception */
+		}
+		/* Use a while loop to support retry if a call to a ServiceFactory fails */
+		while (true) {
+			ServiceUse use;
+			boolean added = false;
+			/* Obtain the ServiceUse object for this service by bundle user */
+			synchronized (servicesInUse) {
+				user.checkValid();
+				use = (ServiceUse) servicesInUse.get(this);
+				if (use == null) {
+					/* if this is the first use of the service
+					 * optimistically record this service is being used. */
+					use = new ServiceUse(user, this);
+					added = true;
+					synchronized (registrationLock) {
+						servicesInUse.put(this, use);
+						if (contextsUsing == null) {
+							contextsUsing = new ArrayList(10);
+						}
+						contextsUsing.add(user);
+					}
+				}
 			}
 
-			Hashtable servicesInUse = user.servicesInUse;
-
-			ServiceUse use = (ServiceUse) servicesInUse.get(reference);
-
-			if (use == null) {
-				use = new ServiceUse(user, this);
-
-				Object service = use.getService();
-
-				if (service != null) {
-					servicesInUse.put(reference, use);
-
-					if (contextsUsing == null) {
-						contextsUsing = new ArrayList(10);
+			/* Obtain and return the service object */
+			synchronized (use) {
+				/* if another thread removed the ServiceUse, then
+				 * go back to the top and start again */
+				synchronized (servicesInUse) {
+					user.checkValid();
+					if (servicesInUse.get(this) != use) {
+						continue;
 					}
-
-					contextsUsing.add(user);
 				}
-
-				return (service);
-			} else {
-				return (use.getService());
+				Object serviceObject = use.getService();
+				/* if the service factory failed to return an object and
+				 * we created the service use, then remove the 
+				 * optimistically added ServiceUse. */
+				if ((serviceObject == null) && added) {
+					synchronized (servicesInUse) {
+						synchronized (registrationLock) {
+							servicesInUse.remove(this);
+							contextsUsing.remove(user);
+						}
+					}
+				}
+				return serviceObject;
 			}
 		}
 	}
@@ -372,32 +455,39 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 	protected boolean ungetService(BundleContextImpl user) {
 		synchronized (registrationLock) {
 			if (state == UNREGISTERED) {
-				return (false);
-			}
-
-			if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
-				String bundle = (user.bundle == null) ? "" : user.bundle.toString(); //$NON-NLS-1$
-				Debug.println("ungetService[" + bundle + "](" + this + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+				return false;
 			}
+		}
 
-			Hashtable servicesInUse = user.servicesInUse;
+		if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
+			Debug.println("ungetService[" + user.bundle + "](" + this + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+		}
 
-			if (servicesInUse != null) {
-				ServiceUse use = (ServiceUse) servicesInUse.get(reference);
+		Map servicesInUse = user.getServicesInUseMap();
+		if (servicesInUse == null) {
+			return false;
+		}
 
-				if (use != null) {
-					if (use.ungetService()) {
-						/* use count is now zero */
-						servicesInUse.remove(reference);
+		ServiceUse use;
+		synchronized (servicesInUse) {
+			use = (ServiceUse) servicesInUse.get(this);
+			if (use == null) {
+				return false;
+			}
+		}
 
+		synchronized (use) {
+			if (use.ungetService()) {
+				/* use count is now zero */
+				synchronized (servicesInUse) {
+					synchronized (registrationLock) {
+						servicesInUse.remove(this);
 						contextsUsing.remove(user);
 					}
-					return (true);
 				}
 			}
-
-			return (false);
 		}
+		return true;
 	}
 
 	/**
@@ -407,30 +497,35 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 	 */
 	protected void releaseService(BundleContextImpl user) {
 		synchronized (registrationLock) {
-			if (reference == null) /* registration dead */
-			{
+			if (reference == null) { /* registration dead */
 				return;
 			}
+		}
 
-			if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
-				String bundle = (user.bundle == null) ? "" : user.bundle.toString(); //$NON-NLS-1$
-				Debug.println("releaseService[" + bundle + "](" + this + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
-			}
-
-			Hashtable servicesInUse = user.servicesInUse;
-
-			if (servicesInUse != null) {
-				ServiceUse use = (ServiceUse) servicesInUse.remove(reference);
+		if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
+			Debug.println("releaseService[" + user.bundle + "](" + this + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+		}
 
-				if (use != null) {
-					use.releaseService();
-					// contextsUsing may have been nulled out by use.releaseService() if the registrant bundle
-					// is listening for events and unregisters the service
-					if (contextsUsing != null)
-						contextsUsing.remove(user);
+		Map servicesInUse = user.getServicesInUseMap();
+		if (servicesInUse == null) {
+			return;
+		}
+		ServiceUse use;
+		synchronized (servicesInUse) {
+			synchronized (registrationLock) {
+				use = (ServiceUse) servicesInUse.remove(this);
+				if (use == null) {
+					return;
 				}
+				// contextsUsing may have been nulled out by use.releaseService() if the registrant bundle
+				// is listening for events and unregisters the service
+				if (contextsUsing != null)
+					contextsUsing.remove(user);
 			}
 		}
+		synchronized (use) {
+			use.releaseService();
+		}
 	}
 
 	/**
@@ -441,21 +536,21 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 	protected AbstractBundle[] getUsingBundles() {
 		synchronized (registrationLock) {
 			if (state == UNREGISTERED) /* service unregistered */
-				return (null);
+				return null;
 
 			if (contextsUsing == null)
-				return (null);
+				return null;
 
 			int size = contextsUsing.size();
 			if (size == 0)
-				return (null);
+				return null;
 
 			/* Copy list of BundleContext into an array of Bundle. */
 			AbstractBundle[] bundles = new AbstractBundle[size];
 			for (int i = 0; i < size; i++)
 				bundles[i] = ((BundleContextImpl) contextsUsing.get(i)).bundle;
 
-			return (bundles);
+			return bundles;
 		}
 	}
 
@@ -479,9 +574,9 @@ public class ServiceRegistrationImpl implements ServiceRegistration {
 		}
 
 		sb.append("}="); //$NON-NLS-1$
-		sb.append(properties);
+		sb.append(getProperties().toString());
 
-		return (sb.toString());
+		return sb.toString();
 	}
 
 	/**
diff --git a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/ServiceUse.java b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/ServiceUse.java
index 90b572be2..87f3a4eea 100644
--- a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/ServiceUse.java
+++ b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/framework/internal/core/ServiceUse.java
@@ -26,22 +26,25 @@ import org.osgi.framework.*;
 public class ServiceUse {
 	/** ServiceFactory object if the service instance represents a factory,
 	 null otherwise */
-	protected ServiceFactory factory;
-	/** Service object either registered or that returned by
-	 ServiceFactory.getService() */
-	protected Object service;
+	private final ServiceFactory factory;
 	/** BundleContext associated with this service use */
-	protected BundleContextImpl context;
+	private final BundleContextImpl context;
 	/** ServiceDescription of the registered service */
-	protected ServiceRegistrationImpl registration;
+	private final ServiceRegistrationImpl registration;
+
+	/** Service object either registered or that returned by
+	 ServiceFactory.getService() */
+	/* @GuardedBy("this") */
+	private Object cachedService;
 	/** bundle's use count for this service */
-	protected int useCount;
+	/* @GuardedBy("this") */
+	private int useCount;
 
 	/** Internal framework object. */
 
 	/**
 	 * Constructs a service use encapsulating the service object.
-	 * Objects of this class should be constrcuted while holding the
+	 * Objects of this class should be constructed while holding the
 	 * registrations lock.
 	 *
 	 * @param   context bundle getting the service
@@ -54,11 +57,11 @@ public class ServiceUse {
 
 		Object service = registration.service;
 		if (service instanceof ServiceFactory) {
-			factory = (ServiceFactory) service;
-			this.service = null;
+			this.factory = (ServiceFactory) service;
+			this.cachedService = null;
 		} else {
 			this.factory = null;
-			this.service = service;
+			this.cachedService = service;
 		}
 	}
 
@@ -97,54 +100,58 @@ public class ServiceUse {
 	 * @return A service object for the service associated with this
 	 * reference.
 	 */
+	/* @GuardedBy("this") */
 	protected Object getService() {
-		if ((useCount == 0) && (factory != null)) {
-			AbstractBundle factorybundle = registration.context.bundle;
-			Object service;
-
-			try {
-				service = AccessController.doPrivileged(new PrivilegedAction() {
-					public Object run() {
-						return factory.getService(context.bundle, registration);
-					}
-				});
-			} catch (Throwable t) {
-				if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
-					Debug.println(factory + ".getService() exception: " + t.getMessage()); //$NON-NLS-1$
-					Debug.printStackTrace(t);
-				}
-				// allow the adaptor to handle this unexpected error
-				context.framework.adaptor.handleRuntimeError(t);
-				BundleException be = new BundleException(NLS.bind(Msg.SERVICE_FACTORY_EXCEPTION, factory.getClass().getName(), "getService"), t); //$NON-NLS-1$ 
-				context.framework.publishFrameworkEvent(FrameworkEvent.ERROR, factorybundle, be);
-				return (null);
-			}
+		if ((useCount > 0) || (factory == null)) {
+			useCount++;
+			return cachedService;
+		}
 
-			if (service == null) {
-				if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
-					Debug.println(factory + ".getService() returned null."); //$NON-NLS-1$
+		if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
+			Debug.println("getService[factory=" + registration.context.bundle + "](" + context.bundle + "," + registration + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
+		}
+		final Object service;
+		try {
+			service = AccessController.doPrivileged(new PrivilegedAction() {
+				public Object run() {
+					return factory.getService(context.bundle, registration);
 				}
+			});
+		} catch (Throwable t) {
+			if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
+				Debug.println(factory + ".getService() exception: " + t.getMessage()); //$NON-NLS-1$
+				Debug.printStackTrace(t);
+			}
+			// allow the adaptor to handle this unexpected error
+			context.framework.adaptor.handleRuntimeError(t);
+			BundleException be = new BundleException(NLS.bind(Msg.SERVICE_FACTORY_EXCEPTION, factory.getClass().getName(), "getService"), t); //$NON-NLS-1$ 
+			context.framework.publishFrameworkEvent(FrameworkEvent.ERROR, registration.context.bundle, be);
+			return null;
+		}
 
-				BundleException be = new BundleException(NLS.bind(Msg.SERVICE_OBJECT_NULL_EXCEPTION, factory.getClass().getName()));
-				context.framework.publishFrameworkEvent(FrameworkEvent.ERROR, factorybundle, be);
-
-				return (null);
+		if (service == null) {
+			if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
+				Debug.println(factory + ".getService() returned null."); //$NON-NLS-1$
 			}
 
-			String[] clazzes = registration.clazzes;
-			String invalidService = BundleContextImpl.checkServiceClass(clazzes, service);
-			if (invalidService != null) {
-				if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
-					Debug.println("Service object is not an instanceof " + invalidService); //$NON-NLS-1$
-				}
-				throw new IllegalArgumentException(NLS.bind(Msg.SERVICE_NOT_INSTANCEOF_CLASS_EXCEPTION, invalidService));
+			BundleException be = new BundleException(NLS.bind(Msg.SERVICE_OBJECT_NULL_EXCEPTION, factory.getClass().getName()));
+			context.framework.publishFrameworkEvent(FrameworkEvent.ERROR, registration.context.bundle, be);
+			return null;
+		}
+
+		String[] clazzes = registration.clazzes;
+		String invalidService = BundleContextImpl.checkServiceClass(clazzes, service);
+		if (invalidService != null) {
+			if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
+				Debug.println("Service object is not an instanceof " + invalidService); //$NON-NLS-1$
 			}
-			this.service = service;
+			throw new IllegalArgumentException(NLS.bind(Msg.SERVICE_NOT_INSTANCEOF_CLASS_EXCEPTION, invalidService));
 		}
 
+		this.cachedService = service;
 		useCount++;
 
-		return (this.service);
+		return service;
 	}
 
 	/**
@@ -175,41 +182,45 @@ public class ServiceUse {
 	 * @return <code>true</code> if the context bundle's use count for the service
 	 *         is zero otherwise <code>false</code>.
 	 */
+	/* @GuardedBy("this") */
 	protected boolean ungetService() {
 		if (useCount == 0) {
-			return (true);
+			return true;
 		}
 
 		useCount--;
+		if (useCount > 0) {
+			return false;
+		}
 
-		if (useCount == 0) {
-			if (factory != null) {
-				try {
-					AccessController.doPrivileged(new PrivilegedAction() {
-						public Object run() {
-							factory.ungetService(context.bundle, registration, service);
-
-							return null;
-						}
-					});
-				} catch (Throwable t) {
-					if (Debug.DEBUG && Debug.DEBUG_GENERAL) {
-						Debug.println(factory + ".ungetService() exception"); //$NON-NLS-1$
-						Debug.printStackTrace(t);
-					}
-
-					AbstractBundle factorybundle = registration.context.bundle;
-					BundleException be = new BundleException(NLS.bind(Msg.SERVICE_FACTORY_EXCEPTION, factory.getClass().getName(), "ungetService"), t); //$NON-NLS-1$ 
-					context.framework.publishFrameworkEvent(FrameworkEvent.ERROR, factorybundle, be);
-				}
+		if (factory == null) {
+			return true;
+		}
 
-				service = null;
+		final Object service = cachedService;
+		cachedService = null;
+
+		if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
+			Debug.println("ungetService[factory=" + registration.context.bundle + "](" + context.bundle + "," + registration + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
+		}
+		try {
+			AccessController.doPrivileged(new PrivilegedAction() {
+				public Object run() {
+					factory.ungetService(context.bundle, registration, service);
+					return null;
+				}
+			});
+		} catch (Throwable t) {
+			if (Debug.DEBUG && Debug.DEBUG_GENERAL) {
+				Debug.println(factory + ".ungetService() exception"); //$NON-NLS-1$
+				Debug.printStackTrace(t);
 			}
 
-			return (true);
+			BundleException be = new BundleException(NLS.bind(Msg.SERVICE_FACTORY_EXCEPTION, factory.getClass().getName(), "ungetService"), t); //$NON-NLS-1$ 
+			context.framework.publishFrameworkEvent(FrameworkEvent.ERROR, registration.context.bundle, be);
 		}
 
-		return (false);
+		return true;
 	}
 
 	/**
@@ -221,30 +232,33 @@ public class ServiceUse {
 	 * is called to release the service object for the bundle.
 	 * </ol>
 	 */
+	/* @GuardedBy("this") */
 	protected void releaseService() {
-		if ((useCount > 0) && (factory != null)) {
-			try {
-				AccessController.doPrivileged(new PrivilegedAction() {
-					public Object run() {
-						factory.ungetService(context.bundle, registration, service);
-
-						return null;
-					}
-				});
-			} catch (Throwable t) {
-				if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
-					Debug.println(factory + ".ungetService() exception"); //$NON-NLS-1$
-					Debug.printStackTrace(t);
-				}
+		if ((useCount == 0) || (factory == null)) {
+			return;
+		}
+		final Object service = cachedService;
+		cachedService = null;
+		useCount = 0;
 
-				AbstractBundle factorybundle = registration.context.bundle;
-				BundleException be = new BundleException(NLS.bind(Msg.SERVICE_FACTORY_EXCEPTION, factory.getClass().getName(), "ungetService"), t); //$NON-NLS-1$ 
-				context.framework.publishFrameworkEvent(FrameworkEvent.ERROR, factorybundle, be);
+		if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
+			Debug.println("releaseService[factory=" + registration.context.bundle + "](" + context.bundle + "," + registration + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
+		}
+		try {
+			AccessController.doPrivileged(new PrivilegedAction() {
+				public Object run() {
+					factory.ungetService(context.bundle, registration, service);
+					return null;
+				}
+			});
+		} catch (Throwable t) {
+			if (Debug.DEBUG && Debug.DEBUG_SERVICES) {
+				Debug.println(factory + ".ungetService() exception"); //$NON-NLS-1$
+				Debug.printStackTrace(t);
 			}
 
-			service = null;
+			BundleException be = new BundleException(NLS.bind(Msg.SERVICE_FACTORY_EXCEPTION, factory.getClass().getName(), "ungetService"), t); //$NON-NLS-1$ 
+			context.framework.publishFrameworkEvent(FrameworkEvent.ERROR, registration.context.bundle, be);
 		}
-
-		useCount = 0;
 	}
 }