You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by dj...@apache.org on 2013/11/20 09:31:36 UTC

svn commit: r1543735 - in /felix/trunk/scr/src: main/java/org/apache/felix/scr/impl/ main/java/org/apache/felix/scr/impl/config/ test/java/org/apache/felix/scr/impl/config/

Author: djencks
Date: Wed Nov 20 08:31:36 2013
New Revision: 1543735

URL: http://svn.apache.org/r1543735
Log:
FELIX-4323 fix CCH.getComponents to return complete list of components, including possible single component.

Modified:
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java
    felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java?rev=1543735&r1=1543734&r2=1543735&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java Wed Nov 20 08:31:36 2013
@@ -29,6 +29,9 @@ import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.List;
 import java.util.StringTokenizer;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.felix.scr.impl.config.ComponentHolder;
 import org.apache.felix.scr.impl.config.ScrConfiguration;
@@ -69,7 +72,8 @@ public class BundleComponentActivator im
     private final ComponentActorThread m_componentActor;
 
     // true as long as the dispose method is not called
-    private boolean m_active;
+    private final AtomicBoolean m_active = new AtomicBoolean(true);
+    private final CountDownLatch m_closeLatch = new CountDownLatch(1);
 
     // the configuration
     private final ScrConfiguration m_configuration;
@@ -94,9 +98,6 @@ public class BundleComponentActivator im
         m_componentActor = componentActor;
         m_context = context;
 
-        // mark this instance active
-        m_active = true;
-
         // have the LogService handy (if available)
         m_logService = new ServiceTracker( context, Activator.LOGSERVICE_CLASS, null );
         m_logService.open();
@@ -316,44 +317,50 @@ public class BundleComponentActivator im
     */
     void dispose( int reason )
     {
-        synchronized ( this )
+        if ( m_active.compareAndSet( true, false ))
         {
-            if ( !m_active )
+            log( LogService.LOG_DEBUG, "BundleComponentActivator : Bundle [{0}] will destroy {1} instances", new Object[]
+                    { m_context.getBundle().getBundleId(), m_managers.size() }, null, null, null );
+
+            while ( m_managers.size() != 0 )
             {
-                return;
+                ComponentHolder holder = m_managers.get( 0 );
+                try
+                {
+                    m_managers.remove( holder );
+                    holder.disposeComponents( reason );
+                }
+                catch ( Exception e )
+                {
+                    log( LogService.LOG_ERROR, "BundleComponentActivator : Exception invalidating", holder
+                            .getComponentMetadata(), null, e );
+                }
+                finally
+                {
+                    m_componentRegistry.unregisterComponentHolder( m_context.getBundle(), holder.getComponentMetadata()
+                            .getName() );
+                }
+
             }
-            // mark instance inactive (no more component activations)
-            m_active = false;
-        }
-        log( LogService.LOG_DEBUG, "BundleComponentActivator : Bundle [{0}] will destroy {1} instances", new Object[]
-            { m_context.getBundle().getBundleId(), m_managers.size() }, null, null, null );
 
-        while ( m_managers.size() != 0 )
+            log( LogService.LOG_DEBUG, "BundleComponentActivator : Bundle [{0}] STOPPED", new Object[]
+                    {m_context.getBundle().getBundleId()}, null, null, null );
+
+            m_logService.close();
+            m_closeLatch.countDown();
+        }
+        else 
         {
-            ComponentHolder holder = m_managers.get( 0 );
             try
             {
-                m_managers.remove( holder );
-                holder.disposeComponents( reason );
+                m_closeLatch.await(m_configuration.lockTimeout(), TimeUnit.MILLISECONDS);
             }
-            catch ( Exception e )
+            catch ( InterruptedException e )
             {
-                log( LogService.LOG_ERROR, "BundleComponentActivator : Exception invalidating", holder
-                    .getComponentMetadata(), null, e );
+                //ignore interruption during concurrent shutdown.
             }
-            finally
-            {
-                m_componentRegistry.unregisterComponentHolder( m_context.getBundle(), holder.getComponentMetadata()
-                    .getName() );
-            }
-
         }
 
