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")