FELIX-3721 Make sure ManagedService[Factory] services are always called back on initial service registration

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1399530 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
index 2de393a..77007a4 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
@@ -36,6 +36,7 @@
 import org.apache.felix.cm.PersistenceManager;
 import org.apache.felix.cm.file.FilePersistenceManager;
 import org.apache.felix.cm.impl.helper.BaseTracker;
+import org.apache.felix.cm.impl.helper.ConfigurationMap;
 import org.apache.felix.cm.impl.helper.ManagedServiceFactoryTracker;
 import org.apache.felix.cm.impl.helper.ManagedServiceTracker;
 import org.apache.felix.cm.impl.helper.TargetedPID;
@@ -903,7 +904,7 @@
      *      be a <code>ManagedServiceFactory</code>. Otherwise the service
      *      is considered to be a <code>ManagedService</code>.
      */
-    public void configure( String[] pid, ServiceReference sr, final boolean factory )
+    public void configure( String[] pid, ServiceReference sr, final boolean factory, final ConfigurationMap<?> configs )
     {
         if ( this.isLogEnabled( LogService.LOG_DEBUG ) )
         {
@@ -914,11 +915,11 @@
         Runnable r;
         if ( factory )
         {
-            r = new ManagedServiceFactoryUpdate( pid, sr );
+            r = new ManagedServiceFactoryUpdate( pid, sr, configs );
         }
         else
         {
-            r = new ManagedServiceUpdate( pid, sr );
+            r = new ManagedServiceUpdate( pid, sr, configs );
         }
         updateThread.schedule( r );
         log( LogService.LOG_DEBUG, "[{0}] scheduled", new Object[]
@@ -1370,10 +1371,14 @@
 
         private final ServiceReference sr;
 
-        ManagedServiceUpdate( String[] pids, ServiceReference sr )
+        private final ConfigurationMap<?> configs;
+
+
+        ManagedServiceUpdate( String[] pids, ServiceReference sr, ConfigurationMap<?> configs )
         {
             this.pids = pids;
             this.sr = sr;
+            this.configs = configs;
         }
 
 
@@ -1428,7 +1433,7 @@
             log( LogService.LOG_DEBUG, "Updating service {0} with configuration {1}@{2}", new Object[]
                 { servicePid, configPid, new Long( revision ) } );
 
-            managedServiceTracker.provideConfiguration( sr, configPid, null, properties, revision);
+            managedServiceTracker.provideConfiguration( sr, configPid, null, properties, revision, this.configs );
         }
 
         public String toString()
@@ -1450,11 +1455,14 @@
 
         private final ServiceReference sr;
 
+        private final ConfigurationMap<?> configs;
 
-        ManagedServiceFactoryUpdate( String[] factoryPids, ServiceReference sr )
+
+        ManagedServiceFactoryUpdate( String[] factoryPids, ServiceReference sr, final ConfigurationMap<?> configs )
         {
             this.factoryPids = factoryPids;
             this.sr = sr;
+            this.configs = configs;
         }
 
 
@@ -1583,7 +1591,7 @@
                 log( LogService.LOG_DEBUG, "{0}: Updating configuration pid={1}", new Object[]
                     { ConfigurationManager.toString( sr ), config.getPid() } );
                 managedServiceFactoryTracker.provideConfiguration( sr, config.getPid(), config.getFactoryPid(),
-                    rawProperties, revision );
+                    rawProperties, revision, this.configs );
             }
         }
 
@@ -1647,7 +1655,7 @@
                             revision = rc.getRevision();
                         }
 
-                        helper.provideConfiguration( sr, configPid, null, properties, -revision );
+                        helper.provideConfiguration( sr, configPid, null, properties, -revision, null );
 
                         return true;
                     }
@@ -1713,7 +1721,7 @@
                     else if ( canReceive( refBundle, configBundleLocation ) )
                     {
                         helper.provideConfiguration( ref, this.config.getPid(), this.config.getFactoryPid(),
-                            this.properties, this.revision );
+                            this.properties, this.revision, null );
                     }
                     else
                     {
@@ -1879,7 +1887,7 @@
                     {
                         // call updated method
                         helper.provideConfiguration( sr, this.config.getPid(), this.config.getFactoryPid(),
-                            this.properties, this.revision );
+                            this.properties, this.revision, null );
                         log( LogService.LOG_DEBUG, "Configuration {0} provided to {1} (new visibility)", new Object[]
                             { config.getPid(), ConfigurationManager.toString( sr ) } );
                     }
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java
index 471ee45..290c08d 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/BaseTracker.java
@@ -67,8 +67,9 @@
             { ConfigurationManager.toString( reference ) } );
 
         final String[] pids = getServicePid( reference );
