You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by fm...@apache.org on 2012/05/31 15:06:31 UTC
svn commit: r1344701 - in /felix/trunk/scr: ./
src/main/appended-resources/META-INF/
src/main/java/org/apache/felix/scr/impl/
src/main/java/org/apache/felix/scr/impl/config/
src/main/java/org/apache/felix/scr/impl/manager/
src/test/java/org/apache/feli...
Author: fmeschbe
Date: Thu May 31 13:06:30 2012
New Revision: 1344701
URL: http://svn.apache.org/viewvc?rev=1344701&view=rev
Log:
FELIX-3456 Apply final patch #7 introducing locking
instead of Java synchronized to prevent deadlocks
Modified:
felix/trunk/scr/pom.xml
felix/trunk/scr/src/main/appended-resources/META-INF/DEPENDENCIES
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/Activator.java
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
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/ComponentFactoryImpl.java
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DelayedComponentManager.java
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.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
felix/trunk/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java
Modified: felix/trunk/scr/pom.xml
URL: http://svn.apache.org/viewvc/felix/trunk/scr/pom.xml?rev=1344701&r1=1344700&r2=1344701&view=diff
==============================================================================
--- felix/trunk/scr/pom.xml (original)
+++ felix/trunk/scr/pom.xml Thu May 31 13:06:30 2012
@@ -110,6 +110,12 @@
<version>2.2.2</version>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>concurrent</groupId>
+ <artifactId>concurrent</artifactId>
+ <version>1.3.4</version>
+ <scope>provided</scope>
+ </dependency>
<!-- Integration Testing with Pax Exam -->
<dependency>
@@ -297,7 +303,8 @@
org.osgi.service.metatype;version="[1.1,2)"
</DynamicImport-Package>
<Embed-Dependency>
- kxml2;inline=org/kxml2/io/KXmlParser.class|org/xmlpull/v1/XmlPull**
+ kxml2;inline=org/kxml2/io/KXmlParser.class|org/xmlpull/v1/XmlPull**,
+ concurrent;inline=EDU/oswego/cs/dl/util/concurrent/ReentrantLock.class|EDU/oswego/cs/dl/util/concurrent/Sync.class
</Embed-Dependency>
</instructions>
</configuration>
Modified: felix/trunk/scr/src/main/appended-resources/META-INF/DEPENDENCIES
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/appended-resources/META-INF/DEPENDENCIES?rev=1344701&r1=1344700&r2=1344701&view=diff
==============================================================================
--- felix/trunk/scr/src/main/appended-resources/META-INF/DEPENDENCIES (original)
+++ felix/trunk/scr/src/main/appended-resources/META-INF/DEPENDENCIES Thu May 31 13:06:30 2012
@@ -13,6 +13,11 @@ This product includes software from http
Copyright (c) 2002,2003, Stefan Haustein, Oberhausen, Rhld., Germany.
Licensed under BSD License.
+This product includes software from http://kxml.sourceforge.net.
+http://gee.cs.oswego.edu/dl/classes/EDU/oswego/cs/dl/util/concurrent/intro.html
+"released to the public domain and may be used for any purpose whatsoever
+without permission or acknowledgment" by Doug Lea.
+
II. Used Software
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/Activator.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/Activator.java?rev=1344701&r1=1344700&r2=1344701&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/Activator.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/Activator.java Thu May 31 13:06:30 2012
@@ -220,7 +220,7 @@ public class Activator implements Bundle
* <i>Service-Component</i> header, this method has no effect. The
* fragments of a bundle are not checked for the header (112.4.1).
* <p>
- * This method calls the {@link #getBundleContext(Bundle)} method to find
+ * This method calls the {@link Bundle#getBundleContext()} method to find
* the <code>BundleContext</code> of the bundle. If the context cannot be
* found, this method does not load components for the bundle.
*/
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java?rev=1344701&r1=1344700&r2=1344701&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java Thu May 31 13:06:30 2012
@@ -238,7 +238,7 @@ public class BundleComponentActivator im
// enable the component
if ( metadata.isEnabled() )
{
- holder.enableComponents();
+ holder.enableComponents( false );
}
}
catch ( Throwable t )
@@ -365,10 +365,8 @@ public class BundleComponentActivator im
/**
* Implements the <code>ComponentContext.enableComponent(String)</code>
* method by first finding the component(s) for the <code>name</code> and
- * then starting a thread to actually enable all components found.
+ * enabling them. The enable method will schedule activation.
* <p>
- * If no component matching the given name is found the thread is not
- * started and the method does nothing.
*
* @param name The name of the component to enable or <code>null</code> to
* enable all components.
@@ -381,45 +379,26 @@ public class BundleComponentActivator im
return;
}
- // FELIX-2368; schedule for asynchronous enablement. According to
- // 112.5.1 the enabled state should be changed immediately but
- // the component(s) should be activated asynchronously. Since
- // we do not really handle the enabled state separately we
- // schedule enablement and activation for asynchronous execution.
- schedule( new Runnable()
+ for ( int i = 0; i < holder.length; i++ )
{
- public void run()
+ try
{
- for ( int i = 0; i < holder.length; i++ )
- {
- try
- {
- log( LogService.LOG_DEBUG, "Enabling Component", holder[i].getComponentMetadata(), null );
- holder[i].enableComponents();
- }
- catch ( Throwable t )
- {
- log( LogService.LOG_ERROR, "Cannot enable component", holder[i].getComponentMetadata(), t );
- }
- }
+ log( LogService.LOG_DEBUG, "Enabling Component", holder[i].getComponentMetadata(), null );
+ holder[i].enableComponents( true );
}
-
-
- public String toString()
+ catch ( Throwable t )
{
- return "enableComponent(" + name + ")";
+ log( LogService.LOG_ERROR, "Cannot enable component", holder[i].getComponentMetadata(), t );
}
- } );
+ }
}
/**
* Implements the <code>ComponentContext.disableComponent(String)</code>
* method by first finding the component(s) for the <code>name</code> and
- * then starting a thread to actually disable all components found.
+ * disabling them. The disable method will schedule deactivation
* <p>
- * If no component matching the given name is found the thread is not
- * started and the method does nothing.
*
* @param name The name of the component to disable or <code>null</code> to
* disable all components.
@@ -432,35 +411,18 @@ public class BundleComponentActivator im
return;
}
- // FELIX-2368; schedule for asynchronous enablement. According to
- // 112.5.1 the enabled state should be changed immediately but
- // the component(s) should be deactivated asynchronously. Since
- // we do not really handle the enabled state separately we
- // schedule disablement and deactivation for asynchronous execution.
- schedule( new Runnable()
+ for ( int i = 0; i < holder.length; i++ )
{
- public void run()
+ try
{
- for ( int i = 0; i < holder.length; i++ )
- {
- try
- {
- log( LogService.LOG_DEBUG, "Disabling Component", holder[i].getComponentMetadata(), null );
- holder[i].disableComponents();
- }
- catch ( Throwable t )
- {
- log( LogService.LOG_ERROR, "Cannot disable component", holder[i].getComponentMetadata(), t );
- }
- }
+ log( LogService.LOG_DEBUG, "Disabling Component", holder[i].getComponentMetadata(), null );
+ holder[i].disableComponents( true );
}
-
-
- public String toString()
+ catch ( Throwable t )
{
- return "disableComponent(" + name + ")";
+ log( LogService.LOG_ERROR, "Cannot disable component", holder[i].getComponentMetadata(), t );
}
- } );
+ }
}
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java?rev=1344701&r1=1344700&r2=1344701&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ComponentHolder.java Thu May 31 13:06:30 2012
@@ -76,15 +76,22 @@ public interface ComponentHolder
Component[] getComponents();
/**
- * Enables all components of this holder.
+ * Enables all components of this holder and if satisifed activates
+ * them.
+ *
+ * @param async Whether the actual activation should take place
+ * asynchronously or not.
*/
- void enableComponents();
+ void enableComponents( boolean async );
/**
* Disables all components of this holder.
+ *
+ * @param async Whether the actual deactivation should take place
+ * asynchronously or not.
*/
- void disableComponents();
+ void disableComponents( boolean async );
/**
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java?rev=1344701&r1=1344700&r2=1344701&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurationSupport.java Thu May 31 13:06:30 2012
@@ -226,6 +226,8 @@ public class ConfigurationSupport implem
catch (IllegalStateException ise)
{
// If the bundle has been stopped conurrently
+ Activator.log(LogService.LOG_WARNING, null, "Unknown ConfigurationEvent type " + event.getType(),
+ ise);
}
}
break;
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java?rev=1344701&r1=1344700&r2=1344701&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java Thu May 31 13:06:30 2012
@@ -188,7 +188,15 @@ public class ImmediateComponentHolder im
if ( pid.equals( getComponentMetadata().getName() ) )
{
// singleton configuration deleted
- m_singleComponent.reconfigure( null );
+ m_singleComponent.obtainStateLock();
+ try
+ {
+ m_singleComponent.reconfigure( null );
+ }
+ finally
+ {
+ m_singleComponent.releaseStateLock();
+ }
}
else
{
@@ -196,36 +204,45 @@ public class ImmediateComponentHolder im
ImmediateComponentManager icm = removeComponentManager( pid );
if ( icm != null )
{
- // special casing if the single component is deconfigured
- if ( m_singleComponent == icm )
+ boolean dispose = true;
+ icm.obtainStateLock();
+ try
{
-
- // if the single component is the last remaining, deconfi
- if ( m_components.isEmpty() )
+// special casing if the single component is deconfigured
+ if ( m_singleComponent == icm )
{
- // if the single component is the last remaining
- // deconfigure it
- icm.reconfigure( null );
- icm = null;
+ // if the single component is the last remaining, deconfi
+ if ( m_components.isEmpty() )
+ {
+
+ // if the single component is the last remaining
+ // deconfigure it
+ icm.reconfigure( null );
+ dispose = false;
+
+ }
+ else
+ {
+
+ // replace the single component field with another
+ // entry from the map
+ m_singleComponent = ( ImmediateComponentManager ) m_components.values().iterator().next();
+ }
}
- else
- {
-
- // replace the single component field with another
- // entry from the map
- m_singleComponent = ( ImmediateComponentManager ) m_components.values().iterator().next();
+ // icm may be null if the last configuration deleted was the
+ // single component's configuration. Otherwise the component
+ // is not the "last" and has to be disposed off
+ if ( dispose )
+ {
+ icm.dispose( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED );
}
}
-
- // icm may be null if the last configuration deleted was the
- // single component's configuration. Otherwise the component
- // is not the "last" and has to be disposed off
- if ( icm != null )
+ finally
{
- icm.dispose( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED );
+ icm.releaseStateLock();
}
}
}
@@ -254,8 +271,16 @@ public class ImmediateComponentHolder im
if ( pid.equals( getComponentMetadata().getName() ) )
{
- // singleton configuration has pid equal to component name
- m_singleComponent.reconfigure( props );
+ m_singleComponent.obtainStateLock();
+ try
+ {
+// singleton configuration has pid equal to component name
+ m_singleComponent.reconfigure( props );
+ }
+ finally
+ {
+ m_singleComponent.releaseStateLock();
+ }
}
else
{
@@ -263,8 +288,16 @@ public class ImmediateComponentHolder im
final ImmediateComponentManager icm = getComponentManager( pid );
if ( icm != null )
{
- // factory configuration updated for existing component instance
- icm.reconfigure( props );
+ icm.obtainStateLock();
+ try
+ {
+ // factory configuration updated for existing component instance
+ icm.reconfigure( props );
+ }
+ finally
+ {
+ icm.releaseStateLock();
+ }
}
else
{
@@ -282,12 +315,20 @@ public class ImmediateComponentHolder im
}
// configure the component
- newIcm.reconfigure( props );
+ newIcm.obtainStateLock();
+ try
+ {
+ newIcm.reconfigure( props );
+ }
+ finally
+ {
+ newIcm.releaseStateLock();
+ }
// enable the component if it is initially enabled
if ( m_enabled && getComponentMetadata().isEnabled() )
{
- newIcm.enable();
+ newIcm.enable( false );
}
// store the component in the map
@@ -305,18 +346,18 @@ public class ImmediateComponentHolder im
}
- public void enableComponents()
+ public void enableComponents( final boolean async )
{
final ImmediateComponentManager[] cms = getComponentManagers( false );
if ( cms == null )
{
- m_singleComponent.enable();
+ m_singleComponent.enable( async );
}
else
{
for ( int i = 0; i < cms.length; i++ )
{
- cms[i].enable();
+ cms[i].enable( async );
}
}
@@ -324,20 +365,20 @@ public class ImmediateComponentHolder im
}
- public void disableComponents()
+ public void disableComponents( final boolean async )
{
m_enabled = false;
final ImmediateComponentManager[] cms = getComponentManagers( false );
if ( cms == null )
{
- m_singleComponent.disable();
+ m_singleComponent.disable( async );
}
else
{
for ( int i = 0; i < cms.length; i++ )
{
- cms[i].disable();
+ cms[i].disable( async );
}
}
}
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=1344701&r1=1344700&r2=1344701&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 Thu May 31 13:06:30 2012
@@ -27,6 +27,9 @@ import java.util.Enumeration;
import java.util.Hashtable;
import java.util.Iterator;
import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.ReentrantLock;
+
import org.apache.felix.scr.Component;
import org.apache.felix.scr.Reference;
import org.apache.felix.scr.impl.BundleComponentActivator;
@@ -36,7 +39,6 @@ import org.apache.felix.scr.impl.metadat
import org.apache.felix.scr.impl.metadata.ServiceMetadata;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
-import org.osgi.framework.Constants;
import org.osgi.framework.InvalidSyntaxException;
import org.osgi.framework.ServicePermission;
import org.osgi.framework.ServiceReference;
@@ -44,6 +46,7 @@ import org.osgi.framework.ServiceRegistr
import org.osgi.service.component.ComponentConstants;
import org.osgi.service.log.LogService;
+
/**
* The default ComponentManager. Objects of this class are responsible for managing
* implementation object's lifecycle.
@@ -51,6 +54,23 @@ import org.osgi.service.log.LogService;
*/
public abstract class AbstractComponentManager implements Component
{
+
+ private static final boolean JUC_AVAILABLE;
+
+ static {
+ boolean juc_available;
+ try
+ {
+ new JLock();
+ juc_available = true;
+ }
+ catch (Throwable t)
+ {
+ juc_available = false;
+ }
+ JUC_AVAILABLE = juc_available;
+ }
+
// the ID of this component
private long m_componentId;
@@ -71,6 +91,9 @@ public abstract class AbstractComponentM
// The ServiceRegistration
private volatile ServiceRegistration m_serviceRegistration;
+ private final LockWrapper m_stateLock;
+
+ private long m_timeout = 5000;
/**
* The constructor receives both the activator and the metadata
@@ -87,6 +110,15 @@ public abstract class AbstractComponentM
m_state = Disabled.getInstance();
m_dependencyManagers = loadDependencyManagers( metadata );
+ if (JUC_AVAILABLE)
+ {
+ m_stateLock = new JLock();
+ }
+ else
+ {
+ m_stateLock = new EDULock();
+ }
+
// dump component details
if ( isLogEnabled( LogService.LOG_DEBUG ) )
{
@@ -115,8 +147,39 @@ public abstract class AbstractComponentM
}
}
+ //ImmediateComponentHolder should be in this manager package and this should be default access.
+ public final void obtainStateLock()
+ {
+ try
+ {
+ if (!m_stateLock.tryLock( m_timeout ) )
+ {
+ throw new IllegalStateException( "Could not obtain lock" );
+ }
+ }
+ catch ( InterruptedException e )
+ {
+ //TODO this is so wrong
+ throw new IllegalStateException( e );
+ }
+ }
+
+
+ public final void releaseStateLock()
+ {
+ m_stateLock.unlock();
+ }
+
+
+ public final void checkLocked()
+ {
+ if ( m_stateLock.getHoldCount() == 0 )
+ {
+ throw new IllegalStateException( "State lock should be held by current thread" );
+ }
+ }
- //---------- Component ID management
+//---------- Component ID management
void registerComponentId()
{
@@ -143,6 +206,7 @@ public abstract class AbstractComponentM
//---------- Asynchronous frontend to state change methods ----------------
+
/**
* Enables this component and - if satisfied - also activates it. If
* enabling the component fails for any reason, the component ends up
@@ -155,8 +219,44 @@ public abstract class AbstractComponentM
*/
public final void enable()
{
- enableInternal();
- activateInternal();
+ enable( true );
+ }
+
+
+ public final void enable( final boolean async )
+ {
+ obtainStateLock();
+ try
+ {
+ enableInternal();
+ if ( !async )
+ {
+ activateInternal();
+ }
+ }
+ finally
+ {
+ releaseStateLock();
+ }
+
+ if ( async )
+ {
+ m_activator.schedule( new Runnable()
+ {
+ public void run()
+ {
+ obtainStateLock();
+ try
+ {
+ activateInternal();
+ }
+ finally
+ {
+ releaseStateLock();
+ }
+ }
+ } );
+ }
}
/**
@@ -167,8 +267,44 @@ public abstract class AbstractComponentM
*/
public final void disable()
{
- deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED );
- disableInternal();
+ disable( true );
+ }
+
+
+ public final void disable( final boolean async )
+ {
+ obtainStateLock();
+ try
+ {
+ if ( !async )
+ {
+ deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED );
+ }
+ disableInternal();
+ }
+ finally
+ {
+ releaseStateLock();
+ }
+
+ if ( async )
+ {
+ m_activator.schedule( new Runnable()
+ {
+ public void run()
+ {
+ obtainStateLock();
+ try
+ {
+ deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED );
+ }
+ finally
+ {
+ releaseStateLock();
+ }
+ }
+ } );
+ }
}
/**
@@ -195,7 +331,15 @@ public abstract class AbstractComponentM
*/
public void dispose( int reason )
{
- disposeInternal( reason );
+ obtainStateLock();
+ try
+ {
+ disposeInternal( reason );
+ }
+ finally
+ {
+ releaseStateLock();
+ }
}
//---------- Component interface ------------------------------------------
@@ -356,8 +500,6 @@ public abstract class AbstractComponentM
*/
final void disposeInternal( int reason )
{
- m_state.deactivate( this, reason );
- m_state.disable( this );
m_state.dispose( this );
}
@@ -388,7 +530,7 @@ public abstract class AbstractComponentM
//---------- Component handling methods ----------------------------------
/**
- * Method is called by {@link #activate()} in STATE_ACTIVATING or by
+ * Method is called by {@link State#activate(AbstractComponentManager)} in STATE_ACTIVATING or by
* {@link DelayedComponentManager#getService(Bundle, ServiceRegistration)}
* in STATE_REGISTERED.
*
@@ -416,13 +558,12 @@ public abstract class AbstractComponentM
{
if ( m_componentMetadata.isFactory() )
{
- if ( getInstance() != null )
+ if ( this instanceof ComponentFactoryImpl.ComponentFactoryConfiguredInstance )
+ {
+ return Active.getInstance();
+ }
+ else if ( this instanceof ComponentFactoryImpl.ComponentFactoryNewInstance )
{
- if ( this instanceof ComponentFactoryImpl.ComponentFactoryConfiguredInstance )
- {
- return Active.getInstance();
- }
-
return FactoryInstance.getInstance();
}
@@ -467,98 +608,23 @@ public abstract class AbstractComponentM
* {@link #registerService()} method. Also records the service
* registration for later {@link #unregisterComponentService()}.
* <p>
- * If the component's state changes during service registration,
- * the service is unregistered again and a WARN message is logged.
- * This situation may happen as described in FELIX-3317.
+ * Due to more extensive locking FELIX-3317 is no longer relevant.
*
- * @param preRegistrationState The component state before service
- * registration. This is the expected state after service
- * registration.
- */
- final void registerComponentService(final State preRegistrationState)
- {
-
- /*
- * Problem: If the component is being activated and a configuration
- * is updated a race condition may happen:
- *
- * 1-T1 Unsatisfied.activate has set the state to Active already
- * 2-T1 registerService is called but field is not assigned yet
- * during registerService ServiceListeners are called
- * 3-T2 A Configuration update comes in an Satisfied(Active).deactivate
- * is called
- * 4-T2 calls unregisterComponentService; does nothing because
- * field is not assigned
- * 5-T2 destroys component
- * 6-T1 assigns field from service registration
- *
- * Now all consumers are bound to a service object which has been
- * deactivated already.
- *
- * Simplest thing to do would be to compare the states before
- * and after service registration: If they are equal and the
- * field is still null, everything is fine. If they are not
- * equal or the field is set (maybe T2 is so fast registering
- * service that it passed by T1), the current registration must
- * be unregistered again and the field not be assigned. This
- * will unbind consumers from the unusable instance.
- *
- * See also FELIX-3317
- */
+ */
+ final void registerComponentService()
+ {
- final ServiceRegistration sr = registerService();
- if ( sr != null )
+ if (this.m_serviceRegistration != null)
{
- final State currentState = state();
-
- synchronized ( this )
- {
- if ( (currentState == preRegistrationState || currentState == Active.getInstance()) && this.m_serviceRegistration == null )
- {
- this.m_serviceRegistration = sr;
- return;
- }
- }
-
- // Get here if:
- // - state changed during service registration
- // - state is the same (again) but field is already set
- // both situations indicate the current registration is not to
- // be used
-
- if ( isLogEnabled( LogService.LOG_WARNING ) )
- {
- StringBuffer msg = new StringBuffer();
- msg.append( "State changed from " ).append( preRegistrationState );
- msg.append( " to " ).append( currentState );
- msg.append( " during service registration; unregistering service [" );
- ServiceReference ref = sr.getReference();
- msg.append( Arrays.asList( ( String[] ) ref.getProperty( Constants.OBJECTCLASS ) ) );
- msg.append( ',' );
- msg.append( ref.getProperty( Constants.SERVICE_ID ) );
- msg.append( ']' );
- log( LogService.LOG_WARNING, msg.toString(), null );
- }
-
- try
- {
- sr.unregister();
- }
- catch ( IllegalStateException ise )
- {
- // ignore
- }
+ throw new IllegalStateException( "Component service already registered: " + this );
}
+ this.m_serviceRegistration = registerService();
}
final void unregisterComponentService()
{
- final ServiceRegistration sr;
- synchronized ( this )
- {
- sr = this.m_serviceRegistration;
- this.m_serviceRegistration = null;
- }
+ ServiceRegistration sr = this.m_serviceRegistration;
+ this.m_serviceRegistration = null;
if ( sr != null )
{
@@ -784,6 +850,16 @@ public abstract class AbstractComponentM
return null;
}
+ private void deactivateDependencyManagers()
+ {
+ Iterator it = getDependencyManagers();
+ while ( it.hasNext() )
+ {
+ DependencyManager dm = (DependencyManager) it.next();
+ dm.deactivate();
+ }
+ }
+
private void disableDependencyManagers()
{
Iterator it = getDependencyManagers();
@@ -959,50 +1035,58 @@ public abstract class AbstractComponentM
ServiceReference getServiceReference( AbstractComponentManager acm )
{
- return null;
+// return null;
+ throw new IllegalStateException("getServiceReference" + this);
}
Object getService( DelayedComponentManager dcm )
{
- log( dcm, "getService" );
- return null;
+// log( dcm, "getService" );
+// return null;
+ throw new IllegalStateException("getService" + this);
}
void ungetService( DelayedComponentManager dcm )
{
- log( dcm, "ungetService" );
+// log( dcm, "ungetService" );
+ throw new IllegalStateException("ungetService" + this);
}
void enable( AbstractComponentManager acm )
{
log( acm, "enable" );
+// throw new IllegalStateException("enable" + this);
}
void activate( AbstractComponentManager acm )
{
log( acm, "activate" );
+// 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" );
+// log( acm, "disable" );
+ throw new IllegalStateException("disable" + this);
}
void dispose( AbstractComponentManager acm )
{
- log( acm, "dispose" );
+// log( acm, "dispose" );
+ throw new IllegalStateException("dispose" + this);
}
@@ -1011,6 +1095,29 @@ public abstract class AbstractComponentM
acm.log( LogService.LOG_DEBUG, "Current state: {0}, Event: {1}", new Object[]
{ m_name, event }, null );
}
+
+ void doDeactivate( AbstractComponentManager acm, int reason )
+ {
+ try
+ {
+ acm.unregisterComponentService();
+ acm.deleteComponent( reason );
+ acm.deactivateDependencyManagers();
+ }
+ catch ( Throwable t )
+ {
+ acm.log( LogService.LOG_WARNING, "Component deactivation threw an exception", t );
+ }
+ }
+
+ void doDisable( AbstractComponentManager acm )
+ {
+ // dispose and recreate dependency managers
+ acm.disableDependencyManagers();
+
+ // reset the component id now (a disabled component has none)
+ acm.unregisterComponentId();
+ }
}
protected static final class Disabled extends State
@@ -1039,7 +1146,6 @@ public abstract class AbstractComponentM
return;
}
- acm.changeState( Enabling.getInstance() );
acm.registerComponentId();
try
{
@@ -1057,10 +1163,13 @@ public abstract class AbstractComponentM
}
}
+ void deactivate( AbstractComponentManager acm, int reason )
+ {
+ doDeactivate( acm, reason );
+ }
void dispose( AbstractComponentManager acm )
{
- acm.changeState( Disposing.getInstance() );
acm.log( LogService.LOG_DEBUG, "Disposing component", null );
acm.clear();
acm.changeState( Disposed.getInstance() );
@@ -1069,23 +1178,6 @@ public abstract class AbstractComponentM
}
}
- protected static final class Enabling extends State
- {
- private static final Enabling m_inst = new Enabling();
-
-
- private Enabling()
- {
- super( "Enabling", STATE_ENABLING );
- }
-
-
- static State getInstance()
- {
- return m_inst;
- }
- }
-
protected static final class Unsatisfied extends State
{
private static final Unsatisfied m_inst = new Unsatisfied();
@@ -1112,8 +1204,6 @@ public abstract class AbstractComponentM
return;
}
- acm.changeState( Activating.getInstance() );
-
acm.log( LogService.LOG_DEBUG, "Activating component", null );
// Before creating the implementation object, we are going to
@@ -1121,10 +1211,21 @@ public abstract class AbstractComponentM
if ( !acm.hasConfiguration() && acm.getComponentMetadata().isConfigurationRequired() )
{
acm.log( LogService.LOG_DEBUG, "Missing required configuration, cannot activate", null );
- acm.changeState( Unsatisfied.getInstance() );
return;
}
+ // set satisifed state before registering the service because
+ // during service registration a listener may try to get the
+ // service from the service reference which may cause a
+ // delayed service object instantiation through the State
+
+ // actually since we don't have the activating state any
+ // longer, we have to set the satisfied state already
+ // before actually creating the component such that services
+ // may be accepted.
+ final State satisfiedState = acm.getSatisfiedState();
+ acm.changeState( satisfiedState );
+
// Before creating the implementation object, we are going to
// test if all the mandatory dependencies are satisfied
if ( !acm.verifyDependencyManagers( acm.getProperties() ) )
@@ -1158,51 +1259,28 @@ public abstract class AbstractComponentM
return;
}
- // set satisifed state before registering the service because
- // during service registration a listener may try to get the
- // service from the service reference which may cause a
- // delayed service object instantiation through the State
- final State satisfiedState = acm.getSatisfiedState();
- acm.changeState( satisfiedState );
-
- acm.registerComponentService( satisfiedState );
+ acm.registerComponentService();
}
void disable( AbstractComponentManager acm )
{
- acm.changeState( Disabling.getInstance() );
-
acm.log( LogService.LOG_DEBUG, "Disabling component", null );
-
- // dispose and recreate dependency managers
- acm.disableDependencyManagers();
-
- // reset the component id now (a disabled component has none)
- acm.unregisterComponentId();
+ doDisable( acm );
// we are now disabled, ready for re-enablement or complete destroyal
acm.changeState( Disabled.getInstance() );
acm.log( LogService.LOG_DEBUG, "Component disabled", null );
}
- }
-
- protected static final class Activating extends State
- {
- private static final Activating m_inst = new Activating();
-
- private Activating()
+ void dispose( AbstractComponentManager acm )
{
- super( "Activating", STATE_ACTIVATING );
+ doDisable( acm );
+ acm.clear(); //content of Disabled.dispose
+ acm.changeState( Disposed.getInstance() );
}
-
- static State getInstance()
- {
- return m_inst;
- }
}
protected static abstract class Satisfied extends State
@@ -1222,25 +1300,30 @@ public abstract class AbstractComponentM
void deactivate( AbstractComponentManager acm, int reason )
{
- acm.changeState( Deactivating.getInstance() );
-
acm.log( LogService.LOG_DEBUG, "Deactivating component", null );
// catch any problems from deleting the component to prevent the
// component to remain in the deactivating state !
- try
- {
- acm.unregisterComponentService();
- acm.deleteComponent( reason );
- }
- catch ( Throwable t )
- {
- acm.log( LogService.LOG_WARNING, "Component deactivation threw an exception", t );
- }
+ doDeactivate(acm, reason);
acm.changeState( Unsatisfied.getInstance() );
acm.log( LogService.LOG_DEBUG, "Component deactivated", null );
}
+
+ void disable( AbstractComponentManager acm )
+ {
+ doDisable( acm );
+ acm.changeState( Disabled.getInstance() );
+ }
+
+ void dispose( AbstractComponentManager acm )
+ {
+ doDeactivate( acm, ComponentConstants.DEACTIVATION_REASON_DISPOSED );
+ doDisable(acm);
+ acm.clear(); //content of Disabled.dispose
+ acm.changeState( Disposed.getInstance() );
+ }
+
}
/**
@@ -1388,14 +1471,17 @@ public abstract class AbstractComponentM
}
}
- protected static final class Deactivating extends State
+ /*
+ final state.
+ */
+ protected static final class Disposed extends State
{
- private static final Deactivating m_inst = new Deactivating();
+ private static final Disposed m_inst = new Disposed();
- private Deactivating()
+ private Disposed()
{
- super( "Deactivating", STATE_DEACTIVATING );
+ super( "Disposed", STATE_DISPOSED );
}
@@ -1403,56 +1489,77 @@ public abstract class AbstractComponentM
{
return m_inst;
}
- }
- protected static final class Disabling extends State
- {
- private static final Disabling m_inst = new Disabling();
+ void activate( AbstractComponentManager acm )
+ {
+ throw new IllegalStateException( "activate: " + this );
+ }
+ void deactivate( AbstractComponentManager acm, int reason )
+ {
+ throw new IllegalStateException( "deactivate: " + this );
+ }
- private Disabling()
+ void disable( AbstractComponentManager acm )
{
- super( "Disabling", STATE_DISABLING );
+ throw new IllegalStateException( "disable: " + this );
}
+ void dispose( AbstractComponentManager acm )
+ {
+ throw new IllegalStateException( "dispose: " + this );
+ }
- static State getInstance()
+ void enable( AbstractComponentManager acm )
{
- return m_inst;
+ throw new IllegalStateException( "enable: " + this );
}
}
- protected static final class Disposing extends State
+ private static interface LockWrapper
{
- private static final Disposing m_inst = new Disposing();
+ boolean tryLock( long milliseconds ) throws InterruptedException;
+ long getHoldCount();
+ void unlock();
+ }
+ private static class JLock implements LockWrapper
+ {
+ private final ReentrantLock lock = new ReentrantLock( true );
- private Disposing()
+ public boolean tryLock( long milliseconds ) throws InterruptedException
{
- super( "Disposing", STATE_DISPOSING );
+ return lock.tryLock( milliseconds, TimeUnit.MILLISECONDS );
}
+ public long getHoldCount()
+ {
+ return lock.getHoldCount();
+ }
- static State getInstance()
+ public void unlock()
{
- return m_inst;
+ lock.unlock();
}
}
- protected static final class Disposed extends State
+ private static class EDULock implements LockWrapper
{
- private static final Disposed m_inst = new Disposed();
+ private final EDU.oswego.cs.dl.util.concurrent.ReentrantLock lock = new EDU.oswego.cs.dl.util.concurrent.ReentrantLock();
-
- private Disposed()
+ public boolean tryLock( long milliseconds ) throws InterruptedException
{
- super( "Disposed", STATE_DISPOSED );
+ return lock.attempt( milliseconds );
}
+ public long getHoldCount()
+ {
+ return lock.holds();
+ }
- static State getInstance()
+ public void unlock()
{
- return m_inst;
+ lock.release();
}
}
}
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java?rev=1344701&r1=1344700&r2=1344701&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java Thu May 31 13:06:30 2012
@@ -108,19 +108,28 @@ public class ComponentFactoryImpl extend
{
final ImmediateComponentManager cm = createComponentManager( true );
- cm.setFactoryProperties( dictionary );
- cm.reconfigure( m_configuration );
+ ComponentInstance instance;
+ cm.obtainStateLock();
+ try
+ {
+ cm.setFactoryProperties( dictionary );
+ cm.reconfigure( m_configuration );
+
+ // enable and activate immediately
+ cm.enableInternal();
+ cm.activateInternal();
- // enable and activate immediately
- cm.enableInternal();
- cm.activateInternal();
-
- final ComponentInstance instance = cm.getComponentInstance();
- if ( instance == null )
- {
- // activation failed, clean up component manager
- cm.dispose();
- throw new ComponentException( "Failed activating component" );
+ instance = cm.getComponentInstance();
+ if ( instance == null )
+ {
+ // activation failed, clean up component manager
+ cm.dispose();
+ throw new ComponentException( "Failed activating component" );
+ }
+ }
+ finally
+ {
+ cm.releaseStateLock();
}
synchronized ( m_componentInstances )
@@ -144,7 +153,7 @@ public class ComponentFactoryImpl extend
ImmediateComponentManager[] cms = getComponentManagers( m_configuredServices );
for ( int i = 0; i < cms.length; i++ )
{
- cms[i].enable();
+ cms[i].enable( false );
}
return true;
@@ -294,50 +303,61 @@ public class ComponentFactoryImpl extend
{
m_configuration = configuration;
}
- else if ( m_isConfigurationFactory )
+ else if ( m_isConfigurationFactory ) //non-spec backwards compatible
{
- ImmediateComponentManager cm;
- Map configuredServices = m_configuredServices;
- if ( configuredServices != null )
+ obtainStateLock();
+ try
{
- synchronized ( configuredServices )
+ ImmediateComponentManager cm;
+ Map configuredServices = m_configuredServices;
+ if ( configuredServices != null )
{
cm = ( ImmediateComponentManager ) configuredServices.get( pid );
}
- }
- else
- {
- m_configuredServices = new HashMap();
- configuredServices = m_configuredServices;
- cm = null;
- }
-
- if ( cm == null )
- {
- // create a new instance with the current configuration
- cm = createComponentManager( false );
-
- // this should not call component reactivation because it is
- // not active yet
- cm.reconfigure( configuration );
-
- // enable asynchronously if components are already enabled
- if ( getState() == STATE_FACTORY )
+ else
{
- cm.enable();
+ m_configuredServices = new HashMap();
+ configuredServices = m_configuredServices;
+ cm = null;
}
- // keep a reference for future updates
- synchronized ( configuredServices )
+ if ( cm == null )
{
+ // create a new instance with the current configuration
+ cm = createComponentManager( false );
+
+ // this should not call component reactivation because it is
+ // not active yet
+ cm.reconfigure( configuration );
+
+ // enable asynchronously if components are already enabled
+ if ( getState() == STATE_FACTORY )
+ {
+ cm.enable( false );
+ }
+
+ // keep a reference for future updates
configuredServices.put( pid, cm );
- }
+ }
+ else
+ {
+ // update the configuration as if called as ManagedService
+ //TODO deadlock potential, we are holding our own state lock.
+ cm.obtainStateLock();
+ try
+ {
+ cm.reconfigure( configuration );
+ }
+ finally
+ {
+ cm.releaseStateLock();
+ }
+ }
}
- else
+ finally
{
- // update the configuration as if called as ManagedService
- cm.reconfigure( configuration );
+ releaseStateLock();
}
}
else
@@ -372,9 +392,9 @@ public class ComponentFactoryImpl extend
* A component factory component holder enables the held components by
* enabling itself.
*/
- public void enableComponents()
+ public void enableComponents( boolean async )
{
- enable();
+ enable( async );
}
@@ -382,9 +402,9 @@ public class ComponentFactoryImpl extend
* A component factory component holder disables the held components by
* disabling itself.
*/
- public void disableComponents()
+ public void disableComponents( boolean async )
{
- disable();
+ disable( async );
}
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DelayedComponentManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DelayedComponentManager.java?rev=1344701&r1=1344700&r2=1344701&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DelayedComponentManager.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DelayedComponentManager.java Thu May 31 13:06:30 2012
@@ -34,7 +34,6 @@ public class DelayedComponentManager ext
{
// keep the using bundles as reference "counters" for instance deactivation
- private final Object m_useCountLock;
private int m_useCount;
@@ -46,7 +45,6 @@ public class DelayedComponentManager ext
ComponentMetadata metadata )
{
super( activator, componentHolder, metadata );
- this.m_useCountLock = new Object();
this.m_useCount = 0;
}
@@ -80,13 +78,18 @@ public class DelayedComponentManager ext
//---------- ServiceFactory interface -------------------------------------
- public synchronized Object getService( Bundle bundle, ServiceRegistration sr )
+ public Object getService( Bundle bundle, ServiceRegistration sr )
{
- synchronized ( m_useCountLock )
+ obtainStateLock();
+ try
{
m_useCount++;
return state().getService( this );
}
+ finally
+ {
+ releaseStateLock();
+ }
}
@@ -98,7 +101,8 @@ public class DelayedComponentManager ext
public void ungetService( Bundle bundle, ServiceRegistration sr, Object service )
{
- synchronized ( m_useCountLock )
+ obtainStateLock();
+ try
{
// the framework should not call ungetService more than it calls
// calls getService. Still, we want to be sure to not go below zero
@@ -115,5 +119,9 @@ public class DelayedComponentManager ext
}
}
}
+ finally
+ {
+ releaseStateLock();
+ }
}
}
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java?rev=1344701&r1=1344700&r2=1344701&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java Thu May 31 13:06:30 2012
@@ -103,8 +103,6 @@ public class DependencyManager implement
m_dependencyMetadata = dependency;
m_bound = Collections.synchronizedMap( new HashMap() );
- // setup the target filter from component descriptor
- setTargetFilter( m_dependencyMetadata.getTarget() );
// dump the reference information if DEBUG is enabled
if ( m_componentManager.isLogEnabled( LogService.LOG_DEBUG ) )
@@ -157,103 +155,110 @@ public class DependencyManager implement
final ServiceReference ref = event.getServiceReference();
final String serviceString = "Service " + m_dependencyMetadata.getInterface() + "/"
+ ref.getProperty( Constants.SERVICE_ID );
-
- switch ( event.getType() )
+ m_componentManager.obtainStateLock();
+ try
{
- case ServiceEvent.REGISTERED:
- m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Adding {0}", new Object[]
- { serviceString }, null );
-
- // consider the service if the filter matches
- if ( targetFilterMatch( ref ) )
- {
- m_size++;
- serviceAdded( ref );
- }
- else
- {
- m_componentManager.log( LogService.LOG_DEBUG,
- "Dependency Manager: Ignoring added Service for {0} : does not match target filter {1}",
- new Object[]
- { m_dependencyMetadata.getName(), getTarget() }, null );
- }
- break;
-
- case ServiceEvent.MODIFIED:
- m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
- { serviceString }, null );
+ switch ( event.getType() )
+ {
+ case ServiceEvent.REGISTERED:
+ m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Adding {0}", new Object[]
+ { serviceString }, null );
- if ( getBoundService( ref ) == null )
- {
- // service not currently bound --- what to do ?
- // if static
- // if inactive and target match: activate
- // if dynamic
- // if multiple and target match: bind
+ // consider the service if the filter matches
if ( targetFilterMatch( ref ) )
{
- // new filter match, so increase the counter
m_size++;
+ serviceAdded( ref );
+ }
+ else
+ {
+ m_componentManager.log( LogService.LOG_DEBUG,
+ "Dependency Manager: Ignoring added Service for {0} : does not match target filter {1}",
+ new Object[]
+ { m_dependencyMetadata.getName(), getTarget() }, null );
+ }
+ break;
+
+ case ServiceEvent.MODIFIED:
+ m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
+ { serviceString }, null );
- if ( isStatic() )
+ if ( getBoundService( ref ) == null )
+ {
+ // service not currently bound --- what to do ?
+ // if static
+ // if inactive and target match: activate
+ // if dynamic
+ // if multiple and target match: bind
+ if ( targetFilterMatch( ref ) )
{
- // if static reference: activate if currentl unsatisifed, otherwise no influence
- if ( m_componentManager.getState() == AbstractComponentManager.STATE_UNSATISFIED )
- {
- m_componentManager.log( LogService.LOG_DEBUG,
- "Dependency Manager: Service {0} registered, activate component", new Object[]
- { m_dependencyMetadata.getName() }, null );
+ // new filter match, so increase the counter
+ m_size++;
- // immediately try to activate the component (FELIX-2368)
- m_componentManager.activateInternal();
+ if ( isStatic() )
+ {
+ // if static reference: activate if currentl unsatisifed, otherwise no influence
+ if ( m_componentManager.getState() == AbstractComponentManager.STATE_UNSATISFIED )
+ {
+ m_componentManager.log( LogService.LOG_DEBUG,
+ "Dependency Manager: Service {0} registered, activate component", new Object[]
+ { m_dependencyMetadata.getName() }, null );
+
+ // immediately try to activate the component (FELIX-2368)
+ m_componentManager.activateInternal();
+ }
+ }
+ else if ( isMultiple() )
+ {
+ // if dynamic and multiple reference, bind, otherwise ignore
+ serviceAdded( ref );
}
- }
- else if ( isMultiple() )
- {
- // if dynamic and multiple reference, bind, otherwise ignore
- serviceAdded( ref );
}
}
- }
- else if ( !targetFilterMatch( ref ) )
- {
- // service reference does not match target any more, remove
- m_size--;
- serviceRemoved( ref );
- }
- else
- {
- // update the service binding due to the new properties
- update( ref );
- }
+ else if ( !targetFilterMatch( ref ) )
+ {
+ // service reference does not match target any more, remove
+ m_size--;
+ serviceRemoved( ref );
+ }
+ else
+ {
+ // update the service binding due to the new properties
+ update( ref );
+ }
- break;
+ break;
- case ServiceEvent.UNREGISTERING:
- m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Removing {0}", new Object[]
- { serviceString }, null );
+ case ServiceEvent.UNREGISTERING:
+ m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Removing {0}", new Object[]
+ { serviceString }, null );
- // manage the service counter if the filter matchs
- if ( targetFilterMatch( ref ) )
- {
- m_size--;
- }
- else
- {
- m_componentManager
- .log(
- LogService.LOG_DEBUG,
- "Dependency Manager: Not counting Service for {0} : Service {1} does not match target filter {2}",
- new Object[]
- { m_dependencyMetadata.getName(), ref.getProperty( Constants.SERVICE_ID ), getTarget() },
- null );
- }
+ // manage the service counter if the filter matchs
+ if ( targetFilterMatch( ref ) )
+ {
+ m_size--;
+ }
+ else
+ {
+ m_componentManager
+ .log(
+ LogService.LOG_DEBUG,
+ "Dependency Manager: Not counting Service for {0} : Service {1} does not match target filter {2}",
+ new Object[]
+ { m_dependencyMetadata.getName(), ref.getProperty( Constants.SERVICE_ID ), getTarget() },
+ null );
+ }
- // remove the service ignoring the filter match because if the
- // service is bound, it has to be removed no matter what
- serviceRemoved( ref );
+ // remove the service ignoring the filter match because if the
+ // service is bound, it has to be removed no matter what
+ serviceRemoved( ref );
- break;
+ break;
+ }
+ }
+ finally
+ {
+ m_componentManager.releaseStateLock();
}
}
@@ -510,6 +515,9 @@ public class DependencyManager implement
*/
void enable() throws InvalidSyntaxException
{
+ // setup the target filter from component descriptor
+ setTargetFilter( m_dependencyMetadata.getTarget() );
+
if ( hasGetPermission() )
{
// get the current number of registered services available
@@ -548,7 +556,10 @@ public class DependencyManager implement
context.removeServiceListener( this );
m_size = 0;
+ }
+ void deactivate()
+ {
// unget all services we once got
ServiceReference[] boundRefs = getBoundServiceReferences();
if ( boundRefs != null )
@@ -558,9 +569,6 @@ public class DependencyManager implement
ungetService( boundRefs[i] );
}
}
-
- // reset the target filter from component descriptor
- setTargetFilter( m_dependencyMetadata.getTarget() );
}
@@ -572,7 +580,7 @@ public class DependencyManager implement
* manager. It is actually the maximum number of services which may be
* bound to this dependency manager.
*
- * @see #isValid()
+ * @see #isSatisfied()
*/
int size()
{
@@ -1321,6 +1329,7 @@ public class DependencyManager implement
*/
private void setTargetFilter( String target )
{
+ m_componentManager.checkLocked();
// do nothing if target filter does not change
if ( ( m_target == null && target == null ) || ( m_target != null && m_target.equals( target ) ) )
{
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=1344701&r1=1344700&r2=1344701&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 Thu May 31 13:06:30 2012
@@ -94,13 +94,6 @@ public class ImmediateComponentManager e
m_componentHolder = componentHolder;
}
-
- ComponentHolder getComponentHolder()
- {
- return m_componentHolder;
- }
-
-
void clear()
{
if ( m_componentHolder != null )
@@ -146,12 +139,6 @@ public class ImmediateComponentManager e
}
- ComponentContext getComponentContext()
- {
- return m_componentContext;
- }
-
-
public ComponentInstance getComponentInstance()
{
return m_componentContext;
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=1344701&r1=1344700&r2=1344701&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 Thu May 31 13:06:30 2012
@@ -100,17 +100,18 @@ public class ServiceFactoryComponentMana
// When the getServiceMethod is called, the implementation object must be created
- // private ComponentContext and implementation instances
- BundleComponentContext serviceContext = new BundleComponentContext( this, bundle );
- Object service = createImplementationObject( serviceContext );
-
- // register the components component context if successfull
- if ( service != null )
+ obtainStateLock();
+ try
{
- serviceContext.setImplementationObject( service );
+// private ComponentContext and implementation instances
+ BundleComponentContext serviceContext = new BundleComponentContext( this, bundle );
+ Object service = createImplementationObject( serviceContext );
- synchronized ( serviceContexts )
+ // register the components component context if successfull
+ if ( service != null )
{
+ serviceContext.setImplementationObject( service );
+
serviceContexts.put( service, serviceContext );
// if this is the first use of this component, switch to ACTIVE state
@@ -119,15 +120,19 @@ public class ServiceFactoryComponentMana
changeState( Active.getInstance() );
}
}
+ else
+ {
+ // log that the service factory component cannot be created (we don't
+ // know why at this moment; this should already have been logged)
+ log( LogService.LOG_ERROR, "Failed creating the component instance; see log for reason", null );
+ }
+
+ return service;
}
- else
+ finally
{
- // log that the service factory component cannot be created (we don't
- // know why at this moment; this should already have been logged)
- log( LogService.LOG_ERROR, "Failed creating the component instance; see log for reason", null );
+ releaseStateLock();
}
-
- return service;
}
@@ -139,24 +144,25 @@ public class ServiceFactoryComponentMana
log( LogService.LOG_DEBUG, "ServiceFactory.ungetService()", null );
// When the ungetServiceMethod is called, the implementation object must be deactivated
-
- // private ComponentContext and implementation instances
- final ComponentContext serviceContext;
- synchronized ( serviceContexts )
+ obtainStateLock();
+ try
{
+// private ComponentContext and implementation instances
+ final ComponentContext serviceContext;
serviceContext = ( ComponentContext ) serviceContexts.remove( service );
- }
- disposeImplementationObject( service, serviceContext, ComponentConstants.DEACTIVATION_REASON_DISPOSED );
+ disposeImplementationObject( service, serviceContext, ComponentConstants.DEACTIVATION_REASON_DISPOSED );
- // if this was the last use of the component, go back to REGISTERED state
- synchronized ( serviceContexts )
- {
+ // if this was the last use of the component, go back to REGISTERED state
if ( serviceContexts.isEmpty() && getState() == STATE_ACTIVE )
{
changeState( Registered.getInstance() );
}
}
+ finally
+ {
+ releaseStateLock();
+ }
}
//---------- Component interface
Modified: felix/trunk/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java?rev=1344701&r1=1344700&r2=1344701&view=diff
==============================================================================
--- felix/trunk/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java (original)
+++ felix/trunk/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java Thu May 31 13:06:30 2012
@@ -649,6 +649,7 @@ public class ComponentFactoryTest extend
}
}
+ //it has already been deactivated.... this should cause an exception?
noMatch.getRegistration().unregister();
delay();
@@ -673,8 +674,9 @@ public class ComponentFactoryTest extend
TestCase.assertNull( instanceNonMatch.getInstance() );
TestCase.assertNull( SimpleComponent.INSTANCE );
- instanceNonMatch.dispose();
- TestCase.assertNull( SimpleComponent.INSTANCE );
- TestCase.assertNull( instanceNonMatch.getInstance() ); // SCR 112.12.6.2
+ //FactoryInstance.deactivate disposes the instance. Don't do it again
+// instanceNonMatch.dispose();
+// TestCase.assertNull( SimpleComponent.INSTANCE );
+// TestCase.assertNull( instanceNonMatch.getInstance() ); // SCR 112.12.6.2
}
}