FELIX-4806 : Ungetting service through ServiceObjects might throw IllegalArgumentException

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1661592 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/framework/src/main/java/org/apache/felix/framework/BundleContextImpl.java b/framework/src/main/java/org/apache/felix/framework/BundleContextImpl.java
index c5a1011..398ccc8 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleContextImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleContextImpl.java
@@ -20,12 +20,10 @@
 
 import java.io.File;
 import java.io.InputStream;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Dictionary;
-import java.util.List;
 
 import org.apache.felix.framework.ext.FelixBundleContext;
 import org.osgi.framework.AdminPermission;
@@ -553,88 +551,37 @@
     {
         private final ServiceReference<S> m_ref;
 
-        private final List<S> srvObjects = new ArrayList<S>();
-
         public ServiceObjectsImpl(final ServiceReference<S> ref)
         {
             this.m_ref = ref;
         }
 
         public S getService() {
-        	S srvObj = null;
-            // special handling for prototype scope
-            if ( m_ref.getProperty(Constants.SERVICE_SCOPE) == Constants.SCOPE_PROTOTYPE )
+            checkValidity();
+
+            // CONCURRENCY NOTE: This is a check-then-act situation,
+            // but we ignore it since the time window is small and
+            // the result is the same as if the calling thread had
+            // won the race condition.
+
+            final Object sm = System.getSecurityManager();
+
+            if (sm != null)
             {
-                checkValidity();
-
-                // CONCURRENCY NOTE: This is a check-then-act situation,
-                // but we ignore it since the time window is small and
-                // the result is the same as if the calling thread had
-                // won the race condition.
-
-                Object sm = System.getSecurityManager();
-
-                if (sm != null)
-                {
-                   ((SecurityManager) sm).checkPermission(new ServicePermission(m_ref, ServicePermission.GET));
-                }
-
-                srvObj = m_felix.getService(m_bundle, m_ref, true);
-            }
-            else
-            {
-            	// getService handles singleton and bundle scope
-            	srvObj = BundleContextImpl.this.getService(m_ref);
+               ((SecurityManager) sm).checkPermission(new ServicePermission(m_ref, ServicePermission.GET));
             }
 
-            if ( srvObj != null )
-            {
-            	synchronized ( srvObjects )
-            	{
-            		srvObjects.add(srvObj);
-            	}
-            }
-
-            return srvObj;
+            return m_felix.getService(m_bundle, m_ref, true);
         }
 
         public void ungetService(final S srvObj)
         {
-        	if ( srvObj != null )
-        	{
-                // check if this object was returned by this service objects
-                synchronized ( srvObjects )
-                {
-	                boolean found = false;
-	                int i = 0;
-	                while ( !found && i < srvObjects.size() )
-	                {
-	                	found = srvObjects.get(i) == srvObj;
-	                	i++;
-	                }
-	                if ( !found )
-	                {
-	                	throw new IllegalArgumentException();
-	                }
-	                srvObjects.remove(i-1);
-                }
+            checkValidity();
 
-        	}
-            // special handling for prototype scope
-            if ( m_ref.getProperty(Constants.SERVICE_SCOPE) == Constants.SCOPE_PROTOTYPE )
+            // Unget the specified service.
+            if ( !m_felix.ungetService(m_bundle, m_ref, srvObj) )
             {
-                checkValidity();
-
-                // Unget the specified service.
-                if ( !m_felix.ungetService(m_bundle, m_ref, srvObj) )
-                {
-                	throw new IllegalArgumentException();
-                }
-            }
-            else
-            {
-                // ungetService handles singleton and bundle scope
-                BundleContextImpl.this.ungetService(m_ref);
+            	throw new IllegalArgumentException();
             }
         }
 
diff --git a/framework/src/main/java/org/apache/felix/framework/Felix.java b/framework/src/main/java/org/apache/felix/framework/Felix.java
index 2b418f3..c6b305a 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -3664,11 +3664,11 @@
 
     }
 
-    <S> S getService(Bundle bundle, ServiceReference<S> ref, boolean isPrototype)
+    <S> S getService(Bundle bundle, ServiceReference<S> ref, boolean isServiceObjetcs)
     {
         try
         {
-            return m_registry.getService(bundle, ref, isPrototype);
+            return m_registry.getService(bundle, ref, isServiceObjetcs);
         }
         catch (ServiceException ex)
         {
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 7bd01d2..dec5a0c 100644
--- a/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
+++ b/framework/src/main/java/org/apache/felix/framework/ServiceRegistry.java
@@ -281,13 +281,15 @@
     }
 
     @SuppressWarnings("unchecked")
-    public <S> S getService(Bundle bundle, ServiceReference<S> ref, boolean isPrototype)
+    public <S> S getService(final Bundle bundle, final ServiceReference<S> ref, final boolean isServiceObjects)
     {
+    	// prototype scope is only possible if called from ServiceObjects
+    	final boolean isPrototype = isServiceObjects && ref.getProperty(Constants.SERVICE_SCOPE) == Constants.SCOPE_PROTOTYPE;
         UsageCount usage = null;
         Object svcObj = null;
 
         // Get the service registration.
-        ServiceRegistrationImpl reg =
+        final ServiceRegistrationImpl reg =
             ((ServiceRegistrationImpl.ServiceReferenceImpl) ref).getRegistration();
 
         synchronized (this)
@@ -312,6 +314,7 @@
                 }
                 catch (InterruptedException ex)
                 {
+                	Thread.currentThread().interrupt();
                 }
             }
 
@@ -337,6 +340,10 @@
                 // service object, if one exists.
                 usage.m_count++;
                 svcObj = usage.m_svcObj;
+                if ( isServiceObjects )
+                {
+                	usage.m_serviceObjectsCount++;
+                }
             }
         }
 
@@ -380,7 +387,10 @@
 
     public boolean ungetService(Bundle bundle, ServiceReference<?> ref, Object svcObj)
     {
-        UsageCount usage = null;
+    	// prototype scope is only possible if called from ServiceObjects
+    	final boolean isPrototype = svcObj != null && ref.getProperty(Constants.SERVICE_SCOPE) == Constants.SCOPE_PROTOTYPE;
+
+    	UsageCount usage = null;
         ServiceRegistrationImpl reg =
             ((ServiceRegistrationImpl.ServiceReferenceImpl) ref).getRegistration();
 
@@ -414,7 +424,19 @@
             {
                 return false;
             }
-
+            // if this is a call from service objects and the service was not fetched from
+            // there, return false
+            if ( svcObj != null )
+            {
+            	if ( usage.m_serviceObjectsCount > 0 )
+            	{
+            		usage.m_serviceObjectsCount--;
+            	}
+            	else
+            	{
+            		return false;
+            	}
+            }
             // Lock the service registration.
             m_lockedRegsMap.put(reg, Thread.currentThread());
         }
@@ -601,8 +623,7 @@
         for (int i = 0; (usages != null) && (i < usages.length); i++)
         {
             if (usages[i].m_ref.equals(ref) 
-               && ((svcObj == null && !usages[i].m_prototype) 
-            		|| (usages[i].m_svcObj == svcObj && usages[i].m_prototype)))
+               && ((svcObj == null && !usages[i].m_prototype) || usages[i].m_svcObj == svcObj))
             {
                 return usages[i];
             }
@@ -838,6 +859,7 @@
         public ServiceReference<?> m_ref;
         public Object m_svcObj;
         public boolean m_prototype;
+        public int m_serviceObjectsCount;
     }
 
     public interface ServiceRegistryCallbacks