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 2013/12/08 09:19:12 UTC
svn commit: r1549004 - in /felix/trunk/scr: ./
src/main/java/org/apache/felix/scr/impl/manager/
Author: djencks
Date: Sun Dec 8 08:19:12 2013
New Revision: 1549004
URL: http://svn.apache.org/r1549004
Log:
FELIX-4348 Use a read-write lock to avoid a race between lookupService and reconfiguration
Modified:
felix/trunk/scr/changelog.txt
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/SingleComponentManager.java
Modified: felix/trunk/scr/changelog.txt
URL: http://svn.apache.org/viewvc/felix/trunk/scr/changelog.txt?rev=1549004&r1=1549003&r2=1549004&view=diff
==============================================================================
--- felix/trunk/scr/changelog.txt (original)
+++ felix/trunk/scr/changelog.txt Sun Dec 8 08:19:12 2013
@@ -1,3 +1,20 @@
+Changes from 1.8 to 1.8.2
+---------------------------
+
+** Bug
+ * [FELIX-4309] - SCR leaves some components in DISABLED state
+ * [FELIX-4313] - Bad synchronization in scr where a lock is held while ungetting a service
+ * [FELIX-4322] - [DS] Prevent activation attempts until all dependency managers are set up with trackers
+ * [FELIX-4323] - [DS] ScrService.getComponents may return a null array element
+ * [FELIX-4325] - [DS] Synchronization issue when activating component
+ * [FELIX-4326] - Possible Invalid BundleContext exception when shutting down the extender
+ * [FELIX-4348] - [DS] locateService calls race with component reconfiguration
+
+** Improvement
+ * [FELIX-4316] - Packages imported dynamically should also be imported statically with an optional flag
+ * [FELIX-4317] - SCR implementation should avoid using bundleContext.getBundle()
+ * [FELIX-4343] - [DS] rationalize log levels
+
Changes from 1.6.2 to 1.8
---------------------------
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=1549004&r1=1549003&r2=1549004&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 Sun Dec 8 08:19:12 2013
@@ -36,6 +36,7 @@ import java.util.concurrent.atomic.Atomi
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;
@@ -101,9 +102,19 @@ public abstract class AbstractComponentM
private final ReentrantLock m_stateLock;
- protected volatile boolean m_enabled;
- protected final AtomicReference< CountDownLatch> m_enabledLatchRef = new AtomicReference<CountDownLatch>( new CountDownLatch(0) );
+ /**
+ * This latch prevents concurrent enable, disable, and reconfigure. Since the enable and disable operations may use
+ * two threads and the initiating thread does not wait for the operation to complete, we can't use a regular lock.
+ */
+ 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;
protected volatile boolean m_disposed;
@@ -489,6 +500,24 @@ 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=1549004&r1=1549003&r2=1549004&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 Sun Dec 8 08:19:12 2013
@@ -96,22 +96,46 @@ public class ComponentContextImpl<S> imp
public Object locateService( String name )
{
- DependencyManager<S, ?> dm = m_componentManager.getDependencyManager( name );
- return ( dm != null ) ? dm.getService() : null;
+ m_componentManager.reconfigureReadLock();
+ try
+ {
+ DependencyManager<S, ?> dm = m_componentManager.getDependencyManager( name );
+ return ( dm != null ) ? dm.getService() : null;
+ }
+ finally
+ {
+ m_componentManager.reconfigureReadUnlock();
+ }
}
public Object locateService( String name, ServiceReference ref )
{
- DependencyManager<S, ?> dm = m_componentManager.getDependencyManager( name );
- return ( dm != null ) ? dm.getService( ref ) : null;
+ m_componentManager.reconfigureReadLock();
+ try
+ {
+ DependencyManager<S, ?> dm = m_componentManager.getDependencyManager( name );
+ return ( dm != null ) ? dm.getService( ref ) : null;
+ }
+ finally
+ {
+ m_componentManager.reconfigureReadUnlock();
+ }
}
public Object[] locateServices( String name )
{
- DependencyManager dm = m_componentManager.getDependencyManager( name );
- return ( dm != null ) ? dm.getServices() : null;
+ m_componentManager.reconfigureReadLock();
+ try
+ {
+ DependencyManager dm = m_componentManager.getDependencyManager( name );
+ return ( dm != null ) ? dm.getServices() : null;
+ }
+ finally
+ {
+ m_componentManager.reconfigureReadUnlock();
+ }
}
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=1549004&r1=1549003&r2=1549004&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 Sun Dec 8 08:19:12 2013
@@ -547,108 +547,115 @@ public class SingleComponentManager<S> e
*/
public void reconfigure( Dictionary<String, Object> configuration, long changeCount, TargetedPID targetedPID )
{
- CountDownLatch enableLatch = null;
+ CountDownLatch enableLatch = enableLatchWait();
try
{
- enableLatch = enableLatchWait();
- if ( targetedPID == null || !targetedPID.equals( m_targetedPID ) )
- {
- m_targetedPID = targetedPID;
- m_changeCount = -1;
- }
- if ( configuration != null )
+ reconfigureWriteLock();
+ try
{
- if ( changeCount <= m_changeCount )
+ 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
{
- 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 );
+ 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;
}
- 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;
- }
- // 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;
- }
+ // 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;
+ }
- // unsatisfied component and non-ignored configuration may change targets
- // to satisfy references
- obtainActivationWriteLock( "reconfigure" );
- try
- {
- if ( getState() == STATE_UNSATISFIED
- && !getComponentMetadata().isConfigurationIgnored() )
+ // if the configuration has been deleted but configuration is required
+ // this component must be deactivated
+ if ( configuration == null && getComponentMetadata().isConfigurationRequired() )
{
- log( LogService.LOG_DEBUG, "Attempting to activate unsatisfied component", null );
- updateTargets( getProperties() );
- releaseActivationWriteeLock( "reconfigure.unsatisfied" );
- activateInternal( getTrackingCount().get() );
+ //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;
}
- if ( !modify() )
+ // unsatisfied component and non-ignored configuration may change targets
+ // to satisfy references
+ obtainActivationWriteLock( "reconfigure" );
+ try
{
- // 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
+ 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;
}
- finally
+
+ if ( !modify() )
{
- releaseActivationWriteeLock( "reconfigure.deactivate.activate" );;
+ // 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() );
}
- activateInternal( getTrackingCount().get() );
+ }
+ finally
+ {
+ //used if modify succeeds or if there's an exception.
+ releaseActivationWriteeLock( "reconfigure.end" );;
}
}
finally
{
- //used if modify succeeds or if there's an exception.
- releaseActivationWriteeLock( "reconfigure.end" );;
+ reconfigureWriteUnlock();
}
}
finally