FELIX-4223 Use read-write lock to prevent activation/deactivation during config update
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1523553 13f79535-47bb-0310-9956-ffa450edef68
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 5140c03..e812dc2 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
@@ -37,6 +37,7 @@
import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import org.apache.felix.scr.Component;
import org.apache.felix.scr.Reference;
@@ -105,7 +106,7 @@
protected volatile boolean m_internalEnabled;
- private volatile boolean m_disposed;
+ protected volatile boolean m_disposed;
//service event tracking
private int m_floor;
@@ -117,6 +118,8 @@
private final Set<Integer> m_missing = new TreeSet<Integer>( );
volatile boolean m_activated;
+
+ protected final ReentrantReadWriteLock m_activationLock = new ReentrantReadWriteLock();
/**
* The constructor receives both the activator and the metadata
@@ -170,13 +173,23 @@
}
}
- final void obtainWriteLock( String source )
+ final long getLockTimeout()
+ {
+ BundleComponentActivator activator = getActivator();
+ if ( activator != null )
+ {
+ return activator.getConfiguration().lockTimeout();
+ }
+ return ScrConfiguration.DEFAULT_LOCK_TIMEOUT_MILLISECONDS;
+ }
+
+ private void obtainLock( Lock lock, String source )
{
try
{
- if (!m_stateLock.tryLock( getLockTimeout(), TimeUnit.MILLISECONDS ) )
+ if (!lock.tryLock( getLockTimeout(), TimeUnit.MILLISECONDS ) )
{
- dumpThreads();
+ dumpThreads();
throw new IllegalStateException( "Could not obtain lock" );
}
}
@@ -184,7 +197,7 @@
{
try
{
- if (!m_stateLock.tryLock( getLockTimeout(), TimeUnit.MILLISECONDS ) )
+ if (!lock.tryLock( getLockTimeout(), TimeUnit.MILLISECONDS ) )
{
dumpThreads();
throw new IllegalStateException( "Could not obtain lock" );
@@ -199,28 +212,46 @@
Thread.currentThread().interrupt();
}
}
-
- long getLockTimeout()
+
+ final void obtainActivationReadLock( String source )
{
- BundleComponentActivator activator = getActivator();
- if ( activator != null )
- {
- return activator.getConfiguration().lockTimeout();
- }
- return ScrConfiguration.DEFAULT_LOCK_TIMEOUT_MILLISECONDS;
+ obtainLock( m_activationLock.readLock(), source);
}
- final void releaseWriteLock( String source )
+ final void releaseActivationReadLock( String source )
+ {
+ m_activationLock.readLock().unlock();
+ }
+
+ final void obtainActivationWriteLock( String source )
+ {
+ obtainLock( m_activationLock.writeLock(), source);
+ }
+
+ final void releaseActivationWriteeLock( String source )
+ {
+ if ( m_activationLock.getWriteHoldCount() > 0 )
+ {
+ m_activationLock.writeLock().unlock();
+ }
+ }
+
+ final void obtainStateLock( String source )
+ {
+ obtainLock( m_stateLock, source );
+ }
+
+ final void releaseStateLock( String source )
{
m_stateLock.unlock();
}
- final boolean isWriteLocked()
+ final boolean isStateLocked()
{
return m_stateLock.getHoldCount() > 0;
}
- void dumpThreads()
+ final void dumpThreads()
{
try
{
@@ -383,10 +414,6 @@
activateInternal( m_trackingCount.get() );
}
}
- catch ( InterruptedException e )
- {
- Thread.currentThread().interrupt();
- }
finally
{
if ( !async )
@@ -424,14 +451,38 @@
}
}
- CountDownLatch enableLatchWait() throws InterruptedException
+ /**
+ * Use a CountDownLatch as a non-reentrant "lock" that can be passed between threads.
+ * This lock assures that enable, disable, and reconfigure operations do not overlap.
+ *
+ * @return the latch to count down when the operation is complete (in the calling or another thread)
+ * @throws InterruptedException
+ */
+ CountDownLatch enableLatchWait()
{
CountDownLatch enabledLatch;
CountDownLatch newEnabledLatch;
do
{
enabledLatch = m_enabledLatchRef.get();
- enabledLatch.await();
+ boolean waited = false;
+ boolean interrupted = false;
+ while ( !waited )
+ {
+ try
+ {
+ enabledLatch.await();
+ waited = true;
+ }
+ catch ( InterruptedException e )
+ {
+ interrupted = true;
+ }
+ }
+ if ( interrupted )
+ {
+ Thread.currentThread().interrupt();
+ }
newEnabledLatch = new CountDownLatch(1);
}
while ( !m_enabledLatchRef.compareAndSet( enabledLatch, newEnabledLatch) );
@@ -466,10 +517,6 @@
}
disableInternal();
}
- catch ( InterruptedException e )
- {
- Thread.currentThread().interrupt();
- }
finally
{
if (!async)
@@ -758,24 +805,32 @@
return;
}
- // Before creating the implementation object, we are going to
- // test if all the mandatory dependencies are satisfied
- if ( !verifyDependencyManagers() )
+ obtainActivationReadLock( "activateInternal" );
+ try
{
- log( LogService.LOG_DEBUG, "Not all dependencies satisfied, cannot activate", null );
- return;
+ // Before creating the implementation object, we are going to
+ // test if all the mandatory dependencies are satisfied
+ if ( !verifyDependencyManagers() )
+ {
+ log( LogService.LOG_DEBUG, "Not all dependencies satisfied, cannot activate", null );
+ return;
+ }
+
+ if ( !registerService() )
+ {
+ //some other thread is activating us, or we got concurrently deactivated.
+ return;
+ }
+
+
+ if ( ( isImmediate() || getComponentMetadata().isFactory() ) )
+ {
+ getServiceInternal();
+ }
}
-
- if ( !registerService() )
+ finally
{
- //some other thread is activating us, or we got concurrently deactivated.
- return;
- }
-
-
- if ( ( isImmediate() || getComponentMetadata().isFactory() ) )
- {
- getServiceInternal();
+ releaseActivationReadLock( "activateInternal" );
}
}
@@ -794,7 +849,15 @@
// catch any problems from deleting the component to prevent the
// component to remain in the deactivating state !
- doDeactivate( reason, disable );
+ obtainActivationReadLock( "deactivateInternal" );
+ try
+ {
+ doDeactivate( reason, disable );
+ }
+ finally
+ {
+ releaseActivationReadLock( "deactivateInternal" );
+ }
if ( isFactory() )
{
clear();
@@ -835,7 +898,7 @@
{
log( LogService.LOG_DEBUG, "Component deactivation occuring on another thread", null );
}
- obtainWriteLock( "AbstractComponentManager.State.doDeactivate.1" );
+ obtainStateLock( "AbstractComponentManager.State.doDeactivate.1" );
try
{
if ( m_activated )
@@ -852,7 +915,7 @@
}
finally
{
- releaseWriteLock( "AbstractComponentManager.State.doDeactivate.1" );
+ releaseStateLock( "AbstractComponentManager.State.doDeactivate.1" );
}
}
catch ( Throwable t )
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
index 4f09836..df9077a 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
@@ -118,7 +118,7 @@
// also be overwritten
protected boolean createComponent()
{
- if ( !isWriteLocked() )
+ if ( !isStateLocked() )
{
throw new IllegalStateException( "need write lock (createComponent)" );
}
@@ -160,7 +160,7 @@
protected void deleteComponent( int reason )
{
- if ( !isWriteLocked() )
+ if ( !isStateLocked() )
{
throw new IllegalStateException( "need write lock (deleteComponent)" );
}
@@ -574,34 +574,11 @@
// reactivate the component to ensure it is provided with the
// configuration data
- if ( ( getState() & ( STATE_DISPOSED | STATE_DISABLED ) ) != 0 )
+ if ( m_disposed || !m_internalEnabled )
{
// nothing to do for inactive components, leave this method
- log( LogService.LOG_DEBUG, "Component can not be configured in state {0}", new Object[] { getState() }, null );
- //m_internalEnabled is false, we don't need to worry about activation
- updateTargets( getProperties() );
- return;
- }
-
- //TODO wait for activation/deactivation to complete, then lock(?) or internal disable...
-
- // unsatisfied component and non-ignored configuration may change targets
- // to satisfy references
- if ( getState() == STATE_UNSATISFIED
- && !getComponentMetadata().isConfigurationIgnored() )
- {
- log( LogService.LOG_DEBUG, "Attempting to activate unsatisfied component", null );
- //do not allow activation before all targets are reset
- m_internalEnabled = false;
- try
- {
- updateTargets( getProperties() );
- }
- finally
- {
- m_internalEnabled = true;
- }
- activateInternal( getTrackingCount().get() );
+ 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;
}
@@ -615,33 +592,51 @@
//when a configuration arrives the properties will get set based on the new configuration.
return;
}
- if ( !modify() )
- {
- // 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
- deactivateInternal( reason, false, getTrackingCount().get() );
- //do not allow reactivation before all targets are reset
- m_internalEnabled = false;
- try
+ // 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;
}
- finally
+
+ if ( !modify() )
{
- m_internalEnabled = true;
+ // 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, getTrackingCount().get() );
+ obtainActivationWriteLock( "reconfigure.deactivate.activate" );
+ try
+ {
+ updateTargets( getProperties() );
+ }
+ finally
+ {
+ releaseActivationWriteeLock( "reconfigure.deactivate.activate" );;
+ }
+ activateInternal( getTrackingCount().get() );
}
- activateInternal( getTrackingCount().get() );
}
- }
- catch ( InterruptedException e )
- {
- Thread.currentThread().interrupt();
+ finally
+ {
+ //used if modify succeeds or if there's an exception.
+ releaseActivationWriteeLock( "reconfigure.end" );;
+ }
}
finally
{
@@ -680,7 +675,7 @@
// 4. call method (nothing to do when failed, since it has already been logged)
// (call with non-null default result to continue even if the
// modify method call failed)
- obtainWriteLock( "ImmediateComponentManager.modify" );
+ obtainStateLock( "ImmediateComponentManager.modify" );
try
{
//cf 112.5.12 where invoking modified method before updating target services is specified.
@@ -720,7 +715,7 @@
}
finally
{
- releaseWriteLock( "ImmediateComponentManager.modify" );
+ releaseStateLock( "ImmediateComponentManager.modify" );
}
}
@@ -818,7 +813,7 @@
null );
success = false;
}
- obtainWriteLock( "ImmediateComponentManager.getService.1" );
+ obtainStateLock( "ImmediateComponentManager.getService.1" );
try
{
if ( m_componentContext == null )
@@ -837,7 +832,7 @@
}
finally
{
- releaseWriteLock( "ImmediateComponentManager.getService.1" );
+ releaseStateLock( "ImmediateComponentManager.getService.1" );
}
}
return success;
@@ -896,7 +891,7 @@
// be kept (FELIX-3039)
if ( useCount == 0 && !isImmediate() && !keepInstances() )
{
- obtainWriteLock( "ImmediateComponentManager.ungetService.1" );
+ obtainStateLock( "ImmediateComponentManager.ungetService.1" );
try
{
if ( m_useCount.get() == 0 )
@@ -907,7 +902,7 @@
}
finally
{
- releaseWriteLock( "ImmediateComponentManager.ungetService.1" );
+ releaseStateLock( "ImmediateComponentManager.ungetService.1" );
}
}
}
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java
index da43d9c..4658333 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java
@@ -78,7 +78,7 @@
*/
protected void deleteComponent( int reason )
{
- if ( !isWriteLocked() )
+ if ( !isStateLocked() )
{
throw new IllegalStateException( "need write lock (deleteComponent)" );
}