You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by pd...@apache.org on 2015/02/01 18:26:36 UTC

svn commit: r1656334 - in /felix/sandbox/pderop/dependencymanager: org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/tests/ org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/

Author: pderop
Date: Sun Feb  1 17:26:36 2015
New Revision: 1656334

URL: http://svn.apache.org/r1656334
Log:
Fixed findbug warnings in DependencyManagerRuntime, and in FactorySet.
Did some minor cleanup.
Always require the PackageAdmin class, which is needed to handle annoted fragment bundles.

Modified:
    felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/tests/ServiceFactoryAnnotationTest.java
    felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/Activator.java
    felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/DependencyManagerRuntime.java
    felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/FactorySet.java

Modified: felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/tests/ServiceFactoryAnnotationTest.java
URL: http://svn.apache.org/viewvc/felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/tests/ServiceFactoryAnnotationTest.java?rev=1656334&r1=1656333&r2=1656334&view=diff
==============================================================================
--- felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/tests/ServiceFactoryAnnotationTest.java (original)
+++ felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/tests/ServiceFactoryAnnotationTest.java Sun Feb  1 17:26:36 2015
@@ -68,5 +68,6 @@ public class ServiceFactoryAnnotationTes
 
         // remove instance
         Assert.assertTrue(factory.remove(conf));
+        Assert.assertFalse(factory.remove(conf));
     }
 }

Modified: felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/Activator.java
URL: http://svn.apache.org/viewvc/felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/Activator.java?rev=1656334&r1=1656333&r2=1656334&view=diff
==============================================================================
--- felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/Activator.java (original)
+++ felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/Activator.java Sun Feb  1 17:26:36 2015
@@ -18,13 +18,9 @@
  */
 package org.apache.felix.dm.runtime;
 
-import java.util.HashMap;
-import java.util.Map;
-
 import org.apache.felix.dm.Component;
 import org.apache.felix.dm.DependencyActivatorBase;
 import org.apache.felix.dm.DependencyManager;
-import org.apache.felix.dm.ServiceDependency;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.service.log.LogService;
@@ -45,13 +41,7 @@ public class Activator extends Dependenc
      * (default = false)
      */
     final static String CONF_LOG = "org.apache.felix.dependencymanager.runtime.log";
