You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by fm...@apache.org on 2009/08/14 21:50:44 UTC

svn commit: r804344 - in /felix/trunk/configadmin/src/main/java/org/apache/felix/cm: file/FilePersistenceManager.java impl/ConfigurationAdapter.java impl/ConfigurationImpl.java impl/ConfigurationManager.java

Author: fmeschbe
Date: Fri Aug 14 19:50:43 2009
New Revision: 804344

URL: http://svn.apache.org/viewvc?rev=804344&view=rev
Log:
FELIX-1477 Apply slightly modified (formatting) patch by Andy Wilkinson
(thanks alot !) to make the implementation more thread-safe

Modified:
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
    felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java?rev=804344&r1=804343&r2=804344&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/file/FilePersistenceManager.java Fri Aug 14 19:50:43 2009
@@ -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

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java?rev=804344&r1=804343&r2=804344&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationAdapter.java Fri Aug 14 19:50:43 2009
@@ -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 )

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java?rev=804344&r1=804343&r2=804344&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationImpl.java Fri Aug 14 19:50:43 2009
@@ -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

Modified: felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java?rev=804344&r1=804343&r2=804344&view=diff
==============================================================================
--- felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java (original)
+++ felix/trunk/configadmin/src/main/java/org/apache/felix/cm/impl/ConfigurationManager.java Fri Aug 14 19:50:43 2009
@@ -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;
         }