FELIX-5028 : ServiceFactory for components might return null

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1703178 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/changelog.txt b/scr/changelog.txt
index d151138..b61287e 100644
--- a/scr/changelog.txt
+++ b/scr/changelog.txt
@@ -4,6 +4,9 @@
     * [FELIX-5001] scr:list Gogo command should display component configurations
     * [FELIX-5020] Don't log exception if metatype is not available
 
+** Bug
+    * [FELIX-5028] ServiceFactory for components might return null
+
 
 Changes from 1.8.2 to 2.0.0
 ---------------------------
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
index fbf2356..7c4b77f 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
@@ -188,7 +188,7 @@
         return ScrConfiguration.DEFAULT_LOCK_TIMEOUT_MILLISECONDS;
     }
 
-    private void obtainLock( Lock lock, String source )
+    private void obtainLock( Lock lock )
     {
         try
         {
@@ -218,22 +218,22 @@
         }
     }
 
-    final void obtainActivationReadLock( String source )
+    final void obtainActivationReadLock(  )
     {
-        obtainLock( m_activationLock.readLock(), source);
+        obtainLock( m_activationLock.readLock());
     }
 
-    final void releaseActivationReadLock( String source )
+    final void releaseActivationReadLock( )
     {
         m_activationLock.readLock().unlock();
     }
 
-    final void obtainActivationWriteLock( String source )
+    final void obtainActivationWriteLock( )
     {
-        obtainLock( m_activationLock.writeLock(), source);
+        obtainLock( m_activationLock.writeLock());
     }
 
-    final void releaseActivationWriteeLock( String source )
+    final void releaseActivationWriteeLock( )
     {
         if ( m_activationLock.getWriteHoldCount() > 0 )
         {
@@ -241,12 +241,12 @@
         }
     }
 
-    final void obtainStateLock( String source )
+    final void obtainStateLock()
     {
-        obtainLock( m_stateLock, source );
+        obtainLock( m_stateLock );
     }
 
-    final void releaseStateLock( String source )
+    final void releaseStateLock()
     {
         m_stateLock.unlock();
     }
@@ -683,7 +683,7 @@
             return;
         }
 
-        obtainActivationReadLock( "activateInternal" );
+        obtainActivationReadLock(  );
         try
         {
             // Double check conditions now that we have obtained the lock
@@ -726,7 +726,7 @@
         }
         finally
         {
-            releaseActivationReadLock( "activateInternal" );
+            releaseActivationReadLock(  );
         }
     }
 
@@ -751,14 +751,14 @@
 
         // catch any problems from deleting the component to prevent the
         // component to remain in the deactivating state !
-        obtainActivationReadLock( "deactivateInternal" );
+        obtainActivationReadLock(  );
         try
         {
             doDeactivate( reason, disable || m_factoryInstance );
         }
         finally
         {
-            releaseActivationReadLock( "deactivateInternal" );
+            releaseActivationReadLock(  );
         }
         if ( isFactory() || m_factoryInstance || dispose )
         {
@@ -775,7 +775,7 @@
             {
                 log( LogService.LOG_DEBUG, "Component deactivation occuring on another thread", null );
             }
-            obtainStateLock( "AbstractComponentManager.State.doDeactivate.1" );
+            obtainStateLock(  );
             try
             {
             	m_satisfied = false;
@@ -789,7 +789,7 @@
             }
             finally
             {
-                releaseStateLock( "AbstractComponentManager.State.doDeactivate.1" );
+                releaseStateLock( );
             }
         }
         catch ( Throwable t )
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
index ae2b33c..cebbc38 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
@@ -20,8 +20,6 @@
 
 
 import java.util.Dictionary;
-import java.util.HashMap;
-import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
@@ -31,7 +29,6 @@
 import org.apache.felix.scr.impl.helper.ReadOnlyDictionary;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
-import org.osgi.framework.ServiceObjects;
 import org.osgi.framework.ServiceReference;
 import org.osgi.service.component.ComponentInstance;
 import org.osgi.service.log.LogService;
@@ -116,7 +113,7 @@
 
     public Object locateService( String name )
     {
-        m_componentManager.obtainActivationReadLock( "locate.service.name" );
+        m_componentManager.obtainActivationReadLock( );
         try
         {
             DependencyManager<S, ?> dm = m_componentManager.getDependencyManager( name );
@@ -124,14 +121,14 @@
         }
         finally
         {
-            m_componentManager.releaseActivationReadLock( "locate.service.name" );
+            m_componentManager.releaseActivationReadLock(  );
         }
     }
 
 
     public Object locateService( String name, ServiceReference ref )
     {
-        m_componentManager.obtainActivationReadLock( "locate.service.ref" );
+        m_componentManager.obtainActivationReadLock(  );
         try
         {
             DependencyManager<S, ?> dm = m_componentManager.getDependencyManager( name );
@@ -139,14 +136,14 @@
         }
         finally
         {
-            m_componentManager.releaseActivationReadLock( "locate.service.ref" );
+            m_componentManager.releaseActivationReadLock( );
         }
     }
 
 
     public Object[] locateServices( String name )
     {
-        m_componentManager.obtainActivationReadLock( "locate.services" );
+        m_componentManager.obtainActivationReadLock(  );
         try
         {
             DependencyManager<S, ?> dm = m_componentManager.getDependencyManager( name );
@@ -154,7 +151,7 @@
         }
         finally
         {
-            m_componentManager.releaseActivationReadLock( "locate.services" );
+            m_componentManager.releaseActivationReadLock( );
         }
     }
 
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java
index 63038e8..dd158a6 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java
@@ -587,14 +587,14 @@
 
             // unsatisfied component and non-ignored configuration may change targets
             // to satisfy references