-        log( LogService.LOG_DEBUG, "BundleComponentActivator : Bundle [{0}] STOPPED", new Object[]
-            {m_context.getBundle().getBundleId()}, null, null, null );
-
-        m_logService.close();
-
     }
 
 
@@ -366,7 +373,7 @@ public class BundleComponentActivator im
      */
     public boolean isActive()
     {
-        return m_active;
+        return m_active.get();
     }
 
 

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java?rev=1543735&r1=1543734&r2=1543735&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java Wed Nov 20 08:31:36 2013
@@ -343,20 +343,22 @@ public class ConfigurableComponentHolder
         return created;
     }
 
-    public synchronized long getChangeCount( String pid)
+    public long getChangeCount( String pid)
     {
-        
-        SingleComponentManager icm =  pid.equals( getComponentMetadata().getConfigurationPid())? 
-                m_singleComponent: m_components.get( pid );
-        return icm == null? -1: icm.getChangeCount();
+
+        synchronized ( m_components )
+        {
+            SingleComponentManager<S> icm =  pid.equals( getComponentMetadata().getConfigurationPid())? 
+                    m_singleComponent: m_components.get( pid );
+            return icm == null? -1: icm.getChangeCount();
+        }
     }
 
     public Component[] getComponents()
     {
         synchronized ( m_components )
         {
-            Component[] components = getComponentManagers( false );
-            return ( components != null ) ? components : new Component[] { m_singleComponent };
+            return getComponentManagers( false );
         }
     }
 
