svn commit: r1684963 - in /felix/trunk/framework/src: main/java/org/apache/felix/framework/ServiceRegistry.java test/java/org/apache/felix/framework/ServiceRegistryTest.java

[da...@apache.org]

Author: davidb
Date: Thu Jun 11 19:13:17 2015
New Revision: 1684963

URL: http://svn.apache.org/r1684963
Log:
FELIX-4866 Improve Service Registry

The previous code that was contributed as part of FELIX-4866 could cause ungetService() on a Service Factory to be called with a null service object. This change fixes that.

Modified:
    felix/trunk/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
    felix/trunk/framework/src/test/java/org/apache/felix/framework/ServiceRegistryTest.java

Modified: felix/trunk/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
URL: http://svn.apache.org/viewvc/felix/trunk/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java?rev=1684963&r1=1684962&r2=1684963&view=diff
==============================================================================
--- felix/trunk/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java (original)
+++ felix/trunk/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java Thu Jun 11 19:13:17 2015
@@ -417,26 +417,37 @@ public class ServiceRegistry
             int count = usage.m_count.decrementAndGet();
             try
             {
-                if (count == 0)
+                if (count <= 0)
                 {
-                    ServiceHolder holder = usage.m_svcHolderRef.getAndSet(null);
+                    ServiceHolder holder = usage.m_svcHolderRef.get();
                     Object svc = holder != null ? holder.m_service : null;
 
-                    // Remove reference from usages array.
-                    ((ServiceRegistrationImpl.ServiceReferenceImpl) ref)
-                        .getRegistration().ungetService(bundle, svc);
+                    if (svc != null)
+                    {
+                        if (usage.m_svcHolderRef.compareAndSet(holder, null))
+                        {
+                            holder.m_service = null;
+
+                            // Remove reference from usages array.
+                            ((ServiceRegistrationImpl.ServiceReferenceImpl) ref)
+                                .getRegistration().ungetService(bundle, svc);
+
+                        }
+
+                    }
                 }
             }
             finally
             {
-                // Finally, decrement usage count and flush if it goes to zero or
-                // the registration became invalid.
+                if (!reg.isValid())
+                {
+                    usage.m_svcHolderRef.set(null);
+                }
 
                 // If the registration is invalid or the usage count has reached
                 // zero, then flush it.
-                if ((count <= 0) || !reg.isValid())
+                if (count <= 0 || !reg.isValid())
                 {
-                    usage.m_svcHolderRef.set(null);
                     flushUsageCount(bundle, ref, usage);
                 }
             }

Modified: felix/trunk/framework/src/test/java/org/apache/felix/framework/ServiceRegistryTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/framework/src/test/java/org/apache/felix/framework/ServiceRegistryTest.java?rev=1684963&r1=1684962&r2=1684963&view=diff
==============================================================================
--- felix/trunk/framework/src/test/java/org/apache/felix/framework/ServiceRegistryTest.java (original)
+++ felix/trunk/framework/src/test/java/org/apache/felix/framework/ServiceRegistryTest.java Thu Jun 11 19:13:17 2015
@@ -526,7 +526,10 @@ public class ServiceRegistryTest extends
                 (ConcurrentMap<Bundle, UsageCount[]>) getPrivateField(sr, "m_inUseMap");
 
         UsageCount uc = new UsageCount(ref, false);
-        uc.m_svcHolderRef.set(new ServiceHolder());
+        ServiceHolder sh = new ServiceHolder();
+        Object svc = new Object();
+        sh.m_service = svc;
+        uc.m_svcHolderRef.set(sh);
         uc.m_count.incrementAndGet();
 
         Mockito.verify(reg, Mockito.never()).
@@ -538,7 +541,7 @@ public class ServiceRegistryTest extends
         assertNull(inUseMap.get(b));
 
         Mockito.verify(reg, Mockito.times(1)).
-        ungetService(Mockito.isA(Bundle.class), Mockito.any());
+            ungetService(Mockito.isA(Bundle.class), Mockito.eq(svc));
     }
 
     @SuppressWarnings("unchecked")