FELIX-4348 fix the deadlock, and use one less lock
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1556079 13f79535-47bb-0310-9956-ffa450edef68
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 857e7c9..e2f63e9 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
@@ -315,6 +315,9 @@
//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 @@
//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 @@
}
}
//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:
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
index 4b109c9..1672a0a 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
@@ -108,12 +108,6 @@
*/
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 @@
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.
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
index b6510ae..0ead123 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
@@ -96,7 +96,7 @@
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 @@
}
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 @@
}
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 @@
}
finally
{
- m_componentManager.reconfigureReadUnlock();
+ m_componentManager.releaseActivationReadLock( "locate.services" );
}
}
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
index c502f10..a8ca1f3 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
@@ -961,7 +961,8 @@
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 @@
if ( reactivate )
{
m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
- synchronized ( getTracker().tracked() )
+ synchronized ( sync )
{
if (refPair == this.refPair)
{
@@ -1318,8 +1319,13 @@
*/
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;
}
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java
index b92105c..9536ede 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java
@@ -554,112 +554,104 @@
CountDownLatch enableLatch = enableLatchWait();
try
{
- reconfigureWriteLock();
+ 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 )
+ {
+ log( LogService.LOG_DEBUG, "No configuration provided (or deleted), nothing to do", null );
+ return;
+ }
+
+ // store the properties
+ m_configurationProperties = configuration;
+
+ // 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 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() )
+ {
+ //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 ( targetedPID == null || !targetedPID.equals( m_targetedPID ) )
+ if ( getState() == STATE_UNSATISFIED
+ && !getComponentMetadata().isConfigurationIgnored() )
{
- m_targetedPID = targetedPID;
- m_changeCount = -1;
+ log( LogService.LOG_DEBUG, "Attempting to activate unsatisfied component", null );
+ updateTargets( getProperties() );
+ releaseActivationWriteeLock( "reconfigure.unsatisfied" );
+ activateInternal( getTrackingCount().get() );
+ return;
}
- if ( configuration != null )
+
+ if ( !modify() )
{
- if ( changeCount <= m_changeCount )
+ // 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,
- "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;
-
- // 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;
- }
-
- // 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() )
- {
- 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