You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by dj...@apache.org on 2014/01/07 01:12:37 UTC

svn commit: r1556079 - in /felix/trunk/scr/src/main/java/org/apache/felix/scr/impl: config/ConfigurationSupport.java manager/AbstractComponentManager.java manager/ComponentContextImpl.java manager/DependencyManager.java manager/SingleComponentManager.java

Author: djencks
Date: Tue Jan  7 00:12:36 2014
New Revision: 1556079

URL: http://svn.apache.org/r1556079
Log:
FELIX-4348 fix the deadlock, and use one less lock

Modified:
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java?rev=1556079&r1=1556078&r2=1556079&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java Tue Jan  7 00:12:36 2014
@@ -315,6 +315,9 @@ public class ConfigurationSupport implem
                         //this sets the location to this component's bundle if not already set.  OK here
                         //since it used to be set to this bundle, ok to reset it
                         final ConfigurationInfo configInfo = getConfigurationInfo( pid, componentHolder, bundleContext );
+                        Activator.log(LogService.LOG_DEBUG, null, "LocationChanged event, same targetedPID {0}, location now {1}",
+                                new Object[] {targetedPid, configInfo.getBundleLocation()},
+                                null);
                         if (configInfo.getProps() == null)
                         {
                             throw new IllegalStateException("Existing Configuration with pid " + pid + 
@@ -338,6 +341,9 @@ public class ConfigurationSupport implem
                         //this sets the location to this component's bundle if not already set.  OK here
                         //because if it is set to this bundle we will use it.
                         final ConfigurationInfo configInfo = getConfigurationInfo( pid, componentHolder, bundleContext );
+                        Activator.log(LogService.LOG_DEBUG, null, "LocationChanged event, better targetedPID {0} compared to {1}, location now {2}",
+                                new Object[] {targetedPid, oldTargetedPID, configInfo.getBundleLocation()},
+                                null);
                         if (configInfo.getProps() == null)
                         {
                             //location has been changed before any properties are set.  We don't care.  Wait for an updated event with the properties
@@ -356,6 +362,12 @@ public class ConfigurationSupport implem
                         }
                     }
                     //else worse match, do nothing
+                    else
+                    {
+                        Activator.log(LogService.LOG_DEBUG, null, "LocationChanged event, worse targetedPID {0} compared to {1}, do nothing",
+                                new Object[] {targetedPid, oldTargetedPID},
+                                null);
+                    }
                     break;
                 }
                 default:

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java?rev=1556079&r1=1556078&r2=1556079&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java Tue Jan  7 00:12:36 2014
@@ -108,12 +108,6 @@ public abstract class AbstractComponentM
      */
     private final AtomicReference< CountDownLatch> m_enabledLatchRef = new AtomicReference<CountDownLatch>( new CountDownLatch(0) );
 
-    /**
-     * This ReadWriteLock prevents locateService calls from the ComponentContext from occurring while target filters are 
-     * being changed during reconfigure, when the set of bound services is unclear.
-     */
-    private final ReadWriteLock m_reconfigureLock = new ReentrantReadWriteLock();
-    
     protected volatile boolean m_enabled;
     protected volatile boolean m_internalEnabled;
     
@@ -500,24 +494,6 @@ public abstract class AbstractComponentM
         return newEnabledLatch;  
     }
 