-            obtainActivationWriteLock( "reconfigure" );
+            obtainActivationWriteLock( );
             try
             {
                 if ( !isSatisfied() && !getComponentMetadata().isConfigurationIgnored() )
                 {
                     log( LogService.LOG_DEBUG, "Attempting to activate unsatisfied component", null );
                     updateTargets( getProperties() );
-                    releaseActivationWriteeLock( "reconfigure.unsatisfied" );
+                    releaseActivationWriteeLock(  );
                     activateInternal( );
                     return;
                 }
@@ -609,20 +609,20 @@
                     // FELIX-2368: cycle component immediately, reconfigure() is
                     //     called through ConfigurationListener API which itself is
                     //     called asynchronously by the Configuration Admin Service
-                    releaseActivationWriteeLock( "reconfigure.modified.1" );
+                    releaseActivationWriteeLock(  );
                     //we have already determined that modify cannot be called. Therefore factory instances must be disposed.
                     boolean dispose = m_factoryInstance;
                     deactivateInternal( reason, dispose, dispose );
                     if ( !dispose )
                     {
-                        obtainActivationWriteLock("reconfigure.deactivate.activate");
+                        obtainActivationWriteLock();
                         try
                         {
                             updateTargets(getProperties());
                         }
                         finally
                         {
-                            releaseActivationWriteeLock("reconfigure.deactivate.activate");
+                            releaseActivationWriteeLock();
                         }
                         activateInternal();
                     }
@@ -631,7 +631,7 @@
             finally
             {
                 //used if modify succeeds or if there's an exception.
-                releaseActivationWriteeLock( "reconfigure.end" );;
+                releaseActivationWriteeLock(  );
             }
         }
         finally
@@ -677,7 +677,7 @@
         // 4. call method (nothing to do when failed, since it has already been logged)
         //   (call with non-null default result to continue even if the
         //    modify method call failed)
-        obtainStateLock( "ImmediateComponentManager.modify" );
+        obtainStateLock(  );
         try
         {
             //cf 112.5.12 where invoking modified method before updating target services is specified.
@@ -717,7 +717,7 @@
         }
         finally
         {
-            releaseStateLock( "ImmediateComponentManager.modify" );
+            releaseStateLock(  );
         }
     }
 
@@ -760,16 +760,35 @@
 
     public S getService( Bundle bundle, ServiceRegistration<S> serviceRegistration )
     {
-        boolean success = getServiceInternal();
-        ComponentContextImpl<S> componentContext = m_componentContext;
-        if ( success && componentContext != null)
+        obtainStateLock(  );
+        try
         {
             m_useCount.incrementAndGet();
-            return componentContext.getImplementationObject( true );
         }
-        else
+        finally
         {
-            return null;
+            releaseStateLock( );
+        }
+        boolean decrement = true;
+        try {
+            boolean success = getServiceInternal();
+            ComponentContextImpl<S> componentContext = m_componentContext;
+            if ( success && componentContext != null)
+            {
+                decrement = false;
+                return componentContext.getImplementationObject( true );
+            }
+            else
+            {
+                return null;
+            }
+        }
+        finally
+        {
+            if ( decrement )
+            {
+                ungetService( bundle, serviceRegistration, null );
+            }
         }
     }
 
@@ -805,7 +824,7 @@
                             null );
                     success = false;
                 }
-                obtainStateLock( "ImmediateComponentManager.getService.1" );
+                obtainStateLock(  );
                 try
                 {
                     if ( m_componentContext == null )
@@ -824,7 +843,7 @@
                 }
                 finally
                 {
-                    releaseStateLock( "ImmediateComponentManager.getService.1" );
+                    releaseStateLock(  );
                 }
             }
             return success;
@@ -872,31 +891,21 @@
 
     public void ungetService( Bundle bundle, ServiceRegistration<S> serviceRegistration, S o )
     {
-        // the framework should not call ungetService more than it calls
-        // calls getService. Still, we want to be sure to not go below zero
-        if ( m_useCount.get() > 0 )
+        obtainStateLock( );
+        try
         {
-            int useCount = m_useCount.decrementAndGet();
-
             // unget the service instance if no bundle is using it
             // any longer unless delayed component instances have to
             // be kept (FELIX-3039)
-            if ( useCount == 0 && !isImmediate() && !keepInstances() )
+            if (  m_useCount.decrementAndGet() == 0 && !isImmediate() && !keepInstances() )
             {
-                obtainStateLock( "ImmediateComponentManager.ungetService.1" );
-                try
-                {
-                    if ( m_useCount.get() == 0 )
-                    {
-                        ungetService( );
-                    }
-                }
-                finally
-                {
-                    releaseStateLock( "ImmediateComponentManager.ungetService.1" );
-                }
+                ungetService( );
             }
         }
+        finally
+        {
+            releaseStateLock(  );
+        }
     }
 
     private void ungetService( )