Bug 288513 - BundleContext#getServiceReference does not return highest ranked serviceR35x_v20091005

4 files changed, 73 insertions, 10 deletions

diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/AbstractBundleTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/AbstractBundleTests.java
index 7bc90bbdd..00bb0b876 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/AbstractBundleTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/AbstractBundleTests.java
@@ -13,8 +13,7 @@ package org.eclipse.osgi.tests.bundles;
 import org.eclipse.core.tests.harness.CoreTest;
 import org.eclipse.osgi.internal.baseadaptor.StateManager;
 import org.eclipse.osgi.tests.OSGiTestsActivator;
-import org.osgi.framework.BundleEvent;
-import org.osgi.framework.FrameworkEvent;
+import org.osgi.framework.*;
 
 public class AbstractBundleTests extends CoreTest {
 	public static int BUNDLE_LISTENER = 0x01;
@@ -51,6 +50,10 @@ public class AbstractBundleTests extends CoreTest {
 		frameworkListenerResults = null;
 	}
 
+	public BundleContext getContext() {
+		return OSGiTestsActivator.getContext();
+	}
+
 	static public void compareResults(Object[] expectedEvents, Object[] actualEvents) {
 		assertEquals("compareResults length", expectedEvents.length, actualEvents.length);
 		for (int i = 0; i < expectedEvents.length; i++) {
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/serviceregistry/ServiceRegistryTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/serviceregistry/ServiceRegistryTests.java
index 470c80f28..a93f31758 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/serviceregistry/ServiceRegistryTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/serviceregistry/ServiceRegistryTests.java
@@ -455,6 +455,32 @@ public class ServiceRegistryTests extends AbstractBundleTests {
 		}
 	}
 
+	public void testModifiedRanking() {
+		Runnable runIt = new Runnable() {
+			public void run() {
+				// nothing
+			}
+		};
+		Hashtable props = new Hashtable();
+		props.put(getName(), Boolean.TRUE);
+		props.put(Constants.SERVICE_RANKING, new Integer(15));
+		ServiceRegistration reg1 = getContext().registerService(Runnable.class.getName(), runIt, props);
+		props.put(Constants.SERVICE_RANKING, new Integer(10));
+		ServiceRegistration reg2 = getContext().registerService(Runnable.class.getName(), runIt, props);
+		try {
+			assertEquals("wrong service reference", reg1.getReference(), getContext().getServiceReference("java.lang.Runnable")); //$NON-NLS-1$//$NON-NLS-2$
+
+			props.put(Constants.SERVICE_RANKING, new Integer(20));
+			reg2.setProperties(props);
+			assertEquals("wrong service reference", reg2.getReference(), getContext().getServiceReference("java.lang.Runnable")); //$NON-NLS-1$//$NON-NLS-2$
+		} finally {
+			if (reg1 != null)
+				reg1.unregister();
+			if (reg2 != null)
+				reg2.unregister();
+		}
+	}
+
 	private void clearResults(boolean[] results) {
 		for (int i = 0; i < results.length; i++)
 			results[i] = false;
diff --git a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java
index 3030c08ca..5cfa1ca3c 100755
--- a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java
+++ b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/internal/serviceregistry/ServiceRegistrationImpl.java
@@ -154,16 +154,18 @@ public class ServiceRegistrationImpl implements ServiceRegistration, Comparable
 	public void setProperties(Dictionary props) {
 		final ServiceReferenceImpl ref;
 		final ServiceProperties previousProperties;
-		synchronized (registrationLock) {
-			if (state != REGISTERED) { /* in the process of unregistering */
-				throw new IllegalStateException(Msg.SERVICE_ALREADY_UNREGISTERED_EXCEPTION);
-			}
+		synchronized (registry) {
+			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 */
-			previousProperties = this.properties;
-			this.properties = createProperties(props);
+				ref = reference; /* used to publish event outside sync */
+				previousProperties = this.properties;
+				this.properties = createProperties(props);
+			}
+			registry.modifyServiceRegistration(context, this);
 		}
-
 		/* must not hold the registrationLock when this event is published */
 		registry.publishServiceEvent(new ModifiedServiceEvent(ref, previousProperties));
 	}
diff --git a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java
index 40fc2d22c..812647f18 100755
--- a/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java
+++ b/bundles/org.eclipse.osgi/core/framework/org/eclipse/osgi/internal/serviceregistry/ServiceRegistry.java
@@ -809,6 +809,38 @@ public class ServiceRegistry {
 	}
 
 	/**
+	 * Modify the ServiceRegistrationImpl in the data structure.
+	 * 
+	 * @param context The BundleContext of the bundle registering the service.
+	 * @param registration The modified ServiceRegistration.
+	 */
+	/* @GuardedBy("this") */
+	void modifyServiceRegistration(BundleContextImpl context, ServiceRegistrationImpl registration) {
+		// The list of Services published by BundleContextImpl is not sorted, so
+		// we do not need to modify it.
+
+		// Remove the ServiceRegistrationImpl from the list of Services published by Class Name
+		// and then add at the correct index.
+		String[] clazzes = registration.getClasses();
+		int insertIndex;
+		for (int i = 0, size = clazzes.length; i < size; i++) {
+			String clazz = clazzes[i];
+			List services = (List) publishedServicesByClass.get(clazz);
+			services.remove(registration);
+			// The list is sorted, so we must find the proper location to insert
+			insertIndex = -Collections.binarySearch(services, registration) - 1;
+			services.add(insertIndex, registration);
+		}
+
+		// Remove the ServiceRegistrationImpl from the list of all published Services
+		// and then add at the correct index.
+		allPublishedServices.remove(registration);
+		// The list is sorted, so we must find the proper location to insert
+		insertIndex = -Collections.binarySearch(allPublishedServices, registration) - 1;
+		allPublishedServices.add(insertIndex, registration);
+	}
+
+	/**
 	 * Remove the ServiceRegistrationImpl from the data structure.
 	 * 
 	 * @param context The BundleContext of the bundle registering the service.