-    void reconfigureWriteLock() {
-        obtainLock(m_reconfigureLock.writeLock(), "AbstractComponentManager.ReconfigureLock.WriteLock");
-    }
-    
-    void reconfigureWriteUnlock() 
-    {
-        m_reconfigureLock.writeLock().unlock();
-    }
-    
-    void reconfigureReadLock() {
-        obtainLock(m_reconfigureLock.readLock(), "AbstractComponentManager.ReconfigureLock.ReadLock");
-    }
-    
-    void reconfigureReadUnlock() 
-    {
-        m_reconfigureLock.readLock().unlock();
-    }
-    
     /**
      * Disables this component and - if active - first deactivates it. The
      * component may be reenabled by calling the {@link #enable()} method.

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java?rev=1556079&r1=1556078&r2=1556079&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java Tue Jan  7 00:12:36 2014
@@ -96,7 +96,7 @@ public class ComponentContextImpl<S> imp
 
     public Object locateService( String name )
     {
-        m_componentManager.reconfigureReadLock();
+        m_componentManager.obtainActivationReadLock( "locate.service.name" );
         try
         {
             DependencyManager<S, ?> dm = m_componentManager.getDependencyManager( name );
@@ -104,14 +104,14 @@ public class ComponentContextImpl<S> imp
         }
         finally
         {
-            m_componentManager.reconfigureReadUnlock();
+            m_componentManager.releaseActivationReadLock( "locate.service.name" );
         }
     }
 
 
     public Object locateService( String name, ServiceReference ref )
     {
-        m_componentManager.reconfigureReadLock();
+        m_componentManager.obtainActivationReadLock( "locate.service.ref" );
         try
         {
             DependencyManager<S, ?> dm = m_componentManager.getDependencyManager( name );
@@ -119,14 +119,14 @@ public class ComponentContextImpl<S> imp
         }
         finally
         {
-            m_componentManager.reconfigureReadUnlock();
+            m_componentManager.releaseActivationReadLock( "locate.service.ref" );
         }
     }
 
 
     public Object[] locateServices( String name )
     {
-        m_componentManager.reconfigureReadLock();
+        m_componentManager.obtainActivationReadLock( "locate.services" );
         try
         {
             DependencyManager dm = m_componentManager.getDependencyManager( name );
@@ -134,7 +134,7 @@ public class ComponentContextImpl<S> imp
         }
         finally
         {
-            m_componentManager.reconfigureReadUnlock();
+            m_componentManager.releaseActivationReadLock( "locate.services" );
         }
     }
 

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java?rev=1556079&r1=1556078&r2=1556079&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java Tue Jan  7 00:12:36 2014
@@ -961,7 +961,8 @@ public class DependencyManager<S, T> imp
             this.trackingCount = trackingCount;
             tracked( trackingCount );
             boolean reactivate;
-            synchronized (getTracker().tracked())
+            final Object sync = getTracker().tracked();
+            synchronized (sync)
             {
                 reactivate = ( isActive() && refPair == this.refPair) || ( !isOptional() && getTracker().isEmpty());
                 if (!reactivate && refPair == this.refPair) {
@@ -971,7 +972,7 @@ public class DependencyManager<S, T> imp
             if ( reactivate )
             {
                 m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
-                synchronized ( getTracker().tracked() )
+                synchronized ( sync )
                 {
                     if (refPair == this.refPair)
                     {
@@ -1318,8 +1319,13 @@ public class DependencyManager<S, T> imp
      */
     private RefPair<T> getRefPair( ServiceReference<T> serviceReference )
     {
-        AtomicInteger trackingCount = new AtomicInteger( );
-        return m_tracker.getTracked( null, trackingCount ).get( serviceReference );
+        final ServiceTracker<T, RefPair<T>> tracker = m_tracker;
+        if ( tracker != null )
+        {
+            AtomicInteger trackingCount = new AtomicInteger( );
+            return tracker.getTracked( null, trackingCount ).get( serviceReference );
+        }
+        return null;
     }
 
 

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java?rev=1556079&r1=1556078&r2=1556079&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java Tue Jan  7 00:12:36 2014
@@ -554,112 +554,104 @@ public class SingleComponentManager<S> e
         CountDownLatch enableLatch = enableLatchWait();
         try
         {
-            reconfigureWriteLock();
-            try
+            if ( targetedPID == null || !targetedPID.equals( m_targetedPID ) )
             {
-                if ( targetedPID == null || !targetedPID.equals( m_targetedPID ) )
-                {
-                    m_targetedPID = targetedPID;
-                    m_changeCount = -1;
-                }
-                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 )
+                m_targetedPID = targetedPID;
+                m_changeCount = -1;
+            }
+            if ( configuration != null )
+            {
+                if ( changeCount <= m_changeCount )
                 {
-                    log( LogService.LOG_DEBUG, "No configuration provided (or deleted), nothing to do", null );
+                    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 )
+            {
+                log( LogService.LOG_DEBUG, "No configuration provided (or deleted), nothing to do", null );
+                return;
+            }
 
-                // store the properties
-                m_configurationProperties = configuration;
+            // store the properties
+            m_configurationProperties = configuration;
 
-                // clear the current properties to force using the configuration data
-                m_properties = null;
+            // clear the current properties to force using the configuration data
+            m_properties = null;
 
 
-                // reactivate the component to ensure it is provided with the
-                // configuration data
-                if ( m_disposed || !m_internalEnabled )
-                {
-                    // nothing to do for inactive components, leave this method
-                    log( LogService.LOG_DEBUG, "Component can not be activated due to configuration in state {0}", new Object[] { getState() }, null );
-                    //enabling the component will set the target properties, do nothing now.
-                    return;
-                }
+            // reactivate the component to ensure it is provided with the
+            // configuration data
+            if ( m_disposed || !m_internalEnabled )
+            {
+                // nothing to do for inactive components, leave this method
+                log( LogService.LOG_DEBUG, "Component can not be activated since it is in state {0}", new Object[] { getState() }, null );
+                //enabling the component will set the target properties, do nothing now.
+                return;
+            }
 
-                // if the configuration has been deleted but configuration is required
-                // this component must be deactivated
-                if ( configuration == null && getComponentMetadata().isConfigurationRequired() )
+            // if the configuration has been deleted but configuration is required
+            // this component must be deactivated
+            if ( configuration == null && getComponentMetadata().isConfigurationRequired() )
+            {
+                //deactivate and remove service listeners
+                deactivateInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED, true, false );
+                //do not reset targets as that will reinstall the service listeners which may activate the component.
+                //when a configuration arrives the properties will get set based on the new configuration.
+                return;
+            }
+
+            // unsatisfied component and non-ignored configuration may change targets
+            // to satisfy references
+            obtainActivationWriteLock( "reconfigure" );
+            try
+            {
+                if ( getState() == STATE_UNSATISFIED
+                        && !getComponentMetadata().isConfigurationIgnored() )
                 {
-                    //deactivate and remove service listeners
-                    deactivateInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED, true, false );
-                    //do not reset targets as that will reinstall the service listeners which may activate the component.
-                    //when a configuration arrives the properties will get set based on the new configuration.
+                    log( LogService.LOG_DEBUG, "Attempting to activate unsatisfied component", null );
+                    updateTargets( getProperties() );
+                    releaseActivationWriteeLock( "reconfigure.unsatisfied" );
+                    activateInternal( getTrackingCount().get() );
                     return;
                 }
 