@@ -368,10 +370,6 @@ public class ConfigurableComponentHolder
         {
             m_enabled = true;
             cms = getComponentManagers( false );
-            if ( cms == null )
-            {
-                cms = new SingleComponentManager[] { m_singleComponent };
-            }
         }
         for ( SingleComponentManager cm : cms )
         {
@@ -388,10 +386,6 @@ public class ConfigurableComponentHolder
             m_enabled = false;
 
             cms = getComponentManagers( false );
-            if ( cms == null )
-            {
-                cms = new SingleComponentManager[] { m_singleComponent };
-            }
         }
         for ( SingleComponentManager cm : cms )
         {
@@ -405,16 +399,7 @@ public class ConfigurableComponentHolder
         SingleComponentManager[] cms;
         synchronized ( m_components )
         {
-            // FELIX-1733: get a copy of the single component and clear
-            // the field to prevent recreation in disposed(ICM)
-            final SingleComponentManager singleComponent = m_singleComponent;
-            m_singleComponent = null;
-
             cms = getComponentManagers( true );
-            if ( cms == null )
-            {
-                cms = new SingleComponentManager[] { singleComponent };
-            }
         }
         for ( SingleComponentManager cm : cms )
         {
@@ -508,27 +493,39 @@ public class ConfigurableComponentHolder
     //---------- internal
 
     /**
-     * Returns all components from the map, optionally also removing them
-     * from the map. If there are no components in the map, <code>null</code>
-     * is returned.
+     * Returns all component managers from the map and the single component manager, optionally also removing them
+     * from the map. If there are no component managers, <code>null</code>
+     * is returned.  Must be called synchronized on m_components.
+     * 
+     * @param clear If true, clear the map and the single component manager.
      */
-    private SingleComponentManager[] getComponentManagers( final boolean clear )
-    {
-        // fast exit if there is no component in the map
-        if ( m_components.isEmpty() )
-        {
-            return null;
-        }
-
-        final SingleComponentManager[] cm = new SingleComponentManager[m_components.size()];
-        m_components.values().toArray( cm );
-
-        if ( clear )
-        {
-            m_components.clear();
-        }
-        return cm;
-    }
+   private SingleComponentManager<S>[] getComponentManagers( final boolean clear )
+   {
+       SingleComponentManager<S>[] cm;
+       if ( m_components.isEmpty() )
+       {
+           if ( m_singleComponent != null)
+           {
+               cm = new SingleComponentManager[] {m_singleComponent};
+           }
+           else 
+           {
+               cm = null;
+           }
+       }
+
+       else
+       {
+           cm = new SingleComponentManager[m_components.size()];
+           m_components.values().toArray( cm );
+       }
+       if ( clear )
+       {
+           m_components.clear();
+           m_singleComponent = null;
+       }
+       return cm;
+   }
 
     public boolean isLogEnabled( int level )
     {

Modified: felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java?rev=1543735&r1=1543734&r2=1543735&view=diff
==============================================================================
--- felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java (original)
+++ felix/trunk/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java Wed Nov 20 08:31:36 2013
@@ -47,7 +47,7 @@ public class ConfiguredComponentHolderTe
         // assert single component and no map
         final SingleComponentManager cmgr = getSingleManager( holder );
         assertNotNull( "Expect single component manager", cmgr );
-        assertNull( "Expect no component manager list", getComponentManagers( holder ) );
+        assertEquals( "Expect no other component manager list", 1, getComponentManagers( holder ).length);
 
         // assert no configuration of single component
         assertFalse( "Expect no configuration", cmgr.hasConfiguration() );
@@ -64,7 +64,7 @@ public class ConfiguredComponentHolderTe
         // assert single component and no map
         final SingleComponentManager cmgr = getSingleManager( holder );
         assertNotNull( "Expect single component manager", cmgr );
-        assertNull( "Expect no component manager list", getComponentManagers( holder ) );
+        assertEquals( "Expect no other component manager list", 1, getComponentManagers( holder ).length);
 
         // assert no configuration of single component
         assertFalse( "Expect no configuration", cmgr.hasConfiguration() );
@@ -77,7 +77,7 @@ public class ConfiguredComponentHolderTe
         // assert single component and no map
         final SingleComponentManager cmgrAfterConfig = getSingleManager( holder );
         assertNotNull( "Expect single component manager", cmgrAfterConfig );
-        assertNull( "Expect no component manager list", getComponentManagers( holder ) );
+        assertEquals( "Expect no other component manager list", 1, getComponentManagers( holder ).length);
 
         // assert configuration of single component
         assertTrue( "Expect configuration after updating it", cmgrAfterConfig.hasConfiguration() );
@@ -90,7 +90,7 @@ public class ConfiguredComponentHolderTe
         // assert single component and no map
         final SingleComponentManager cmgrAfterUnconfig = getSingleManager( holder );
         assertNotNull( "Expect single component manager", cmgrAfterUnconfig );
-        assertNull( "Expect no component manager list", getComponentManagers( holder ) );
+        assertEquals( "Expect no other component manager list", 1, getComponentManagers( holder ).length);
 
         // assert no configuration of single component
         assertFalse( "Expect no configuration", cmgrAfterUnconfig.hasConfiguration() );
@@ -107,7 +107,7 @@ public class ConfiguredComponentHolderTe
         // assert single component and no map
         final SingleComponentManager cmgr = getSingleManager( holder );
         assertNotNull( "Expect single component manager", cmgr );
-        assertNull( "Expect no component manager list", getComponentManagers( holder ) );
+        assertEquals( "Expect no other component manager list", 1, getComponentManagers( holder ).length);
 
         // assert no configuration of single component
         assertFalse( "Expect no configuration", cmgr.hasConfiguration() );
@@ -166,7 +166,7 @@ public class ConfiguredComponentHolderTe
         final SingleComponentManager cmgrAfterAllUnconfig = getSingleManager( holder );
         final SingleComponentManager[] cmgrsAfterAllUnconfig = getComponentManagers( holder );
         assertNotNull( "Expect single component manager", cmgrAfterAllUnconfig );
-        assertNull( "Expect no component manager list", cmgrsAfterAllUnconfig );
+        assertEquals( "Expect no component manager list", 1, cmgrsAfterAllUnconfig.length );
 
     }