FELIX-3456 only obtain read lock once to avoid problems with reentrancy
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1371285 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java b/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
index 701303c..130fd31 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
@@ -188,14 +188,17 @@
if ( pid.equals( getComponentMetadata().getConfigurationPid() ) )
{
// singleton configuration deleted
- m_singleComponent.obtainReadLock();
+ final boolean release = m_singleComponent.obtainReadLock();
try
{
m_singleComponent.reconfigure( null );
}
finally
{
- m_singleComponent.releaseReadLock();
+ if ( release )
+ {
+ m_singleComponent.releaseReadLock();
+ }
}
}
else
@@ -205,7 +208,7 @@
if ( icm != null )
{
boolean dispose = true;
- icm.obtainReadLock();
+ final boolean release = icm.obtainReadLock();
try
{
// special casing if the single component is deconfigured
@@ -242,7 +245,10 @@
}
finally
{
- icm.releaseReadLock();
+ if ( release )
+ {
+ icm.releaseReadLock();
+ }
}
}
}
@@ -271,7 +277,7 @@
if ( pid.equals( getComponentMetadata().getConfigurationPid() ) )
{
- m_singleComponent.obtainReadLock();
+ final boolean release = m_singleComponent.obtainReadLock();
try
{
// singleton configuration has pid equal to component name
@@ -279,7 +285,10 @@
}
finally
{
- m_singleComponent.releaseReadLock();
+ if ( release )
+ {
+ m_singleComponent.releaseReadLock();
+ }
}
}
else
@@ -288,7 +297,7 @@
final ImmediateComponentManager icm = getComponentManager( pid );
if ( icm != null )
{
- icm.obtainReadLock();
+ final boolean release = icm.obtainReadLock();
try
{
// factory configuration updated for existing component instance
@@ -296,7 +305,10 @@
}
finally
{
- icm.releaseReadLock();
+ if ( release )
+ {
+ icm.releaseReadLock();
+ }
}
}
else
@@ -315,14 +327,17 @@
}
// configure the component
- newIcm.obtainReadLock();
+ final boolean release = newIcm.obtainReadLock();
try
{
newIcm.reconfigure( props );
}
finally
{
- newIcm.releaseReadLock();
+ if ( release )
+ {
+ newIcm.releaseReadLock();
+ }
}
// enable the component if it is initially enabled
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 85f7b32..a1c3d02 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
@@ -149,9 +149,14 @@
}
//ImmediateComponentHolder should be in this manager package and this should be default access.
- public final void obtainReadLock()
+ public final boolean obtainReadLock()
{
// new Exception("Stack trace obtainReadLock").printStackTrace();
+ if (m_stateLock.getReadHoldCount() >0)
+ {
+ return false;
+// throw new IllegalStateException( "nested read locks" );
+ }
try
{
if (!m_stateLock.tryReadLock( m_timeout ) )
@@ -164,6 +169,7 @@
//TODO this is so wrong
throw new IllegalStateException( "Could not obtain lock (Reason: " + e + ")" );
}
+ return true;
}
@@ -256,7 +262,7 @@
public final void enable( final boolean async )
{
- obtainReadLock();
+ final boolean release = obtainReadLock();
try
{
enableInternal();
@@ -267,7 +273,10 @@
}
finally
{
- releaseReadLock();
+ if ( release )
+ {
+ releaseReadLock();
+ }
}
if ( async )
@@ -276,14 +285,17 @@
{
public void run()
{
- obtainReadLock();
+ final boolean release = obtainReadLock();
try
{
activateInternal();
}
finally
{
- releaseReadLock();
+ if ( release )
+ {
+ releaseReadLock();
+ }
}
}
} );
@@ -304,7 +316,7 @@
public final void disable( final boolean async )
{
- obtainReadLock();
+ final boolean release = obtainReadLock();
try
{
if ( !async )
@@ -315,7 +327,10 @@
}
finally
{
- releaseReadLock();
+ if ( release )
+ {
+ releaseReadLock();
+ }
}
if ( async )
@@ -324,14 +339,17 @@
{
public void run()
{
- obtainReadLock();
+ final boolean release = obtainReadLock();
try
{
deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED );
}
finally
{
- releaseReadLock();
+ if ( release )
+ {
+ releaseReadLock();
+ }
}
}
} );
@@ -362,14 +380,17 @@
*/
public void dispose( int reason )
{
- obtainReadLock();
+ final boolean release = obtainReadLock();
try
{
disposeInternal( reason );
}
finally
{
- releaseReadLock();
+ if ( release )
+ {
+ releaseReadLock();
+ }
}
}
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
index 3a4476f..cb21db5 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
@@ -94,7 +94,7 @@
final ImmediateComponentManager cm = createComponentManager();
ComponentInstance instance;
- cm.obtainReadLock();
+ final boolean release = cm.obtainReadLock();
try
{
cm.setFactoryProperties( dictionary );
@@ -108,13 +108,16 @@
if ( instance == null )
{
// activation failed, clean up component manager
- cm.dispose();
+ cm.disposeInternal( ComponentConstants.DEACTIVATION_REASON_DISPOSED );
throw new ComponentException( "Failed activating component" );
}
}
finally
{
- cm.releaseReadLock();
+ if ( release )
+ {
+ cm.releaseReadLock();
+ }
}
synchronized ( m_componentInstances )
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
index 3a4be5c..e698544 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
@@ -139,7 +139,7 @@
}
else //non-spec backwards compatible
{
- obtainReadLock();
+ final boolean release = obtainReadLock();
try
{
ImmediateComponentManager cm;
@@ -178,20 +178,26 @@
{
// update the configuration as if called as ManagedService
//TODO deadlock potential, we are holding our own state lock.
- cm.obtainReadLock();
+ final boolean releaseInner = cm.obtainReadLock();
try
{
cm.reconfigure( configuration );
}
finally
{
- cm.releaseReadLock();
+ if ( releaseInner )
+ {
+ cm.releaseReadLock();
+ }
}
}
}
finally
{
- releaseReadLock();
+ if ( release )
+ {
+ releaseReadLock();
+ }
}
}
}
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 eab5cb2..578ff1c 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
@@ -155,7 +155,7 @@
final ServiceReference ref = event.getServiceReference();
final String serviceString = "Service " + m_dependencyMetadata.getInterface() + "/"
+ ref.getProperty( Constants.SERVICE_ID );
- m_componentManager.obtainReadLock();
+ final boolean release = m_componentManager.obtainReadLock();
try
{
switch ( event.getType() )
@@ -258,7 +258,10 @@
}
finally
{
- m_componentManager.releaseReadLock();
+ if ( release )
+ {
+ m_componentManager.releaseReadLock();
+ }
}
}
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 d921028..5e5b0f3 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
@@ -617,7 +617,7 @@
public Object getService( Bundle bundle, ServiceRegistration serviceRegistration )
{
- obtainReadLock();
+ final boolean release = obtainReadLock();
try
{
if ( m_useCount == 0 )
@@ -641,13 +641,16 @@
}
finally
{
- releaseReadLock();
+ if ( release )
+ {
+ releaseReadLock();
+ }
}
}
public void ungetService( Bundle bundle, ServiceRegistration serviceRegistration, Object o )
{
- obtainReadLock();
+ final boolean release = obtainReadLock();
try
{
// the framework should not call ungetService more than it calls
@@ -678,7 +681,10 @@
}
finally
{
- releaseReadLock();
+ if ( release )
+ {
+ releaseReadLock();
+ }
}
}
}
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 735e4c3..4b324fb 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
@@ -95,7 +95,7 @@
// When the getServiceMethod is called, the implementation object must be created
- obtainReadLock();
+ final boolean release = obtainReadLock();
try
{
// private ComponentContext and implementation instances
@@ -126,7 +126,10 @@
}
finally
{
- releaseReadLock();
+ if ( release )
+ {
+ releaseReadLock();
+ }
}
}
@@ -139,7 +142,7 @@
log( LogService.LOG_DEBUG, "ServiceFactory.ungetService()", null );
// When the ungetServiceMethod is called, the implementation object must be deactivated
- obtainReadLock();
+ final boolean release = obtainReadLock();
try
{
// private ComponentContext and implementation instances
@@ -156,7 +159,10 @@
}
finally
{
- releaseReadLock();
+ if ( release )
+ {
+ releaseReadLock();
+ }
}
}