Author: djencks
Date: Mon Sep 16 07:39:34 2013
New Revision: 1523553
URL: http://svn.apache.org/r1523553
Log:
FELIX-4223 Use read-write lock to prevent activation/deactivation during config
update
Modified:
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/ImmediateComponentManager.java
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java
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=1523553&r1=1523552&r2=1523553&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
Mon Sep 16 07:39:34 2013
@@ -37,6 +37,7 @@ import java.util.concurrent.atomic.Atomi
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 @@ public abstract class AbstractComponentM
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 @@ public abstract class AbstractComponentM
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 @@ public abstract class AbstractComponentM
}
}
- 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 @@ public abstract class AbstractComponentM
{
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 @@ public abstract class AbstractComponentM
Thread.currentThread().interrupt();
}
}
+
+ final void obtainActivationReadLock( String source )
+ {
+ obtainLock( m_activationLock.readLock(), source);
+ }
- long getLockTimeout()
+ final void releaseActivationReadLock( String source )
{
- BundleComponentActivator activator = getActivator();
- if ( activator != null )
+ m_activationLock.readLock().unlock();
+ }
+
+ final void obtainActivationWriteLock( String source )
+ {
+ obtainLock( m_activationLock.writeLock(), source);
+ }
+
+ final void releaseActivationWriteeLock( String source )
+ {
+ if ( m_activationLock.getWriteHoldCount() > 0 )
{
- return activator.getConfiguration().lockTimeout();
+ m_activationLock.writeLock().unlock();
}
- return ScrConfiguration.DEFAULT_LOCK_TIMEOUT_MILLISECONDS;
+ }
+
+ final void obtainStateLock( String source )
+ {
+ obtainLock( m_stateLock, source );
}
- final void releaseWriteLock( String 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 @@ public abstract class AbstractComponentM
activateInternal( m_trackingCount.get() );
}
}
- catch ( InterruptedException e )
- {
- Thread.currentThread().interrupt();
- }
finally
{
if ( !async )
@@ -424,14 +451,38 @@ public abstract class AbstractComponentM
}
}
- 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 @@ public abstract class AbstractComponentM
}
disableInternal();
}
- catch ( InterruptedException e )
- {
- Thread.currentThread().interrupt();
- }
finally
{
if (!async)
@@ -758,24 +805,32 @@ public abstract class AbstractComponentM
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 ( !registerService() )
+ {
+ //some other thread is activating us, or we got concurrently
deactivated.
+ return;
+ }
- if ( ( isImmediate() || getComponentMetadata().isFactory() ) )
+ if ( ( isImmediate() || getComponentMetadata().isFactory() ) )
+ {
+ getServiceInternal();
+ }
+ }
+ finally
{
- getServiceInternal();
+ releaseActivationReadLock( "activateInternal" );
}
}
@@ -794,7 +849,15 @@ public abstract class AbstractComponentM
// 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 @@ public abstract class AbstractComponentM
{
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 @@ public abstract class AbstractComponentM
}
finally
{
- releaseWriteLock(
"AbstractComponentManager.State.doDeactivate.1" );
+ releaseStateLock(
"AbstractComponentManager.State.doDeactivate.1" );
}
}
catch ( Throwable t )
Modified:
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
URL:
http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java?rev=1523553&r1=1523552&r2=1523553&view=diff
==============================================================================
---
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
(original)
+++
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
Mon Sep 16 07:39:34 2013
@@ -118,7 +118,7 @@ public class ImmediateComponentManager<S
// also be overwritten
protected boolean createComponent()
{
- if ( !isWriteLocked() )
+ if ( !isStateLocked() )
{
throw new IllegalStateException( "need write lock
(createComponent)" );
}
@@ -160,7 +160,7 @@ public class ImmediateComponentManager<S
protected void deleteComponent( int reason )
{
- if ( !isWriteLocked() )
+ if ( !isStateLocked() )
{
throw new IllegalStateException( "need write lock
(deleteComponent)" );
}
@@ -574,34 +574,11 @@ public class ImmediateComponentManager<S
// 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 @@ public class ImmediateComponentManager<S
//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
- deactivateInternal( reason, false, getTrackingCount().get() );
- //do not allow reactivation before all targets are reset
- m_internalEnabled = false;
- 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 @@ public class ImmediateComponentManager<S
// 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 @@ public class ImmediateComponentManager<S
}
finally
{
- releaseWriteLock( "ImmediateComponentManager.modify" );
+ releaseStateLock( "ImmediateComponentManager.modify" );
}
}
@@ -818,7 +813,7 @@ public class ImmediateComponentManager<S
null );
success = false;
}
- obtainWriteLock( "ImmediateComponentManager.getService.1" );
+ obtainStateLock( "ImmediateComponentManager.getService.1" );
try
{
if ( m_componentContext == null )
@@ -837,7 +832,7 @@ public class ImmediateComponentManager<S
}
finally
{
- releaseWriteLock( "ImmediateComponentManager.getService.1"
);
+ releaseStateLock( "ImmediateComponentManager.getService.1"
);
}
}
return success;
@@ -896,7 +891,7 @@ public class ImmediateComponentManager<S
// 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 @@ public class ImmediateComponentManager<S
}
finally
{
- releaseWriteLock(
"ImmediateComponentManager.ungetService.1" );
+ releaseStateLock(
"ImmediateComponentManager.ungetService.1" );
}
}
}
Modified:
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java
URL:
http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java?rev=1523553&r1=1523552&r2=1523553&view=diff
==============================================================================
---
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java
(original)
+++
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java
Mon Sep 16 07:39:34 2013
@@ -78,7 +78,7 @@ public class ServiceFactoryComponentMana
*/
protected void deleteComponent( int reason )
{
- if ( !isWriteLocked() )
+ if ( !isStateLocked() )
{
throw new IllegalStateException( "need write lock
(deleteComponent)" );
}