-        configure( reference, pids );
-        return createConfigurationMap( pids );
+        final ConfigurationMap<?> configurations = createConfigurationMap( pids );
+        configure( reference, pids, configurations );
+        return configurations;
     }
 
 
@@ -81,7 +82,7 @@
         String[] pids = getServicePid( reference );
         if ( service.isDifferentPids( pids ) )
         {
-            configure( reference, pids );
+            configure( reference, pids, service );
             service.setConfiguredPids( pids );
         }
     }
@@ -96,11 +97,11 @@
     }
 
 
-    private void configure( ServiceReference<S> reference, String[] pids )
+    private void configure( ServiceReference<S> reference, String[] pids, ConfigurationMap<?> configurations )
     {
         if ( pids != null )
         {
-            this.cm.configure( pids, reference, managedServiceFactory );
+            this.cm.configure( pids, reference, managedServiceFactory, configurations );
         }
     }
 
@@ -165,17 +166,33 @@
      * @param factoryPid The targeted factory PID or <code>null</code> for
      *      a non-factory configuration
      * @param properties The configuration properties, which may be
-     *      <code>null</code> for a non-factory configuration
-     * @param revision The configuration revision
+     *      <code>null</code> if this is the provisioning call upon
+     *      service registration of a ManagedService
+     * @param revision The configuration revision or -1 if there is no
+     *      configuration actually to provide.
+     * @param configurationMap The PID to configuration map for PIDs
+     *      used by the service to update
      *
      * @see {@link ManagedServiceTracker#provideConfiguration(ServiceReference, TargetedPID, TargetedPID, Dictionary, long)}
      * @see {@link ManagedServiceFactoryTracker#provideConfiguration(ServiceReference, TargetedPID, TargetedPID, Dictionary, long)}
      */
     public abstract void provideConfiguration( ServiceReference<S> service, TargetedPID configPid,
-        TargetedPID factoryPid, Dictionary<String, ?> properties, long revision );
+        TargetedPID factoryPid, Dictionary<String, ?> properties, long revision,
+        ConfigurationMap<?> configurationMap);
 
 
-    public abstract void removeConfiguration( ServiceReference<S> service, TargetedPID configPid, TargetedPID factoryPid );
+    /**
+     * Remove the configuration indicated by the {@code configPid} from
+     * the service.
+     *
+     * @param service The reference to the service from which the
+     *      configuration is to be removed.
+     * @param configPid The {@link TargetedPID} of the configuration
+     * @param factoryPid The {@link TargetedPID factory PID} of the
+     *      configuration. This may be {@code null} for a non-factory
+     *      configuration.
+     */
+    public abstract void removeConfiguration( ServiceReference<S> service, TargetedPID configPid, TargetedPID factoryPid);
 
 
     protected final S getRealService( ServiceReference<S> reference )
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java
index e782773..998a3fb 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ConfigurationMap.java
@@ -26,7 +26,7 @@
 import java.util.Set;
 
 
-abstract class ConfigurationMap<T>
+public abstract class ConfigurationMap<T>
 {
     private Map<String, T> configurations;
 
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java
index f57f3c4..91ead4e 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceFactoryTracker.java
@@ -54,29 +54,47 @@
 
     @Override
     public void provideConfiguration( ServiceReference<ManagedServiceFactory> reference, TargetedPID configPid,
-        TargetedPID factoryPid, Dictionary<String, ?> properties, long revision )
+        TargetedPID factoryPid, Dictionary<String, ?> properties, long revision, ConfigurationMap<?> configs )
     {
+        // Get the ManagedServiceFactory and terminate here if already
+        // unregistered from the framework concurrently
         ManagedServiceFactory service = getRealService( reference );
-        final ConfigurationMap configs = this.getService( reference );
-        if ( service != null && configs != null )
+        if (service == null) {
+            return;
+        }
+
+        // Get the Configuration-to-PID map from the parameter or from
+        // the service tracker. If not available, the service tracker
+        // already unregistered this service concurrently
+        if ( configs == null )
         {
-            if ( configs.shallTake( configPid, factoryPid, revision ) )
+            configs =  this.getService( reference );
+            if ( configs == null )
             {
-                try
-                {
-                    Dictionary props = getProperties( properties, reference, configPid.toString(),
-                        factoryPid.toString() );
-                    service.updated( configPid.toString(), props );
-                    configs.record( configPid, factoryPid, revision );
-                }
-                catch ( Throwable t )
-                {
-                    this.handleCallBackError( t, reference, configPid );
-                }
-                finally
-                {
-                    this.ungetRealService( reference );
-                }
+                return;
+            }
+        }
+
+        // Both the ManagedService to update and the Configuration-to-PID
+        // are available, so the service can be updated with the
+        // configuration (which may be null)
+
+        if ( configs.shallTake( configPid, factoryPid, revision ) )
+        {
+            try
+            {
+                Dictionary props = getProperties( properties, reference, configPid.toString(),
+                    factoryPid.toString() );
+                service.updated( configPid.toString(), props );
+                configs.record( configPid, factoryPid, revision );
+            }
+            catch ( Throwable t )
+            {
+                this.handleCallBackError( t, reference, configPid );
+            }
+            finally
+            {
+                this.ungetRealService( reference );
             }
         }
     }
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java
index b0d8c68..0d62181 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/helper/ManagedServiceTracker.java
@@ -75,10 +75,10 @@
      */
     @Override
     public void provideConfiguration( ServiceReference<ManagedService> service, TargetedPID configPid,
-        TargetedPID factoryPid, Dictionary<String, ?> properties, long revision )
+        TargetedPID factoryPid, Dictionary<String, ?> properties, long revision, ConfigurationMap<?> configs )
     {
         Dictionary<String, ?> supplied = ( properties == null ) ? INITIAL_MARKER : properties;
-        updateService( service, configPid, supplied, revision );
+        updateService( service, configPid, supplied, revision, configs );
     }
 
 
