FELIX-1477 Apply slightly modified (formatting) patch by Andy Wilkinson
(thanks alot !) to make the implementation more thread-safe
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@804344 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java b/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java
index a74b57a..07642c1 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java
@@ -371,7 +371,10 @@
*/
public void delete( String pid )
{
- getFile( pid ).delete();
+ synchronized ( this )
+ {
+ getFile( pid ).delete();
+ }
}
@@ -385,7 +388,10 @@
*/
public boolean exists( String pid )
{
- return getFile( pid ).isFile();
+ synchronized ( this )
+ {
+ return getFile( pid ).isFile();
+ }
}
@@ -436,14 +442,19 @@
// after writing the file, rename it but ensure, that no other
// might at the same time open the new file
// see load(File)
- synchronized (this) {
+ synchronized ( this )
+ {
// make sure the cfg file does not exists (just for sanity)
- if (cfgFile.exists()) {
+ if ( cfgFile.exists() )
+ {
cfgFile.delete();
}
// rename the temporary file to the new file
- tmpFile.renameTo( cfgFile );
+ if ( !tmpFile.renameTo( cfgFile ) )
+ {
+ throw new IOException( "Failed to rename configuration file from '" + tmpFile + "' to '" + cfgFile );
+ }
}
}
finally
diff --git a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
index d9db802..0ad02e8 100644
--- a/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
+++ b/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
@@ -34,8 +34,8 @@
public class ConfigurationAdapter implements Configuration
{
- private ConfigurationAdminImpl configurationAdmin;
- private ConfigurationImpl delegatee;
+ private final ConfigurationAdminImpl configurationAdmin;
+ private final ConfigurationImpl delegatee;
ConfigurationAdapter( ConfigurationAdminImpl configurationAdmin, ConfigurationImpl delegatee )
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 16b71d1..395ebe3 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
@@ -58,30 +58,30 @@
* been assigned with possible no data.
*/
private static final String CONFIGURATION_NEW = "_felix_.cm.newConfiguration";
-
+
/**
* The {@link ConfigurationManager configuration manager} instance which
* caused this configuration object to be created.
*/
- private ConfigurationManager configurationManager;
+ private final ConfigurationManager configurationManager;
/**
* The {@link PersistenceManager persistence manager} which loaded this
* configuration instance and which is used to store and delete configuration
* data.
*/
- private PersistenceManager persistenceManager;
+ private volatile PersistenceManager persistenceManager;
/**
* The serviceReference PID of this configuration.
*/
- private String pid;
+ private final String pid;
/**
* The factory serviceReference PID of this configuration or <code>null</code> if this
* is not a factory configuration.
*/
- private String factoryPID;
+ private final String factoryPID;
/**
* The location of the bundle to which this configuration instance is bound.
@@ -89,7 +89,7 @@
* {@link #configurationAdmin}. If this configuration is not bound to a
* bundle, this field is <code>null</code>.
*/
- private String bundleLocation;
+ private volatile String bundleLocation;
/**
* The <code>ServiceReference</code> of the serviceReference which first asked for
@@ -98,7 +98,7 @@
* or <code>ManagedServiceFactory.updated(String, Dictionary)</code>
* method.
*/
- private ServiceReference serviceReference;
+ private volatile ServiceReference serviceReference;
/**
* The configuration data of this configuration instance. This is a private
@@ -107,7 +107,7 @@
* the configuration has been created and never been updated with acutal
* configuration properties.
*/
- private CaseInsensitiveDictionary properties;
+ private volatile CaseInsensitiveDictionary properties;
/**
* Flag indicating that this configuration object has been delivered to the
@@ -119,17 +119,27 @@
* @see #setDelivered(boolean)
* @see #isDelivered()
*/
- private boolean delivered;
+ private volatile boolean delivered;
ConfigurationImpl( ConfigurationManager configurationManager, PersistenceManager persistenceManager,
Dictionary properties )
{
+ if ( configurationManager == null )
+ {
+ throw new IllegalArgumentException( "ConfigurationManager must not be null" );
+ }
+
+ if ( persistenceManager == null )
+ {
+ throw new IllegalArgumentException( "PersistenceManager must not be null" );
+ }
+
this.configurationManager = configurationManager;
this.persistenceManager = persistenceManager;
- pid = ( String ) properties.remove( Constants.SERVICE_PID );
- factoryPID = ( String ) properties.remove( ConfigurationAdmin.SERVICE_FACTORYPID );
- bundleLocation = ( String ) properties.remove( ConfigurationAdmin.SERVICE_BUNDLELOCATION );
+ this.pid = ( String ) properties.remove( Constants.SERVICE_PID );
+ this.factoryPID = ( String ) properties.remove( ConfigurationAdmin.SERVICE_FACTORYPID );
+ this.bundleLocation = ( String ) properties.remove( ConfigurationAdmin.SERVICE_BUNDLELOCATION );
// set the properties internally
configureFromPersistence( properties );
@@ -139,8 +149,19 @@
ConfigurationImpl( ConfigurationManager configurationManager, PersistenceManager persistenceManager, String pid,
String factoryPid, String bundleLocation ) throws IOException
{
+ if ( configurationManager == null )
+ {
+ throw new IllegalArgumentException( "ConfigurationManager must not be null" );
+ }
+
+ if ( persistenceManager == null )
+ {
+ throw new IllegalArgumentException( "PersistenceManager must not be null" );
+ }
+
this.configurationManager = configurationManager;
this.persistenceManager = persistenceManager;
+
this.pid = pid;
this.factoryPID = factoryPid;
this.bundleLocation = bundleLocation;
@@ -181,19 +202,20 @@
/*
* (non-Javadoc)
- *
+ *
* @see org.osgi.service.cm.Configuration#delete()
*/
public void delete() throws IOException
{
- if ( !isDeleted() )
+ PersistenceManager localPersistenceManager = getPersistenceManager();
+ if ( localPersistenceManager != null )
{
- persistenceManager.delete( pid );
- persistenceManager = null;
+ localPersistenceManager.delete( this.pid );
+ this.persistenceManager = null;
// ensure configuration is being delivered
setDelivered( false );
-
+
configurationManager.deleted( this );
}
}
@@ -234,7 +256,7 @@
* <code>deepCopy</code> parameter is true array and collection values are
* copied into new arrays or collections. Otherwise just a new dictionary
* referring to the same objects is returned.
- *
+ *
* @param deepCopy
* <code>true</code> if a deep copy is to be returned.
* @return
@@ -283,10 +305,11 @@
*/
public void update() throws IOException
{
- if ( !isDeleted() )
+ PersistenceManager localPersistenceManager = getPersistenceManager();
+ if ( localPersistenceManager != null )
{
// read configuration from persistence (again)
- Dictionary properties = persistenceManager.load( pid );
+ Dictionary properties = localPersistenceManager.load( pid );
// ensure serviceReference pid
String servicePid = ( String ) properties.get( Constants.SERVICE_PID );
@@ -300,7 +323,7 @@
// ensure configuration is being delivered
setDelivered( false );
-
+
configurationManager.updated( this );
}
}
@@ -311,16 +334,17 @@
*/
public void update( Dictionary properties ) throws IOException
{
- if ( !isDeleted() )
+ PersistenceManager localPersistenceManager = getPersistenceManager();
+ if ( localPersistenceManager != null )
{
CaseInsensitiveDictionary newProperties = new CaseInsensitiveDictionary( properties );
setAutoProperties( newProperties, true );
- persistenceManager.store( pid, newProperties );
+ localPersistenceManager.store( pid, newProperties );
configure( newProperties );
-
+
// if this is a factory configuration, update the factory with
String factoryPid = getFactoryPid();
if ( factoryPid != null )
@@ -336,7 +360,7 @@
// ensure configuration is being delivered
setDelivered( false );
-
+
configurationManager.updated( this );
}
}
@@ -423,19 +447,37 @@
}
+ /**
+ * Returns <code>true</code> if this configuration has already been deleted
+ * on the persistence.
+ */
boolean isDeleted()
{
- if ( persistenceManager != null )
+ PersistenceManager persistenceManager = getPersistenceManager();
+ return persistenceManager == null;
+ }
+
+
+ /**
+ * Returns the persistence manager used to store this configuration or
+ * <code>null</code> if the configuration has been removed.
+ */
+ private PersistenceManager getPersistenceManager()
+ {
+ PersistenceManager localPersistenceManager = this.persistenceManager;
+ if ( localPersistenceManager != null )
{
- if ( properties == null || persistenceManager.exists( pid ) )
+ boolean exists = localPersistenceManager.exists( pid );
+
+ if ( this.properties == null || exists )
{
- return false;
+ return localPersistenceManager;
}
- persistenceManager = null;
+ this.persistenceManager = null;
}
- return true;
+ return null;
}
@@ -452,7 +494,7 @@
this.properties = null;
}
}
-
+
private void configure( Dictionary properties )
{
// remove predefined properties
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 f129642..82e5b16 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
@@ -560,6 +560,7 @@
if ( persistenceManagers == null || currentPmtCount > pmtCount )
{
+ List pmList = new ArrayList();
PersistenceManager[] pm;
ServiceReference[] refs = persistenceManagerTracker.getServiceReferences();
@@ -577,16 +578,21 @@
}
// create the service array from the sorted set of referenecs
- pm = new PersistenceManager[pms.size()];
int pmIndex = 0;
- for ( Iterator pi = pms.iterator(); pi.hasNext(); pmIndex++)
+ for ( Iterator pi = pms.iterator(); pi.hasNext(); pmIndex++ )
{
ServiceReference ref = ( ServiceReference ) pi.next();
- pm[pmIndex] = ( PersistenceManager ) persistenceManagerTracker.getService( ref );
+ Object service = persistenceManagerTracker.getService( ref );
+ if ( service != null )
+ {
+ pmList.add( service );
+ }
}
+
+ pm = ( PersistenceManager[] ) pmList.toArray( new PersistenceManager[pmList.size()] );
}
- pmtCount = currentPmtCount;
+ pmtCount = pm.length;
persistenceManagers = pm;
}