FELIX-3231 Remove update counter and rename lastModificationTime to revision to better reflect the intent of the field.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1203134 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
index 1c48117..5b07060 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
@@ -59,7 +59,7 @@
* Schedules task to update service with the configuration
*
* T1. Runs again creating the UpdateConfiguration task with the
- * configuration persisted before being preempted
+ * configuration persisted before being preempted
* Schedules task to update service
*
* Update Thread:
@@ -67,33 +67,14 @@
* Updates ManagedService with configuration prepared by T1
*
* The correct behaviour would be here, that the second call to update
- * would not take place.
+ * would not take place. We cannot at this point in time easily fix
+ * this issue. Also, it seems that changes for this to happen are
+ * small.
*
- * This concurrency safety is implemented with the help of the
- * lastModificationTime field updated by the configure(Dictionary) method
- * when setting the properties field and the lastUpdatedTime field updated
- * in the Update Thread after calling the update(Dictionary) method of
- * the ManagedService[Factory] service.
- *
- * The UpdateConfiguration task compares the lastModificationTime to the
- * lastUpdateTime. If the configuration has been modified after being
- * updated the last time, it is updated in the ManagedService[Factory]. If
- * the configuration has already been updated since being modified (as in
- * the case above), the UpdateConfiguration thread does not call the update
- * method (but still sends the CM_UPDATED event).
- *
- * See also FELIX-1542.
- *
- * FELIX-1545 provides further update to the concurrency situation defining
- * three more failure cases:
- *
- * (1) System.currentTimeMillis() may be too coarse graind to protect
- * against race condition.
- * (2) ManagedService update sets last update time regardless of whether
- * configuration was provided or not. This may cause a configuration
- * update to be lost.
- * (3) ManagedService update does not respect last update time which
- * in turn may cause duplicate configuration delivery.
+ * This class provides modification counter (lastModificationTime)
+ * which is incremented on each change of the configuration. This
+ * helps the update tasks in the ConfigurationManager to log the
+ * revision of the configuration supplied.
*/
/**
@@ -150,23 +131,13 @@
private volatile boolean isDeleted;
/**
- * Current configuration modification counter. This field is incremented
- * each time the {@link #properties} field is set (in the constructor or the
- * {@link #configure(Dictionary)} method. field.
+ * Configuration revision counter incremented each time the
+ * {@link #properties} is set (in the constructor or the
+ * {@link #configure(Dictionary)} method. This counter is transient
+ * and not persisted. Thus it is restarted from zero each time
+ * an instance of this class is created.
*/
- private volatile long lastModificationTime;
-
- /**
- * Value of the {@link #lastModificationTime} counter at the time the non-
- * <code>null</code> properties of this configuration have been updated to a
- * ManagedService[Factory]. This field is initialized to -1 in the
- * constructors and set to the value of the {@link #lastModificationTime} by
- * the {@link #setLastUpdatedTime()} method called from the respective task
- * updating the configuration.
- *
- * @see #lastModificationTime
- */
- private volatile long lastUpdatedTime;
+ private volatile long revision;
ConfigurationImpl( ConfigurationManager configurationManager, PersistenceManager persistenceManager,
@@ -176,7 +147,6 @@
this.factoryPID = ( String ) properties.remove( ConfigurationAdmin.SERVICE_FACTORYPID );
this.isDeleted = false;
- this.lastUpdatedTime = -1;
// set bundle location from persistence and/or check for dynamic binding
this.staticBundleLocation = ( String ) properties.remove( ConfigurationAdmin.SERVICE_BUNDLELOCATION ) ;
@@ -194,7 +164,6 @@
this.factoryPID = factoryPid;
this.isDeleted = false;
- this.lastUpdatedTime = -1;
// set bundle location from persistence and/or check for dynamic binding
this.staticBundleLocation = bundleLocation;
@@ -202,7 +171,7 @@
// first "update"
this.properties = null;
- setLastModificationTime();
+ this.revision = 1;
// this is a new configuration object, store immediately unless
// the new configuration object is created from a factory, in which
@@ -503,73 +472,15 @@
/**
- * Increments the last modification counter of this configuration to cause
- * the ManagedService or ManagedServiceFactory subscribed to this
- * configuration to be updated.
+ * Returns the revision of this configuration object.
* <p>
- * This method is intended to only be called by the constructor(s) of this
- * class and the {@link #update(Dictionary)} method to indicate to the
- * update threads, the configuration is ready for distribution.
- * <p>
- * Setting the properties field and incrementing this counter should be
- * done synchronized on this instance.
+ * When getting both the configuration properties and this revision
+ * counter, the two calls should be synchronized on this instance to
+ * ensure configuration values and revision counter match.
*/
- void setLastModificationTime( )
+ long getRevision()
{
- this.lastModificationTime++;
- }
-
-
- /**
- * Returns the modification counter of the last modification of the
- * properties of this configuration object.
- * <p>
- * This value may be compared to the {@link #getLastUpdatedTime()} to decide
- * whether to update the ManagedService[Factory] or not.
- * <p>
- * Getting the properties of this configuration and this counter should be
- * done synchronized on this instance.
- */
- long getLastModificationTime()
- {
- return lastModificationTime;
- }
-
-
- /**
- * Returns the modification counter of the last update of this configuration
- * to the subscribing ManagedService or ManagedServiceFactory. This value
- * may be compared to the {@link #getLastModificationTime()} to decide
- * whether the configuration should be updated or not.
- */
- long getLastUpdatedTime()
- {
- return lastUpdatedTime;
- }
-
-
- /**
- * Sets the last update time field to the given value of the last
- * modification time to indicate the version of configuration properties
- * that have been updated in a ManagedService[Factory].
- * <p>
- * This method should only be called from the Update Thread after supplying
- * the configuration to the ManagedService[Factory].
- *
- * @param lastModificationTime The value of the
- * {@link #getLastModificationTime() last modification time field} at
- * which the properties have been extracted from the configuration to
- * be supplied to the service.
- */
- void setLastUpdatedTime( long lastModificationTime )
- {
- synchronized ( this )
- {
- if ( this.lastUpdatedTime < lastModificationTime )
- {
- this.lastUpdatedTime = lastModificationTime;
- }
- }
+ return revision;
}
@@ -635,7 +546,7 @@
synchronized ( this )
{
this.properties = newProperties;
- setLastModificationTime();
+ this.revision++;
}
}
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 673a3fb..9c0cb74 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
@@ -1114,6 +1114,8 @@
{
if ( location == null )
{
+ log( LogService.LOG_DEBUG, "canReceive=true; bundle={0}; configuration:(unbound)", new Object[]
+ { bundle.getLocation() } );
return true;
}
else if ( location.startsWith( "?" ) )
@@ -1121,17 +1123,30 @@
// multi-location
if ( System.getSecurityManager() != null )
{
- return bundle.hasPermission( new ConfigurationPermission( location, ConfigurationPermission.TARGET ) );
+ final boolean hasPermission = bundle.hasPermission( new ConfigurationPermission( location,
+ ConfigurationPermission.TARGET ) );
+ log( LogService.LOG_DEBUG, "canReceive={0}: bundle={1}; configuration={2} (SecurityManager check)",
+ new Object[]
+ { new Boolean( hasPermission ), bundle.getLocation(), location } );
+ return hasPermission;
}
+
+ log( LogService.LOG_DEBUG, "canReceive=true; bundle={0}; configuration={1} (no SecurityManager)",
+ new Object[]
+ { bundle.getLocation(), location } );
return true;
}
else
{
// single location, must match
- return location.equals( bundle.getLocation() );
+ final boolean hasPermission = location.equals( bundle.getLocation() );
+ log( LogService.LOG_DEBUG, "canReceive={0}: bundle={1}; configuration={2}", new Object[]
+ { new Boolean( hasPermission ), bundle.getLocation(), location } );
+ return hasPermission;
}
}
+
// ---------- inner classes
private ServiceHelper createServiceHelper( ConfigurationImpl config )
@@ -1324,7 +1339,7 @@
private final Dictionary rawProperties;
- private final long lastModificationTime;
+ private final long revision;
ManagedServiceUpdate( String pid, ServiceReference sr, ManagedService service )
{
@@ -1335,7 +1350,7 @@
// get or load configuration for the pid
ConfigurationImpl config = null;
Dictionary rawProperties = null;
- long lastModificationTime = -1;
+ long revision = -1;
try
{
config = getConfiguration( pid );
@@ -1344,7 +1359,7 @@
synchronized ( config )
{
rawProperties = config.getProperties( true );
- lastModificationTime = config.getLastModificationTime();
+ revision = config.getRevision();
}
}
}
@@ -1356,31 +1371,19 @@
this.config = config;
this.rawProperties = rawProperties;
- this.lastModificationTime = lastModificationTime;
+ this.revision = revision;
}
public void run()
{
- // only update configuration if lastModificationTime is less than
- // lastUpdateTime
Dictionary properties = rawProperties;
- if ( properties != null && config != null && lastModificationTime < config.getLastUpdatedTime() )
- {
- log(
- LogService.LOG_DEBUG,
- "Configuration {0} at modification #{1} has already been updated to update #{2}, nothing to be done anymore.",
- new Object[]
- { config.getPid(), new Long( config.getLastModificationTime() ),
- new Long( config.getLastUpdatedTime() ) } );
- return;
- }
// check configuration and call plugins if existing
if ( config != null )
{
- log( LogService.LOG_DEBUG, "Updating configuration {0} to modification #{1}", new Object[]
- { pid, new Long( config.getLastModificationTime() ) } );
+ log( LogService.LOG_DEBUG, "Updating configuration {0} to revision #{1}", new Object[]
+ { pid, new Long( revision ) } );
Bundle serviceBundle = sr.getBundle();
if ( serviceBundle == null )
@@ -1432,14 +1435,6 @@
{
handleCallBackError( t, sr, config );
}
-
- // update the lastUpdatedTime if there is configuration
- if ( config != null && properties != null )
- {
- config.setLastUpdatedTime( lastModificationTime );
- log( LogService.LOG_DEBUG, "Updated configuration {0} to update #{1}", new Object[]
- { config.getPid(), new Long( config.getLastUpdatedTime() ) } );
- }
}
public String toString()
@@ -1465,7 +1460,7 @@
private final Map configs;
- private final Map stamps;
+ private final Map revisions;
ManagedServiceFactoryUpdate( String factoryPid, ServiceReference sr, ManagedServiceFactory service )
{
@@ -1475,13 +1470,13 @@
Factory factory = null;
Map configs = null;
- Map stamps = null;
+ Map revisions = null;
try
{
factory = getFactory( factoryPid );
if (factory != null) {
configs = new HashMap();
- stamps = new HashMap();
+ revisions = new HashMap();
for ( Iterator pi = factory.getPIDs().iterator(); pi.hasNext(); )
{
final String pid = ( String ) pi.next();
@@ -1534,7 +1529,7 @@
synchronized ( cfg )
{
configs.put( cfg, cfg.getProperties( true ) );
- stamps.put( cfg, new Long( cfg.getLastModificationTime() ) );
+ revisions.put( cfg, new Long( cfg.getRevision() ) );
}
}
}
@@ -1546,7 +1541,7 @@
}
this.configs = configs;
- this.stamps = stamps;
+ this.revisions = revisions;
}
@@ -1568,21 +1563,10 @@
final Map.Entry entry = (Map.Entry) ci.next();
final ConfigurationImpl cfg = (ConfigurationImpl) entry.getKey();
final Dictionary properties = (Dictionary) entry.getValue();
- final long lastModificationTime = ( ( Long ) stamps.get( cfg ) ).longValue();
+ final long revision = ( ( Long ) revisions.get( cfg ) ).longValue();
- if ( lastModificationTime <= cfg.getLastUpdatedTime() )
- {
- log(
- LogService.LOG_DEBUG,
- "Configuration {0} at modification #{1} has already been updated to update #{2}, nothing to be done anymore.",
- new Object[]
- { cfg.getPid(), new Long( cfg.getLastModificationTime() ),
- new Long( cfg.getLastUpdatedTime() ) } );
- continue;
- }
-
- log( LogService.LOG_DEBUG, "Updating configuration {0} to modification #{1}", new Object[]
- { cfg.getPid(), new Long( cfg.getLastModificationTime() ) } );
+ log( LogService.LOG_DEBUG, "Updating configuration {0} to revision #{1}", new Object[]
+ { cfg.getPid(), new Long( revision ) } );
// CM 1.4 / 104.13.2.1
if ( !canReceive( serviceBundle, cfg.getBundleLocation() ) )
@@ -1616,12 +1600,6 @@
{
handleCallBackError( t, sr, cfg );
}
-
- // update the lastUpdatedTime
- cfg.setLastUpdatedTime( lastModificationTime );
-
- log( LogService.LOG_DEBUG, "Updated configuration {0} to update #{1}", new Object[]
- { cfg.getPid(), new Long( cfg.getLastUpdatedTime() ) } );
}
}
}
@@ -1645,7 +1623,7 @@
private final ConfigurationImpl config;
private final ServiceHelper helper;
- private final long lastModificationTime;
+ private final long revision;
UpdateConfiguration( final ConfigurationImpl config )
@@ -1654,26 +1632,15 @@
synchronized ( config )
{
this.helper = createServiceHelper( config );
- this.lastModificationTime = config.getLastModificationTime();
+ this.revision = config.getRevision();
}
}
public void run()
{
- if ( lastModificationTime <= config.getLastUpdatedTime() )
- {
- log(
- LogService.LOG_DEBUG,
- "Configuration {0} at modification #{1} has already been updated to update #{2}, nothing to be done anymore.",
- new Object[]
- { config.getPid(), new Long( config.getLastModificationTime() ),
- new Long( config.getLastUpdatedTime() ) } );
- return;
- }
-
- log( LogService.LOG_DEBUG, "Updating configuration {0} to modification #{1}", new Object[]
- { config.getPid(), new Long( config.getLastModificationTime() ) } );
+ log( LogService.LOG_DEBUG, "Updating configuration {0} to revision #{1}", new Object[]
+ { config.getPid(), new Long( revision ) } );
final ServiceReference[] srList = helper.getServices();
if ( srList != null )
@@ -1700,17 +1667,7 @@
}
helper.provide( ref );
-
- log(
- LogService.LOG_DEBUG,
- "Updated {0} with configuration {1} (update #{1})",
- new Object[]
- { ConfigurationManager.toString( ref ), config.getPid(),
- new Long( config.getLastUpdatedTime() ) } );
}
-
- // update the lastUpdatedTime
- config.setLastUpdatedTime( lastModificationTime );
}
else if ( isLogEnabled( LogService.LOG_DEBUG ) )
{