FELIX-4323 fix CCH.getComponents to return complete list of components, including possible single component.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1543735 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java b/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
index 315137c..c33620a 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
@@ -29,6 +29,9 @@
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 @@
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 @@
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 @@
*/
void dispose( int reason )
{
- synchronized ( this )
+ if ( m_active.compareAndSet( true, false ))
{
- if ( !m_active )
- {
- return;
- }
- // 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 );
+ 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 )
+ while ( m_managers.size() != 0 )
+ {
+ 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() );
+ }
+
+ }
+
+ 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 boolean isActive()
{
- return m_active;
+ return m_active.get();
}
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java b/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java
index 1a7ab04..f35e022 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java
@@ -343,20 +343,22 @@
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 @@
{
m_enabled = true;
cms = getComponentManagers( false );
- if ( cms == null )
- {
- cms = new SingleComponentManager[] { m_singleComponent };
- }
}
for ( SingleComponentManager cm : cms )
{
@@ -388,10 +386,6 @@
m_enabled = false;
cms = getComponentManagers( false );
- if ( cms == null )
- {
- cms = new SingleComponentManager[] { m_singleComponent };
- }
}
for ( SingleComponentManager cm : cms )
{
@@ -405,16 +399,7 @@
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 @@
//---------- 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;
- }
+ 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;
+ }
+ }
- final SingleComponentManager[] cm = new SingleComponentManager[m_components.size()];
- m_components.values().toArray( cm );
-
- if ( clear )
- {
- m_components.clear();
- }
- return cm;
- }
+ 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 )
{
diff --git a/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java b/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java
index 5d301c4..946ac2c 100644
--- a/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java
+++ b/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java
@@ -47,7 +47,7 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
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 );
}