FELIX-3645 Finish unlocking around all calls to register service and obtain dependent services. Some cleanup.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1381408 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 e7575d3..51ef594 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
@@ -91,7 +91,9 @@
// The dependency managers that manage every dependency
private final List m_dependencyManagers;
- //<Map<DependencyManager, Map<ServiceReference, Object[]>>>
+ private boolean m_dependencyManagersInitialized;
+
+ //<Map<DependencyManager, Map<ServiceReference, RefPair>>>
private final AtomicReferenceWrapper m_dependencies_map;
// A reference to the BundleComponentActivator
@@ -216,15 +218,14 @@
}
}
- final void escalateLock( String source )
+ final void obtainWriteLock( String source )
{
- lockingActivity.add( "escalateLock from: " + source + " readLocks: " + m_stateLock.getReadHoldCount() + " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread() + " time: " + System.currentTimeMillis());
- m_stateLock.unlockReadLock();
+ lockingActivity.add( "obtainWriteLock from: " + source + " readLocks: " + m_stateLock.getReadHoldCount() + " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread() + " time: " + System.currentTimeMillis());
try
{
if (!m_stateLock.tryWriteLock( m_timeout ) )
{
- lockingActivity.add( "escalateLock failure from: " + source + " readLocks: " + m_stateLock.getReadHoldCount() + " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread() + " time: " + System.currentTimeMillis() + " Could not obtain write lock.");
+ lockingActivity.add( "obtainWriteLock failure from: " + source + " readLocks: " + m_stateLock.getReadHoldCount() + " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread() + " time: " + System.currentTimeMillis() + " Could not obtain write lock.");
throw new IllegalStateException( "Could not obtain lock" );
}
lockingThread = Thread.currentThread();
@@ -694,24 +695,33 @@
protected void registerService( String[] provides )
{
- ServiceRegistration existing = ( ServiceRegistration ) m_serviceRegistration.get();
- if ( existing == null )
+ releaseReadLock( "register.service.1" );
+ try
{
- log( LogService.LOG_DEBUG, "registering services", null );
-
- // get a copy of the component properties as service properties
- final Dictionary serviceProperties = getServiceProperties();
-
- ServiceRegistration newRegistration = getActivator().getBundleContext().registerService(
- provides,
- getService(), serviceProperties );
- boolean weWon = m_serviceRegistration.compareAndSet( existing, newRegistration );
- if (weWon)
+ ServiceRegistration existing = ( ServiceRegistration ) m_serviceRegistration.get();
+ if ( existing == null )
{
- return;
+ log( LogService.LOG_DEBUG, "registering services", null );
+
+ // get a copy of the component properties as service properties
+ final Dictionary serviceProperties = getServiceProperties();
+
+ ServiceRegistration newRegistration = getActivator().getBundleContext().registerService(
+ provides,
+ getService(), serviceProperties );
+ boolean weWon = m_serviceRegistration.compareAndSet( existing, newRegistration );
+ if (weWon)
+ {
+ return;
+ }
+ newRegistration.unregister();
}
- newRegistration.unregister();
}
+ finally
+ {
+ obtainReadLock( "register.service.1" );
+ }
+
}
/**
@@ -738,15 +748,13 @@
}
}
- protected boolean collectDependencies()
+ boolean initDependencyManagers()
{
- Map old = ( Map ) m_dependencies_map.get();
- if ( old != null)
+ if ( m_dependencyManagersInitialized )
{
- log( LogService.LOG_DEBUG, "dependency map already present, do not collect dependencies", null );
- return false;
+ return true;
}
- Class implementationObjectClass = null;
+ Class implementationObjectClass;
try
{
implementationObjectClass = getActivator().getBundleContext().getBundle().loadClass(
@@ -757,19 +765,46 @@
log( LogService.LOG_ERROR, "Could not load implementation object class", e );
return false;
}
- Map newDeps = new HashMap( );//<DependencyManager, Map<ServiceReference, RefPair>
for (Iterator it = m_dependencyManagers.iterator(); it.hasNext(); )
{
DependencyManager dependencyManager = ( DependencyManager ) it.next();
dependencyManager.initBindingMethods( implementationObjectClass );
+ }
+ m_dependencyManagersInitialized = true;
+ return true;
+ }
+
+ /**
+ * Collect and store in m_dependencies_map all the services for dependencies, outside of any locks.
+ * Throwing IllegalStateException on failure to collect all the dependencies is needed so getService can
+ * know to return null.
+ *
+ * @return true if this thread collected the dependencies;
+ * false if some other thread successfully collected the dependencies;
+ * @throws IllegalStateException if some dependency is no longer available.
+ */
+ protected boolean collectDependencies() throws IllegalStateException
+ {
+ Map old = ( Map ) m_dependencies_map.get();
+ if ( old != null)
+ {
+ log( LogService.LOG_DEBUG, "dependency map already present, do not collect dependencies", null );
+ return false;
+ }
+ initDependencyManagers();
+ Map newDeps = new HashMap( );//<DependencyManager, Map<ServiceReference, RefPair>
+ for (Iterator it = m_dependencyManagers.iterator(); it.hasNext(); )
+ {
+ DependencyManager dependencyManager = ( DependencyManager ) it.next();
+
if (!dependencyManager.prebind( newDeps) )
{
//not actually satisfied any longer
returnServices( newDeps );
log( LogService.LOG_DEBUG, "Could not get dependency for dependency manager: {0}",
new Object[] {dependencyManager}, null );
- return false;
+ throw new IllegalStateException( "Missing dependencies, not satisfied" );
}
}
if ( !setDependencyMap( old, newDeps ) )
@@ -871,7 +906,7 @@
m_dependencyManagers.clear();
}
- //<DepeendencyManager, Map<ServiceReference, Object[]>>
+ //<DependencyManager, Map<ServiceReference, RefPair>>
protected Map getParameterMap()
{
return ( Map ) m_dependencies_map.get();
@@ -1246,22 +1281,18 @@
ServiceReference getServiceReference( AbstractComponentManager acm )
{
-// return null;
throw new IllegalStateException("getServiceReference" + this);
}
Object getService( ImmediateComponentManager dcm )
{
-// log( dcm, "getService" );
-// return null;
throw new IllegalStateException("getService" + this);
}
void ungetService( ImmediateComponentManager dcm )
{
-// log( dcm, "ungetService" );
throw new IllegalStateException("ungetService" + this);
}
@@ -1269,7 +1300,6 @@
void enable( AbstractComponentManager acm )
{
log( acm, "enable" );
-// throw new IllegalStateException("enable" + this);
}
@@ -1277,27 +1307,23 @@
{
log( acm, "activate" );
return false;
-// throw new IllegalStateException("activate" + this);
}
void deactivate( AbstractComponentManager acm, int reason )
{
log( acm, "deactivate (reason: " + reason + ")" );
-// throw new IllegalStateException("deactivate" + this);
}
void disable( AbstractComponentManager acm )
{
-// log( acm, "disable" );
throw new IllegalStateException("disable" + this);
}
void dispose( AbstractComponentManager acm, int reason )
{
-// log( acm, "dispose (reason: " + reason + ")" );
throw new IllegalStateException("dispose" + this);
}
@@ -1312,8 +1338,9 @@
{
try
{
+ acm.releaseReadLock( "AbstractComponentManager.State.doDeactivate.1" );
acm.unregisterComponentService();
- acm.escalateLock( "AbstractComponentManager.State.doDeactivate.1" );
+ acm.obtainWriteLock( "AbstractComponentManager.State.doDeactivate.1" );
try
{
acm.deleteComponent( reason );
@@ -1477,13 +1504,24 @@
// 4. Call the activate method, if present
if ( ( acm.isImmediate() || acm.getComponentMetadata().isFactory() ) )
{
+ acm.releaseReadLock( "AbstractComponentManager.Unsatisfied.activate.1" );
//don't collect dependencies for a factory component.
- if ( !acm.collectDependencies() )
- {
- acm.log( LogService.LOG_DEBUG, "Not all dependencies collected, cannot create object", null );
- return false;
- }
- acm.escalateLock( "AbstractComponentManager.Unsatisifed.activate.1" );
+ try
+ {
+ if ( !acm.collectDependencies() )
+ {
+ acm.log( LogService.LOG_DEBUG, "Not all dependencies collected, cannot create object (1)", null );
+ acm.obtainReadLock( "AbstractComponentManager.Unsatisfied.activate.1" );
+ return false;
+ }
+ }
+ catch ( IllegalStateException e )
+ {
+ acm.log( LogService.LOG_DEBUG, "Not all dependencies collected, cannot create object (2)", null );
+ acm.obtainReadLock( "AbstractComponentManager.Unsatisfied.activate.1" );
+ return false;
+ }
+ acm.obtainWriteLock( "AbstractComponentManager.Unsatisfied.activate.1" );
try
{
acm.changeState( acm.getActiveState() );
@@ -1496,7 +1534,7 @@
}
finally
{
- acm.deescalateLock( "AbstractComponentManager.Unsatisifed.activate.1" );
+ acm.deescalateLock( "AbstractComponentManager.Unsatisfied.activate.1" );
}
}
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 df72c9a..f9c2499 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
@@ -59,18 +59,12 @@
private static final int STATE_MASK = //Component.STATE_UNSATISFIED |
Component.STATE_ACTIVE | Component.STATE_REGISTERED | Component.STATE_FACTORY;
- // pseudo service to mark a bound service without actual service instance
-// private static final Object BOUND_SERVICE_SENTINEL = new Object();
-
// the component to which this dependency belongs
private final AbstractComponentManager m_componentManager;
// Reference to the metadata
private final ReferenceMetadata m_dependencyMetadata;
- // The map of bound services indexed by their ServiceReference
-// private final Map m_bound;
-
// the number of matching services registered in the system
private volatile int m_size;
@@ -102,8 +96,6 @@
{
m_componentManager = componentManager;
m_dependencyMetadata = dependency;
-// m_bound = Collections.synchronizedMap( new HashMap() );
-
// dump the reference information if DEBUG is enabled
if ( m_componentManager.isLogEnabled( LogService.LOG_DEBUG ) )
@@ -321,8 +313,8 @@
return;
}
//release our read lock and wait for activation to complete
- m_componentManager.escalateLock( "DependencyManager.serviceAdded.nothandled.1" );
- m_componentManager.deescalateLock( "DependencyManager.serviceAdded.nothandled.2" );
+ m_componentManager.releaseReadLock( "DependencyManager.serviceAdded.nothandled.1" );
+ m_componentManager.obtainReadLock( "DependencyManager.serviceAdded.nothandled.2" );
m_componentManager.log( LogService.LOG_DEBUG,
"Dependency Manager: Service {0} activation on other thread: after releasing lock, component instance is: {1}", new Object[]
{m_dependencyMetadata.getName(), m_componentManager.getInstance()}, null );
@@ -613,12 +605,15 @@
void deactivate()
{
// unget all services we once got
- ServiceReference[] boundRefs = getBoundServiceReferences();
- if ( boundRefs != null )
+ if ( m_componentManager.getDependencyMap() != null )
{
- for ( int i = 0; i < boundRefs.length; i++ )
+ ServiceReference[] boundRefs = getBoundServiceReferences();
+ if ( boundRefs != null )
{
- ungetService( boundRefs[i] );
+ for ( int i = 0; i < boundRefs.length; i++ )
+ {
+ ungetService( boundRefs[i] );
+ }
}
}
}
@@ -807,32 +802,17 @@
*/
private boolean isBound()
{
- Map bound = ( Map ) m_componentManager.getDependencyMap().get( this );
+ Map dependencyMap = m_componentManager.getDependencyMap();
+ if (dependencyMap == null )
+ {
+ return false;
+ }
+ Map bound = ( Map ) dependencyMap.get( this );
return !bound.isEmpty();
}
/**
- * Adds the {@link #BOUND_SERVICE_SENTINEL} object as a pseudo service to
- * the map of bound services. This method allows keeping track of services
- * which have been bound but not retrieved from the service registry, which
- * is the case if the bind method is called with a ServiceReference instead
- * of the service object itself.
- * <p>
- * We have to keep track of all services for which we called the bind
- * method to be able to call the unbind method in case the service is
- * unregistered.
- *
- * @param serviceReference The reference to the service being marked as
- * bound.
- */
-// private void bindService( ServiceReference serviceReference )
-// {
-// m_bound.put( serviceReference, BOUND_SERVICE_SENTINEL );
-// }
-
-
- /**
* Returns the RefPair containing the given service reference and the bound service
* or <code>null</code> if this is instance is not currently bound to that
* service.
@@ -890,7 +870,7 @@
return null;
}
- // keep the service for latter ungetting
+ // keep the service for later ungetting
if ( serviceObject != null )
{
if (refPair != null)
@@ -989,7 +969,6 @@
boolean open( Object instance, Map parameters )
{
-// initBindingMethods( instance.getClass() );
m_componentInstance = instance;
return bind(parameters);
}
@@ -1008,9 +987,6 @@
finally
{
m_componentInstance = null;
-// m_bind = null;
-// m_unbind = null;
-// m_updated = null;
}
}
@@ -1172,16 +1148,34 @@
private boolean invokeBindMethod( ServiceReference ref )
{
//event driven, and we already checked this ref is not yet handled.
- Map dependencyMap = m_componentManager.getDependencyMap();
- if ( dependencyMap != null )
+ if ( m_componentInstance != null )
{
- Map deps = ( Map ) dependencyMap.get( this );
- BundleContext bundleContext = m_componentManager.getActivator().getBundleContext();
- AbstractComponentManager.RefPair refPair = m_bind.getServiceObject( ref, bundleContext );
- deps.put( ref, refPair );
- return invokeBindMethod( refPair );
+ Map dependencyMap = m_componentManager.getDependencyMap();
+ if ( dependencyMap != null )
+ {
+ if (m_bind == null)
+ {
+ m_componentManager.log( LogService.LOG_ERROR,
+ "For dependency {0}, bind method not set: component state {1}",
+ new Object[]
+ { new Integer(m_componentManager.getState()) }, null );
+
+ }
+ Map deps = ( Map ) dependencyMap.get( this );
+ BundleContext bundleContext = m_componentManager.getActivator().getBundleContext();
+ AbstractComponentManager.RefPair refPair = m_bind.getServiceObject( ref, bundleContext );
+ deps.put( ref, refPair );
+ return invokeBindMethod( refPair );
+ }
+ return false;
}
- return false;
+ else
+ {
+ m_componentManager.log( LogService.LOG_DEBUG,
+ "DependencyManager : component not yet created, assuming bind method call succeeded",
+ null );
+ return true;
+ }
}
/**
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 549b442..7a0c438 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
@@ -615,20 +615,29 @@
public Object getService( Bundle bundle, ServiceRegistration serviceRegistration )
{
- final boolean release = obtainReadLock( "ImmediateComponentManager.getService.1" );
+ boolean release = obtainReadLock( "ImmediateComponentManager.getService.1" );
try
{
if ( m_useCount == 0 )
{
- if ( !collectDependencies() )
+ releaseReadLock( "ImmediateComponentManager.getService.1" );
+ try
{
- log(
- LogService.LOG_INFO,
- "getService did not win collecting dependencies, try creating object anyway.",
- null );
+ if ( !collectDependencies() )
+ {
+ log(
+ LogService.LOG_INFO,
+ "getService did not win collecting dependencies, try creating object anyway.",
+ null );
+ }
}
- escalateLock( "ImmediateComponentManager.getService.1" );
+ catch ( IllegalStateException e )
+ {
+ release = false;
+ return null;
+ }
+ obtainWriteLock( "ImmediateComponentManager.getService.1" );
try
{
if ( m_useCount == 0 )
@@ -675,7 +684,8 @@
// be kept (FELIX-3039)
if ( m_useCount == 0 && !isImmediate() && !getActivator().getConfiguration().keepInstances() )
{
- escalateLock( "ImmediateComponentManager.ungetService.1" );
+ releaseReadLock( "ImmediateComponentManager.ungetService.1" );
+ obtainWriteLock( "ImmediateComponentManager.ungetService.1" );
try
{
if ( m_useCount == 0 )