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.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1684963 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java b/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
index e23f640..dd7d4d0 100644
--- a/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
+++ b/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
@@ -417,26 +417,37 @@
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);
}
}
diff --git a/framework/src/test/java/org/apache/felix/framework/ServiceRegistryTest.java b/framework/src/test/java/org/apache/felix/framework/ServiceRegistryTest.java
index abab92c..061a94b 100644
--- a/framework/src/test/java/org/apache/felix/framework/ServiceRegistryTest.java
+++ b/framework/src/test/java/org/apache/felix/framework/ServiceRegistryTest.java
@@ -526,7 +526,10 @@
(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 @@
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")