-                // unsatisfied component and non-ignored configuration may change targets
-                // to satisfy references
-                obtainActivationWriteLock( "reconfigure" );
-                try
+                if ( !modify() )
                 {
-                    if ( getState() == STATE_UNSATISFIED
-                            && !getComponentMetadata().isConfigurationIgnored() )
+                    // SCR 112.7.1 - deactivate if configuration is deleted or no modified method declared
+                    log( LogService.LOG_DEBUG, "Deactivating and Activating to reconfigure from configuration", null );
+                    int reason = ( configuration == null ) ? ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED
+                            : ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_MODIFIED;
+
+                    // FELIX-2368: cycle component immediately, reconfigure() is
+                    //     called through ConfigurationListener API which itself is
+                    //     called asynchronously by the Configuration Admin Service
+                    releaseActivationWriteeLock( "reconfigure.modified.1" );;
+                    deactivateInternal( reason, false, false );
+                    obtainActivationWriteLock( "reconfigure.deactivate.activate" );
+                    try
                     {
-                        log( LogService.LOG_DEBUG, "Attempting to activate unsatisfied component", null );
                         updateTargets( getProperties() );
-                        releaseActivationWriteeLock( "reconfigure.unsatisfied" );
-                        activateInternal( getTrackingCount().get() );
-                        return;
                     }
-
-                    if ( !modify() )
+                    finally
                     {
-                        // SCR 112.7.1 - deactivate if configuration is deleted or no modified method declared
-                        log( LogService.LOG_DEBUG, "Deactivating and Activating to reconfigure from configuration", null );
-                        int reason = ( configuration == null ) ? ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED
-                                : ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_MODIFIED;
-
-                        // FELIX-2368: cycle component immediately, reconfigure() is
-                        //     called through ConfigurationListener API which itself is
-                        //     called asynchronously by the Configuration Admin Service
-                        releaseActivationWriteeLock( "reconfigure.modified.1" );;
-                        deactivateInternal( reason, false, false );
-                        obtainActivationWriteLock( "reconfigure.deactivate.activate" );
-                        try
-                        {
-                            updateTargets( getProperties() );
-                        }
-                        finally
-                        {
-                            releaseActivationWriteeLock( "reconfigure.deactivate.activate" );;
-                        }
-                        activateInternal( getTrackingCount().get() );
+                        releaseActivationWriteeLock( "reconfigure.deactivate.activate" );;
                     }
-                }
-                finally
-                {
-                    //used if modify succeeds or if there's an exception.
-                    releaseActivationWriteeLock( "reconfigure.end" );;
+                    activateInternal( getTrackingCount().get() );
                 }
             }
             finally
             {
-                reconfigureWriteUnlock();
+                //used if modify succeeds or if there's an exception.
+                releaseActivationWriteeLock( "reconfigure.end" );;
             }
         }
         finally