@@ -86,57 +86,75 @@
     public void removeConfiguration( ServiceReference<ManagedService> service, TargetedPID configPid,
         TargetedPID factoryPid )
     {
-        updateService( service, configPid, null, -1 );
+        updateService( service, configPid, null, -1, null );
     }
 
 
     private void updateService( ServiceReference<ManagedService> service, final TargetedPID configPid,
-        Dictionary<String, ?> properties, long revision )
+        Dictionary<String, ?> properties, long revision, ConfigurationMap<?> configs)
     {
+        // Get the ManagedService and terminate here if already
+        // unregistered from the framework concurrently
         final ManagedService srv = this.getRealService( service );
-        final ConfigurationMap configs = this.getService( service );
-        if ( srv != null && configs != null )
-        {
-            boolean doUpdate = false;
-            if ( properties == null )
-            {
-                doUpdate = configs.removeConfiguration( configPid, null );
-            }
-            else if ( properties == INITIAL_MARKER )
-            {
-                // initial call to ManagedService may supply null properties
-                properties = null;
-                revision = -1;
-                doUpdate = true;
-            }
-            else if ( revision < 0 || configs.shallTake( configPid, null, revision ) )
-            {
-                // run the plugins and cause the update
-                properties = getProperties( properties, service, configPid.toString(), null );
-                doUpdate = true;
-                revision = Math.abs( revision );
-            }
-            else
-            {
-                // new configuration is not a better match, don't update
-                doUpdate = false;
-            }
+        if (srv == null) {
+            return;
+        }
 
-            if ( doUpdate )
+        // Get the Configuration-to-PID map from the parameter or from
+        // the service tracker. If not available, the service tracker
+        // already unregistered this service concurrently
+        if ( configs == null )
+        {
+            configs = this.getService( service );
+            if ( configs == null )
             {
-                try
-                {
-                    srv.updated( properties );
-                    configs.record( configPid, null, revision );
-                }
-                catch ( Throwable t )
-                {
-                    this.handleCallBackError( t, service, configPid );
-                }
-                finally
-                {
-                    this.ungetRealService( service );
-                }
+                return;
+            }
+        }
+
+        // Both the ManagedService to update and the Configuration-to-PID
+        // are available, so the service can be updated with the
+        // configuration (which may be null)
+
+        boolean doUpdate = false;
+        if ( properties == null )
+        {
+            doUpdate = configs.removeConfiguration( configPid, null );
+        }
+        else if ( properties == INITIAL_MARKER )
+        {
+            // initial call to ManagedService may supply null properties
+            properties = null;
+            revision = -1;
+            doUpdate = true;
+        }
+        else if ( revision < 0 || configs.shallTake( configPid, null, revision ) )
+        {
+            // run the plugins and cause the update
+            properties = getProperties( properties, service, configPid.toString(), null );
+            doUpdate = true;
+            revision = Math.abs( revision );
+        }
+        else
+        {
+            // new configuration is not a better match, don't update
+            doUpdate = false;
+        }
+
+        if ( doUpdate )
+        {
+            try
+            {
+                srv.updated( properties );
+                configs.record( configPid, null, revision );
+            }
+            catch ( Throwable t )
+            {
+                this.handleCallBackError( t, service, configPid );
+            }
+            finally
+            {
+                this.ungetRealService( service );
             }
         }
    }