FELIX-3915 R5 way of eliminating timing hole using changecount
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1453151 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/pom.xml b/scr/pom.xml
index 4105f25..6d90324 100644
--- a/scr/pom.xml
+++ b/scr/pom.xml
@@ -70,6 +70,7 @@
<bundle.file.name>
${bundle.build.name}/${project.build.finalName}.jar
</bundle.file.name>
+ <felix.ca.version>1.0.10</felix.ca.version>
</properties>
@@ -77,13 +78,13 @@
<dependency>
<groupId>org.osgi</groupId>
<artifactId>org.osgi.core</artifactId>
- <version>4.3.1</version>
+ <version>5.0.0</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.osgi</groupId>
- <artifactId>org.osgi.compendium</artifactId>
- <version>4.3.1</version>
+ <artifactId>org.osgi.enterprise</artifactId>
+ <version>5.0.0</version>
<scope>provided</scope>
</dependency>
<dependency>
@@ -201,12 +202,12 @@
<version>1.0.0</version>
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>org.codehaus.mojo</groupId>
- <artifactId>animal-sniffer-annotations</artifactId>
- <version>1.10-SNAPSHOT</version>
- <scope>compile</scope>
- </dependency>
+ <dependency>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>animal-sniffer-annotations</artifactId>
+ <version>1.10-SNAPSHOT</version>
+ <scope>compile</scope>
+ </dependency>
</dependencies>
<build>
<directory>${bundle.build.name}</directory>
@@ -371,12 +372,10 @@
</execution>
</executions>
<configuration>
- <systemProperties>
- <property>
- <name>project.bundle.file</name>
- <value>${bundle.file.name}</value>
- </property>
- </systemProperties>
+ <systemPropertyVariables>
+ <project.bundle.file>${bundle.file.name}</project.bundle.file>
+ <felix.ca.version>${felix.ca.version}</felix.ca.version>
+ </systemPropertyVariables>
<excludes>
<exclude>**/components/**</exclude>
</excludes>
@@ -468,6 +467,14 @@
</dependency>
</dependencies>
</profile>
+ <profile>
+ <!-- use to test with R5 ca, change count, targetetPids -->
+ <!-- remember you need to specify all profiles using this, e.g. -PcaR5,felix -->
+ <id>caR5</id>
+ <properties>
+ <felix.ca.version>1.6.0</felix.ca.version>
+ </properties>
+ </profile>
</profiles>
</project>
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java b/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
index 4fcee90..83515c5 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
@@ -66,15 +66,17 @@
/**
* Configure a component with configuration from the given PID.
*
- * @param pid The PID of the configuration used to configure the component
+ * @param pid The PID of the configuration used to configure the component.
+ * @param props the property dictionary from the configuration.
+ * @param changeCount change count of the configuration, or R4 imitation.
*/
- void configurationUpdated( String pid, Dictionary<String, Object> props );
+ void configurationUpdated( String pid, Dictionary<String, Object> props, long changeCount );
/**
- * Has this component holder ever been configured? Used to avoid double update during startup
- * @return true if configurationUpdated has ever been called on this holder.
+ * Change count (or fake R4 imitation)
+ * @return the last change count from a configurationUpdated call for the given pid.
*/
- boolean isConfigured();
+ long getChangeCount( String pid );
/**
* Returns all <code>Component</code> instances held by this holder.
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java b/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java
index 4dd6e5b..7df6535 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java
@@ -19,6 +19,8 @@
package org.apache.felix.scr.impl.config;
import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
import java.util.Collection;
import java.util.Dictionary;
import java.util.Hashtable;
@@ -40,6 +42,27 @@
public class ConfigurationSupport implements ConfigurationListener
{
+ private static final ChangeCount changeCounter;
+ static
+ {
+ ChangeCount cc = null;
+ try
+ {
+ Configuration.class.getMethod( "getChangeCount", null );
+ cc = new R5ChangeCount();
+ }
+ catch ( SecurityException e )
+ {
+ }
+ catch ( NoSuchMethodException e )
+ {
+ }
+ if ( cc == null )
+ {
+ cc = new R4ChangeCount();
+ }
+ changeCounter = cc;
+ }
// the registry of components to be configured
private final ComponentRegistry m_registry;
@@ -74,7 +97,7 @@
{
// 112.7 configure unless configuration not required
- if (!holder.isConfigured() && !holder.getComponentMetadata().isConfigurationIgnored())
+ if (!holder.getComponentMetadata().isConfigurationIgnored())
{
final BundleContext bundleContext = holder.getActivator().getBundleContext();
final String bundleLocation = bundleContext.getBundle().getLocation();
@@ -97,8 +120,9 @@
for (int i = 0; i < factory.length; i++)
{
final String pid = factory[i].getPid();
- final Dictionary props = getConfiguration(ca, pid, bundleLocation);
- holder.configurationUpdated(pid, props);
+ final Configuration config = getConfiguration(ca, pid, bundleLocation);
+ long changeCount = changeCounter.getChangeCount( config, false, -1 );
+ holder.configurationUpdated(pid, config.getProperties(), changeCount);
}
}
else
@@ -107,8 +131,9 @@
final Configuration singleton = findSingletonConfiguration(ca, confPid);
if (singleton != null)
{
- final Dictionary props = getConfiguration(ca, confPid, bundleLocation);
- holder.configurationUpdated(confPid, props);
+ final Configuration config = getConfiguration(ca, confPid, bundleLocation);
+ long changeCount = changeCounter.getChangeCount( config, false, -1 );
+ holder.configurationUpdated(confPid, config.getProperties(), changeCount);
}
}
}
@@ -234,11 +259,12 @@
if ( cao instanceof ConfigurationAdmin )
{
final ConfigurationAdmin ca = ( ConfigurationAdmin ) cao;
- final Dictionary dict = getConfiguration( ca, pid, bundleContext
+ final Configuration config = getConfiguration( ca, pid, bundleContext
.getBundle().getLocation() );
- if ( dict != null )
+ if ( config != null )
{
- componentHolder.configurationUpdated( pid, dict );
+ long changeCount = changeCounter.getChangeCount( config, true, componentHolder.getChangeCount( pid ) );
+ componentHolder.configurationUpdated( pid, config.getProperties(), changeCount );
}
}
else
@@ -280,14 +306,14 @@
}
}
- private Dictionary getConfiguration(final ConfigurationAdmin ca, final String pid, final String bundleLocation)
+ private Configuration getConfiguration(final ConfigurationAdmin ca, final String pid, final String bundleLocation)
{
try
{
final Configuration cfg = ca.getConfiguration(pid);
if (bundleLocation.equals(cfg.getBundleLocation()))
{
- return cfg.getProperties();
+ return cfg;
}
// configuration belongs to another bundle, cannot be used here
@@ -349,4 +375,26 @@
// no factories in case of problems
return null;
}
+
+
+ private interface ChangeCount {
+ long getChangeCount( Configuration configuration, boolean fromEvent, long previous );
+ }
+
+ private static class R5ChangeCount implements ChangeCount {
+
+ public long getChangeCount(Configuration configuration, boolean fromEvent, long previous)
+ {
+ return configuration.getChangeCount();
+ }
+ }
+
+ private static class R4ChangeCount implements ChangeCount {
+
+ public long getChangeCount(Configuration configuration, boolean fromEvent, long previous)
+ {
+ return fromEvent? previous + 1:0;
+ }
+
+ }
}
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java b/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
index 7c452b7..7ae7569 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
@@ -100,14 +100,12 @@
* Whether components have already been enabled by calling the
* {@link #enableComponents(boolean)} method. If this field is <code>true</code>
* component instances created per configuration by the
- * {@link #configurationUpdated(String, Dictionary)} method are also
+ * {@link #configurationUpdated(String, Dictionary, long)} method are also
* enabled. Otherwise they are not enabled immediately.
*/
private volatile boolean m_enabled;
private final ComponentMethods m_componentMethods;
- private volatile boolean m_configured;
-
public ImmediateComponentHolder( final BundleComponentActivator activator, final ComponentMetadata metadata )
{
@@ -248,7 +246,7 @@
// is not the "last" and has to be disposed off
if ( deconfigure )
{
- icm.reconfigure( null );
+ icm.reconfigure( null, -1 );
}
else
{
@@ -270,12 +268,11 @@
* this case a new component is created, configured and stored in the map</li>
* </ul>
*/
- public void configurationUpdated( final String pid, final Dictionary<String, Object> props )
+ public void configurationUpdated( final String pid, final Dictionary<String, Object> props, long changeCount )
{
log( LogService.LOG_DEBUG, "ImmediateComponentHolder configuration updated for pid {0} with properties {1}",
new Object[] {pid, props}, null);
- m_configured = true;
// component to update or create
final ImmediateComponentManager icm;
final String message;
@@ -342,7 +339,7 @@
log( LogService.LOG_DEBUG, message, new Object[] {pid}, null);
// configure the component
- icm.reconfigure( props );
+ icm.reconfigure( props, changeCount );
log( LogService.LOG_DEBUG, "ImmediateComponentHolder Finished configuring the dependency managers for component for pid {0} ",
new Object[] {pid}, null );
@@ -359,9 +356,12 @@
}
}
- public boolean isConfigured()
+ public synchronized long getChangeCount( String pid)
{
- return m_configured;
+
+ ImmediateComponentManager icm = pid.equals( getComponentMetadata().getConfigurationPid())?
+ m_singleComponent: m_components.get( pid );
+ return icm == null? -1: icm.getChangeCount();
}
public Component[] getComponents()
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
index 6cb1b56..29e4087 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
@@ -21,6 +21,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.Dictionary;
+import java.util.HashMap;
import java.util.Hashtable;
import java.util.IdentityHashMap;
import java.util.List;
@@ -85,9 +86,9 @@
private volatile boolean m_hasConfiguration;
/**
- * whether this holder has ever been configured.
+ * Configuration change count (R5) or imitation (R4)
*/
- private volatile boolean m_isConfigured;
+ protected volatile long m_changeCount = -1;
public ComponentFactoryImpl( BundleComponentActivator activator, ComponentMetadata metadata )
{
@@ -115,7 +116,7 @@
ComponentInstance instance;
cm.setFactoryProperties( dictionary );
//configure the properties
- cm.reconfigure( m_configuration );
+ cm.reconfigure( m_configuration, m_changeCount );
// enable
cm.enableInternal();
//activate immediately
@@ -274,16 +275,6 @@
protected boolean collectDependencies()
{
-// Map<DependencyManager<S, ?>, Map<ServiceReference<?>, RefPair<?>>> old = getDependencyMap();
-// if ( old == null )
-// {
-// Map<DependencyManager<S, ?>, Map<ServiceReference<?>, RefPair<?>>> dependenciesMap = new HashMap<DependencyManager<S, ?>, Map<ServiceReference<?>, RefPair<?>>>();
-// for (DependencyManager dm: getDependencyManagers() )
-// {
-// dependenciesMap.put( dm, Collections.EMPTY_MAP );
-// }
-// setDependencyMap( old, dependenciesMap );
-// }
return true;
}
@@ -318,6 +309,7 @@
{
log( LogService.LOG_DEBUG, "Handling configuration removal", null );
+ m_changeCount = -1;
// nothing to do if there is no configuration currently known.
if ( !m_hasConfiguration )
{
@@ -346,9 +338,23 @@
}
- public void configurationUpdated( String pid, Dictionary<String, Object> configuration )
+ public void configurationUpdated( String pid, Dictionary<String, Object> configuration, long changeCount )
{
- m_isConfigured = true;
+ if ( configuration != null )
+ {
+ if ( changeCount <= m_changeCount )
+ {
+ log( LogService.LOG_DEBUG,
+ "ImmediateComponentHolder out of order configuration updated for pid {0} with existing count {1}, new count {2}",
+ new Object[] { getConfigurationPid(), m_changeCount, changeCount }, null );
+ return;
+ }
+ m_changeCount = changeCount;
+ }
+ else
+ {
+ m_changeCount = -1;
+ }
if ( pid.equals( getComponentMetadata().getConfigurationPid() ) )
{
log( LogService.LOG_INFO, "Configuration PID updated for Component Factory", null );
@@ -404,11 +410,12 @@
}
- public boolean isConfigured()
+ public synchronized long getChangeCount( String pid)
{
- return m_isConfigured;
+
+ return m_changeCount;
}
-
+
public Component[] getComponents()
{
List<AbstractComponentManager> cms = getComponentList();
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
index 02008c9..4762e90 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
@@ -56,7 +56,7 @@
* {@link org.apache.felix.scr.impl.manager.ImmediateComponentManager} for configuration updating this map is
* lazily created.
*/
- private Map m_configuredServices;
+ private Map<String, ImmediateComponentManager> m_configuredServices;
public ConfigurationComponentFactoryImpl( BundleComponentActivator activator, ComponentMetadata metadata )
{
@@ -132,23 +132,23 @@
}
- public void configurationUpdated( String pid, Dictionary<String, Object> configuration )
+ public void configurationUpdated( String pid, Dictionary<String, Object> configuration, long changeCount )
{
if ( pid.equals( getComponentMetadata().getConfigurationPid() ) )
{
- super.configurationUpdated( pid, configuration );
+ super.configurationUpdated( pid, configuration, changeCount );
}
else //non-spec backwards compatible
{
ImmediateComponentManager cm;
- Map configuredServices = m_configuredServices;
+ Map<String, ImmediateComponentManager> configuredServices = m_configuredServices;
if ( configuredServices != null )
{
cm = ( ImmediateComponentManager ) configuredServices.get( pid );
}
else
{
- m_configuredServices = new HashMap();
+ m_configuredServices = new HashMap<String, ImmediateComponentManager>();
configuredServices = m_configuredServices;
cm = null;
}
@@ -160,7 +160,7 @@
// this should not call component reactivation because it is
// not active yet
- cm.reconfigure( configuration );
+ cm.reconfigure( configuration, changeCount );
// enable asynchronously if components are already enabled
if ( getState() == STATE_FACTORY )
@@ -175,7 +175,7 @@
else
{
// update the configuration as if called as ManagedService
- cm.reconfigure( configuration );
+ cm.reconfigure( configuration, changeCount );
}
}
}
@@ -212,6 +212,20 @@
dispose( reason );
}
+ public synchronized long getChangeCount( String pid)
+ {
+
+ if (pid.equals( getComponentMetadata().getConfigurationPid()))
+ {
+ return m_changeCount;
+ }
+ if (m_configuredServices == null)
+ {
+ return -1;
+ }
+ ImmediateComponentManager icm = m_configuredServices.get( pid );
+ return icm == null? -1: icm.getChangeCount();
+ }
//---------- internal
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
index 3520634..05b2083 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
@@ -77,6 +77,8 @@
// the component properties from the Configuration Admin Service
// this is null, if none exist or none are provided
private Dictionary<String, Object> m_configurationProperties;
+
+ private volatile long m_changeCount = -1;
/**
* The constructor receives both the activator and the metadata
@@ -517,9 +519,25 @@
* @param configuration The configuration properties for the component from
* the Configuration Admin Service or <code>null</code> if there is
* no configuration or if the configuration has just been deleted.
+ * @param changeCount TODO
*/
- public void reconfigure( Dictionary<String, Object> configuration )
+ public void reconfigure( Dictionary<String, Object> configuration, long changeCount )
{
+ if ( configuration != null )
+ {
+ if ( changeCount <= m_changeCount )
+ {
+ log( LogService.LOG_DEBUG,
+ "ImmediateComponentHolder out of order configuration updated for pid {0} with existing count {1}, new count {2}",
+ new Object[] { getConfigurationPid(), m_changeCount, changeCount }, null );
+ return;
+ }
+ m_changeCount = changeCount;
+ }
+ else
+ {
+ m_changeCount = -1;
+ }
// nothing to do if there is no configuration (see FELIX-714)
if ( configuration == null && m_configurationProperties == null )
{
@@ -780,4 +798,9 @@
}
}
}
+
+ public long getChangeCount()
+ {
+ return m_changeCount;
+ }
}
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 dbc033a..028920b 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
@@ -71,7 +71,7 @@
// configure with the singleton configuration
final Dictionary config = new Hashtable();
config.put( "value", name );
- holder.configurationUpdated( name, config );
+ holder.configurationUpdated( name, config, 0 );
// assert single component and no map
final ImmediateComponentManager cmgrAfterConfig = getSingleManager( holder );
@@ -115,7 +115,7 @@
final String pid1 = "test.factory.0001";
final Dictionary config1 = new Hashtable();
config1.put( "value", pid1 );
- holder.configurationUpdated( pid1, config1 );
+ holder.configurationUpdated( pid1, config1, 0 );
// assert single component and single-entry map
final ImmediateComponentManager cmgrAfterConfig = getSingleManager( holder );
@@ -128,7 +128,7 @@
final String pid2 = "test.factory.0002";
final Dictionary config2 = new Hashtable();
config1.put( "value", pid2 );
- holder.configurationUpdated( pid2, config2 );
+ holder.configurationUpdated( pid2, config2, 1 );
// assert single component and single-entry map
final ImmediateComponentManager cmgrAfterConfig2 = getSingleManager( holder );
@@ -148,7 +148,7 @@
assertEquals( "Expect one component manager in list", 1, cmgrsAfterUnConfig2.length );
// add second config again and remove first config -> replace singleton component
- holder.configurationUpdated( pid2, config2 );
+ holder.configurationUpdated( pid2, config2, 2 );
holder.configurationDeleted( pid1 );
// assert single component and single-entry map
@@ -250,7 +250,7 @@
}
- public void reconfigure( Dictionary configuration )
+ public void reconfigure( Dictionary configuration, long changeCount )
{
this.m_configuration = configuration;
}
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java b/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java
index 0836696..b88ec11 100644
--- a/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java
+++ b/scr/src/test/java/org/apache/felix/scr/integration/ComponentTestBase.java
@@ -117,6 +117,8 @@
//set to true to only get last 1000 lines of log.
protected static boolean restrictedLogging;
+
+ protected static String felixCaVersion = System.getProperty( "felix.ca.version" );
static
@@ -153,7 +155,7 @@
provision(
CoreOptions.bundle( bundleFile.toURI().toString() ),
mavenBundle( "org.ops4j.pax.tinybundles", "tinybundles", "1.0.0" ),
- mavenBundle( "org.apache.felix", "org.apache.felix.configadmin", "1.0.10" )
+ mavenBundle( "org.apache.felix", "org.apache.felix.configadmin", felixCaVersion )
),
junitBundles(),
systemProperty( "ds.factory.enabled" ).value( Boolean.toString( NONSTANDARD_COMPONENT_FACTORY_BEHAVIOR ) ),