-    
-    /**
-     * Name of bundle context property telling if PackageAdmin service is required or not.
-     * (default = false)
-     */
-    private final static String CONF_PACKAGE_ADMIN = "org.apache.felix.dependencymanager.runtime.packageAdmin";
-    
+        
     /**
      * Name of bundle context property telling if Components must be auto configured
      * with BundleContext/ServiceRegistration etc .. (default = false) 
@@ -74,31 +64,22 @@ public class Activator extends Dependenc
     @Override
     public void init(BundleContext context, DependencyManager dm) throws Exception
     {
-        Component component = createComponent()
-            .setImplementation( DependencyManagerRuntime.class )
-            .setComposition( "getComposition" )
-            .add(createBundleDependency()
-                .setRequired( false )
-                .setAutoConfig( false )
-                .setStateMask( Bundle.ACTIVE )
-                .setFilter( "(DependencyManager-Component=*)" )
-                .setCallbacks( "bundleStarted", "bundleStopped" ) );
-                
-        Map<String, Class<?>> services = new HashMap<String, Class<?>>( 2 );
-        services.put( CONF_LOG, LogService.class );
-        services.put( CONF_PACKAGE_ADMIN, PackageAdmin.class );
-        for (Map.Entry<String, Class<?>> entry : services.entrySet())
-        {
-            String serviceProperty = entry.getKey();
-            Class<?> service = entry.getValue();
-            boolean serviceActive = "true".equalsIgnoreCase(context.getProperty(serviceProperty));
-            ServiceDependency serviceDep =
-                    serviceActive ? createTemporalServiceDependency(30000L)
-                                 : createServiceDependency().setRequired(false);
-            serviceDep.setService(service).setAutoConfig(true);
-            component.add(serviceDep);
-        }
-        dm.add( component );
+		Component component = createComponent()
+				.setImplementation(DependencyManagerRuntime.class)
+				.setComposition("getComposition")
+				.add(createBundleDependency()
+						.setRequired(false)
+						.setStateMask(Bundle.ACTIVE)
+						.setFilter("(DependencyManager-Component=*)")
+						.setCallbacks("bundleStarted", "bundleStopped"))
+				.add(createServiceDependency()
+						.setRequired(true)
+						.setService(PackageAdmin.class))
+				.add(createServiceDependency()
+						.setRequired("true".equalsIgnoreCase(context.getProperty(CONF_LOG)))
+						.setService(LogService.class));
+				                
+        dm.add(component);
     }
 
     /**

Modified: felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/DependencyManagerRuntime.java
URL: http://svn.apache.org/viewvc/felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/DependencyManagerRuntime.java?rev=1656334&r1=1656333&r2=1656334&view=diff
==============================================================================
--- felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/DependencyManagerRuntime.java (original)
+++ felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/DependencyManagerRuntime.java Sun Feb  1 17:26:36 2015
@@ -23,8 +23,9 @@ import java.io.IOException;
 import java.io.InputStreamReader;
 import java.net.URL;
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.Map;
 
 import org.apache.felix.dm.Component;
 import org.apache.felix.dm.DependencyManager;
@@ -40,11 +41,23 @@ import org.osgi.service.packageadmin.Pac
  */
 public class DependencyManagerRuntime
 {
-    private ConcurrentHashMap<Bundle, DependencyManager> m_managers =
-            new ConcurrentHashMap<Bundle, DependencyManager>();
-    private DescriptorParser m_parser;
-    private PackageAdmin m_packageAdmin; // Possibly a NullObject
-
+    /**
+     * Map between bundles and their corresponding DependencyManager objects used to create bundle's components.
+     * Notice that we can safely use this map without synchronization because we are relying on the new DM4 thread
+     * model which serialize all component events safely.
+     */
+    private final Map<Bundle, DependencyManager> m_managers = new HashMap<Bundle, DependencyManager>();
+    
+    /**
+     * Parser used to scan component descriptors defined in bundles meta data.
+     */
+    private final DescriptorParser m_parser;
+    
+    /**
+     * We use the PackageAdmin service to allow support for annotations in fragment bundles.
+     */
+    private volatile PackageAdmin m_packageAdmin;
+    
     /**
      * Our constructor. We'll initialize here our DM component builders.
      */

Modified: felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/FactorySet.java
URL: http://svn.apache.org/viewvc/felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/FactorySet.java?rev=1656334&r1=1656333&r2=1656334&view=diff
==============================================================================
--- felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/FactorySet.java (original)
+++ felix/sandbox/pderop/dependencymanager/org.apache.felix.dependencymanager.runtime/src/org/apache/felix/dm/runtime/FactorySet.java Sun Feb  1 17:26:36 2015
@@ -52,12 +52,10 @@ public class FactorySet extends Abstract
      */
     private final static String DM_FACTORY_INSTANCE = "dm.factory.instance";
     
-    public final static String DM_FACTORY_NAME = "dm.factory.name";
-
     /**
      * The actual Service instance that is allocated for each dictionaries added in this Set.
      */
-    private Object m_impl;
+    private volatile Object m_impl;
 
     /**
      * The Service provided in the OSGi registry.
@@ -75,31 +73,32 @@ public class FactorySet extends Abstract
     private final String m_configure;
 
     /**
-     * The map between our Dictionaries and corresponding Service instances.
+     * The map between our Dictionaries and corresponding Service instances. 
+     * This map is modified from our serial executor, or from the add method.
      */
     private final ConcurrentHashMap<ServiceKey, Object> m_services = new ConcurrentHashMap<ServiceKey, Object>();
 
     /**
      * The list of Dependencies which are applied in the Service.
      */
-    private MetaData m_srvMeta;
+    private final MetaData m_srvMeta;
 
     /**
      * The list of Dependencies which are applied in the Service.
      */
-    private List<MetaData> m_depsMeta;
+    private final List<MetaData> m_depsMeta;
 
     /**
      * The DependencyManager which is used to create Service instances.
      */
-    private DependencyManager m_dm;
+    private volatile DependencyManager m_dm;
 
     /**
      * This class is used to serialize concurrent method calls, and allow to leave our methods unsynchronized.
      * This is required because some of our methods may invoke some Service callbacks, which must be called outside 
      * synchronized block (in order to avoid potential dead locks).
      */
-    private SerialExecutor m_serialExecutor = new SerialExecutor();
+    private final SerialExecutor m_serialExecutor = new SerialExecutor();
 
     /**
      * Flag used to check if our service is Active.
@@ -109,7 +108,7 @@ public class FactorySet extends Abstract
     /**
      * The bundle containing the Service annotated with the factory attribute.
      */
-    private Bundle m_bundle;
+    private final Bundle m_bundle;
 
     /**
      * Flag used to check if a service is being created
@@ -214,23 +213,16 @@ public class FactorySet extends Abstract
 
         // Check if service is being created
         ServiceKey serviceKey = new ServiceKey(configuration);
-        boolean creating = false;
-        synchronized (this)
+        boolean creating = m_services.putIfAbsent(serviceKey, SERVICE_CREATING) == null;
+        
+        // Create or Update the Service.
+        m_serialExecutor.enqueue(new Runnable()
         {
-            if (!m_services.containsKey(serviceKey))
+            public void run()
             {
-                m_services.put(serviceKey, SERVICE_CREATING);
-                creating = true;
+                doAdd(configuration);
             }
-            // Create or Update the Service.
-            m_serialExecutor.enqueue(new Runnable()
-            {
-                public void run()
-                {
-                    doAdd(configuration);
-                }
-            });
-        }
+        });
 
         m_serialExecutor.execute();
         return creating;
@@ -289,7 +281,7 @@ public class FactorySet extends Abstract
             return;
         }
 
-        // Create or Update the Service.
+        // Make sure add/update/clear events are handled in FIFO order (serially).
         m_serialExecutor.enqueue(new Runnable()
         {
             public void run()
@@ -350,6 +342,7 @@ public class FactorySet extends Abstract
         // Check if the service exists.
         ServiceKey serviceKey = new ServiceKey(configuration);
         Object service = m_services.get(serviceKey);
+
         if (service == null || service == SERVICE_CREATING)
         {
             try
@@ -457,20 +450,14 @@ public class FactorySet extends Abstract
 
     private void doClear()
     {
-        try
+        for (Object service : m_services.values())
         {
-            for (Object service: m_services.values())
+            if (service instanceof Component)
             {
-                if (service instanceof Component)
-                {
-                    m_dm.remove((Component) service);
-                }
+                m_dm.remove((Component) service);
             }
         }
-        finally
-        {
-            m_services.clear();
-        }
+        m_services.clear();
     }
 
     /**