You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by dj...@apache.org on 2013/09/11 23:13:51 UTC
svn commit: r1522046 - in
/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl:
helper/ComponentMethods.java manager/AbstractComponentManager.java
manager/ComponentFactoryImpl.java manager/DependencyManager.java
manager/ImmediateComponentManager.java
Author: djencks
Date: Wed Sep 11 21:13:50 2013
New Revision: 1522046
URL: http://svn.apache.org/r1522046
Log:
FELIX-4223 Dependency manager targets should be set during enable; setting during activate leads to race conditions. Also prevent activation during modification of target properties.
Modified:
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/helper/ComponentMethods.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/DependencyManager.java
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/helper/ComponentMethods.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/helper/ComponentMethods.java?rev=1522046&r1=1522045&r2=1522046&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/helper/ComponentMethods.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/helper/ComponentMethods.java Wed Sep 11 21:13:50 2013
@@ -54,9 +54,8 @@ public class ComponentMethods
m_modifiedMethod = new ModifiedMethod( componentMetadata.getModified(), implementationObjectClass, isDS11, isDS12Felix );
- for ( Iterator it = componentMetadata.getDependencies().iterator(); it.hasNext(); )
+ for ( ReferenceMetadata referenceMetadata: componentMetadata.getDependencies() )
{
- ReferenceMetadata referenceMetadata = ( ReferenceMetadata ) it.next();
String refName = referenceMetadata.getName();
BindMethods bindMethods = new BindMethods( referenceMetadata, implementationObjectClass, isDS11, isDS12Felix);
bindMethodMap.put( refName, bindMethods );
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=1522046&r1=1522045&r2=1522046&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 Wed Sep 11 21:13:50 2013
@@ -91,7 +91,7 @@ public abstract class AbstractComponentM
private volatile boolean m_dependenciesCollected;
- private final AtomicInteger trackingCount = new AtomicInteger( );
+ private final AtomicInteger m_trackingCount = new AtomicInteger( );
// A reference to the BundleComponentActivator
private BundleComponentActivator m_activator;
@@ -100,22 +100,21 @@ public abstract class AbstractComponentM
private final ReentrantLock m_stateLock;
- protected volatile boolean enabled;
- protected volatile CountDownLatch enabledLatch;
- private final Object enabledLatchLock = new Object();
+ protected volatile boolean m_enabled;
+ protected final AtomicReference< CountDownLatch> m_enabledLatchRef = new AtomicReference<CountDownLatch>( new CountDownLatch(0) );
protected volatile boolean m_internalEnabled;
- private volatile boolean disposed;
+ private volatile boolean m_disposed;
//service event tracking
- private int floor;
+ private int m_floor;
- private volatile int ceiling;
+ private volatile int m_ceiling;
- private final Lock missingLock = new ReentrantLock();
- private final Condition missingCondition = missingLock.newCondition();
- private final Set<Integer> missing = new TreeSet<Integer>( );
+ private final Lock m_missingLock = new ReentrantLock();
+ private final Condition m_missingCondition = m_missingLock.newCondition();
+ private final Set<Integer> m_missing = new TreeSet<Integer>( );
volatile boolean m_activated;
@@ -237,43 +236,43 @@ public abstract class AbstractComponentM
//service event tracking
void tracked( int trackingCount )
{
- missingLock.lock();
+ m_missingLock.lock();
try
{
- if (trackingCount == floor + 1 )
+ if (trackingCount == m_floor + 1 )
{
- floor++;
- missing.remove( trackingCount );
+ m_floor++;
+ m_missing.remove( trackingCount );
}
- else if ( trackingCount < ceiling )
+ else if ( trackingCount < m_ceiling )
{
- missing.remove( trackingCount );
+ m_missing.remove( trackingCount );
}
- if ( trackingCount > ceiling )
+ if ( trackingCount > m_ceiling )
{
- for (int i = ceiling + 1; i < trackingCount; i++ )
+ for (int i = m_ceiling + 1; i < trackingCount; i++ )
{
- missing.add( i );
+ m_missing.add( i );
}
- ceiling = trackingCount;
+ m_ceiling = trackingCount;
}
- missingCondition.signalAll();
+ m_missingCondition.signalAll();
}
finally
{
- missingLock.unlock();
+ m_missingLock.unlock();
}
}
void waitForTracked( int trackingCount )
{
- missingLock.lock();
+ m_missingLock.lock();
try
{
- while ( ceiling < trackingCount || ( !missing.isEmpty() && missing.iterator().next() < trackingCount))
+ while ( m_ceiling < trackingCount || ( !m_missing.isEmpty() && m_missing.iterator().next() < trackingCount))
{
log( LogService.LOG_DEBUG, "waitForTracked trackingCount: {0} ceiling: {1} missing: {2}",
- new Object[] {trackingCount, ceiling, missing}, null );
+ new Object[] {trackingCount, m_ceiling, m_missing}, null );
try
{
if ( !doMissingWait())
@@ -293,7 +292,7 @@ public abstract class AbstractComponentM
catch ( InterruptedException e1 )
{
log( LogService.LOG_ERROR, "waitForTracked interrupted twice: {0} ceiling: {1} missing: {2}, Expect further errors",
- new Object[] {trackingCount, ceiling, missing}, e1 );
+ new Object[] {trackingCount, m_ceiling, m_missing}, e1 );
}
Thread.currentThread().interrupt();
}
@@ -301,18 +300,18 @@ public abstract class AbstractComponentM
}
finally
{
- missingLock.unlock();
+ m_missingLock.unlock();
}
}
private boolean doMissingWait() throws InterruptedException
{
- if ( !missingCondition.await( getLockTimeout(), TimeUnit.MILLISECONDS ))
+ if ( !m_missingCondition.await( getLockTimeout(), TimeUnit.MILLISECONDS ))
{
log( LogService.LOG_ERROR, "waitForTracked timed out: {0} ceiling: {1} missing: {2}, Expect further errors",
- new Object[] {trackingCount, ceiling, missing}, null );
+ new Object[] {m_trackingCount, m_ceiling, m_missing}, null );
dumpThreads();
- missing.clear();
+ m_missing.clear();
return false;
}
return true;
@@ -365,24 +364,18 @@ public abstract class AbstractComponentM
public final void enable( final boolean async )
{
- if (enabled)
+ if (m_enabled)
{
return;
}
+ CountDownLatch enableLatch = null;
try
{
- synchronized ( enabledLatchLock )
- {
- if ( enabledLatch != null )
- {
- enabledLatch.await();
- }
- enabledLatch = new CountDownLatch( 1 );
- }
+ enableLatch = enableLatchWait();
enableInternal();
if ( !async )
{
- activateInternal( trackingCount.get() );
+ activateInternal( m_trackingCount.get() );
}
}
catch ( InterruptedException e )
@@ -393,13 +386,14 @@ public abstract class AbstractComponentM
{
if ( !async )
{
- enabledLatch.countDown();
+ enableLatch.countDown();
}
- enabled = true;
+ m_enabled = true;
}
if ( async )
{
+ final CountDownLatch latch = enableLatch;
m_activator.schedule( new Runnable()
{
@@ -409,11 +403,11 @@ public abstract class AbstractComponentM
{
try
{
- activateInternal( trackingCount.get() );
+ activateInternal( m_trackingCount.get() );
}
finally
{
- enabledLatch.countDown();
+ latch.countDown();
}
}
@@ -425,6 +419,20 @@ public abstract class AbstractComponentM
}
}
+ CountDownLatch enableLatchWait() throws InterruptedException
+ {
+ CountDownLatch enabledLatch;
+ CountDownLatch newEnabledLatch;
+ do
+ {
+ enabledLatch = m_enabledLatchRef.get();
+ enabledLatch.await();
+ newEnabledLatch = new CountDownLatch(1);
+ }
+ while ( !m_enabledLatchRef.compareAndSet( enabledLatch, newEnabledLatch) );
+ return newEnabledLatch;
+ }
+
/**
* Disables this component and - if active - first deactivates it. The
* component may be reenabled by calling the {@link #enable()} method.
@@ -439,23 +447,17 @@ public abstract class AbstractComponentM
public final void disable( final boolean async )
{
- if (!enabled)
+ if (!m_enabled)
{
return;
}
+ CountDownLatch enableLatch = null;
try
{
- synchronized ( enabledLatchLock )
- {
- if (enabledLatch != null)
- {
- enabledLatch.await();
- }
- enabledLatch = new CountDownLatch( 1 );
- }
+ enableLatch = enableLatchWait();
if ( !async )
{
- deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED, true, trackingCount.get() );
+ deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED, true, m_trackingCount.get() );
}
disableInternal();
}
@@ -467,13 +469,14 @@ public abstract class AbstractComponentM
{
if (!async)
{
- enabledLatch.countDown();
+ enableLatch.countDown();
}
- enabled = false;
+ m_enabled = false;
}
if ( async )
{
+ final CountDownLatch latch = enableLatch;
m_activator.schedule( new Runnable()
{
@@ -483,11 +486,11 @@ public abstract class AbstractComponentM
{
try
{
- deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED, true, trackingCount.get() );
+ deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED, true, m_trackingCount.get() );
}
finally
{
- enabledLatch.countDown();
+ latch.countDown();
}
}
@@ -517,7 +520,7 @@ public abstract class AbstractComponentM
*/
public void dispose( int reason )
{
- disposed = true;
+ m_disposed = true;
disposeInternal( reason );
}
@@ -678,7 +681,7 @@ public abstract class AbstractComponentM
final void enableInternal()
{
- if ( disposed )
+ if ( m_disposed )
{
throw new IllegalStateException( "enable: " + this );
}
@@ -690,6 +693,15 @@ public abstract class AbstractComponentM
}
registerComponentId();
+ // Before creating the implementation object, we are going to
+ // test if we have configuration if such is required
+ if ( hasConfiguration() || !getComponentMetadata().isConfigurationRequired() )
+ {
+ // Update our target filters.
+ log( LogService.LOG_DEBUG, "Updating target filters", null );
+ updateTargets( getProperties() );
+ }
+
m_internalEnabled = true;
log( LogService.LOG_DEBUG, "Component enabled", null );
}
@@ -698,7 +710,7 @@ public abstract class AbstractComponentM
{
log( LogService.LOG_DEBUG, "ActivateInternal",
null );
- if ( disposed )
+ if ( m_disposed )
{
log( LogService.LOG_DEBUG, "ActivateInternal: disposed",
null );
@@ -741,10 +753,6 @@ public abstract class AbstractComponentM
return;
}
- // Update our target filters.
- log( LogService.LOG_DEBUG, "Updating target filters", null );
- updateTargets( getProperties() );
-
// Before creating the implementation object, we are going to
// test if all the mandatory dependencies are satisfied
if ( !verifyDependencyManagers() )
@@ -768,7 +776,7 @@ public abstract class AbstractComponentM
final void deactivateInternal( int reason, boolean disable, int trackingCount )
{
- if ( disposed )
+ if ( m_disposed )
{
return;
}
@@ -792,7 +800,7 @@ public abstract class AbstractComponentM
final void disableInternal()
{
m_internalEnabled = false;
- if ( disposed )
+ if ( m_disposed )
{
throw new IllegalStateException( "Cannot disable a disposed component " + getName() );
}
@@ -976,7 +984,7 @@ public abstract class AbstractComponentM
AtomicInteger getTrackingCount()
{
- return trackingCount;
+ return m_trackingCount;
}
@@ -1170,14 +1178,11 @@ public abstract class AbstractComponentM
return depMgrList;
}
- protected void updateTargets(Dictionary<String, Object> properties)
+ final void updateTargets(Dictionary<String, Object> properties)
{
- if ( m_internalEnabled )
+ for ( DependencyManager<S, ?> dm: getDependencyManagers() )
{
- for ( DependencyManager<S, ?> dm: getDependencyManagers() )
- {
- dm.setTargetFilter( properties );
- }
+ dm.setTargetFilter( properties );
}
}
@@ -1360,7 +1365,7 @@ public abstract class AbstractComponentM
public int getState()
{
- if (disposed)
+ if (m_disposed)
{
return Component.STATE_DISPOSED;
}
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=1522046&r1=1522045&r2=1522046&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 Wed Sep 11 21:13:50 2013
@@ -358,7 +358,7 @@ public class ComponentFactoryImpl<S> ext
{
log( LogService.LOG_INFO, "Configuration PID updated for Component Factory", null );
- // Ignore the configuration is our policy is 'ignore'
+ // Ignore the configuration if our policy is 'ignore'
if ( getComponentMetadata().isConfigurationIgnored() )
{
return false;
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=1522046&r1=1522045&r2=1522046&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 Wed Sep 11 21:13:50 2013
@@ -61,7 +61,7 @@ import org.osgi.service.log.LogService;
public class DependencyManager<S, T> implements Reference
{
// mask of states ok to send events
- private static final int STATE_MASK = //Component.STATE_UNSATISFIED |
+ private static final int STATE_MASK =
Component.STATE_ACTIVE | Component.STATE_REGISTERED | Component.STATE_FACTORY;
// the component to which this dependency belongs
@@ -72,11 +72,13 @@ public class DependencyManager<S, T> imp
private final int m_index;
- private final AtomicReference<ServiceTracker<T, RefPair<T>>> trackerRef = new AtomicReference<ServiceTracker<T, RefPair<T>>>();
+ private final Customizer<T> m_customizer;
- private final Customizer customizer;
+ //only set once, but it's not clear there is enough other synchronization to get the correct object before it's used.
+ private volatile BindMethods m_bindMethods;
- private BindMethods m_bindMethods;
+ //reset on filter change
+ private volatile ServiceTracker<T, RefPair<T>> m_tracker;
// the target service filter string
private volatile String m_target;
@@ -84,7 +86,7 @@ public class DependencyManager<S, T> imp
// the target service filter
private volatile Filter m_targetFilter;
- private boolean registered;
+ //private volatile boolean m_registered;
/**
* Constructor that receives several parameters.
@@ -96,7 +98,7 @@ public class DependencyManager<S, T> imp
m_componentManager = componentManager;
m_dependencyMetadata = dependency;
m_index = index;
- customizer = newCustomizer();
+ m_customizer = newCustomizer();
// dump the reference information if DEBUG is enabled
if ( m_componentManager.isLogEnabled( LogService.LOG_DEBUG ) )
@@ -156,7 +158,7 @@ public class DependencyManager<S, T> imp
public void setTracker( ServiceTracker<T, RefPair<T>> tracker )
{
- trackerRef.set( tracker );
+ m_tracker = tracker;
m_componentManager.log( LogService.LOG_DEBUG, "dm {0} tracker reset (closed)", new Object[] {getName()}, null );
trackerOpened = false;
}
@@ -173,7 +175,7 @@ public class DependencyManager<S, T> imp
protected ServiceTracker<T, RefPair<T>> getTracker()
{
- return trackerRef.get();
+ return m_tracker;
}
/**
@@ -1110,7 +1112,7 @@ public class DependencyManager<S, T> imp
void deactivate()
{
- customizer.close();
+ m_customizer.close();
}
@@ -1127,7 +1129,7 @@ public class DependencyManager<S, T> imp
int size()
{
AtomicInteger trackingCount = new AtomicInteger( );
- return trackerRef.get().getTracked( null, trackingCount ).size();
+ return m_tracker.getTracked( null, trackingCount ).size();
}
@@ -1185,11 +1187,7 @@ public class DependencyManager<S, T> imp
*/
private RefPair<T> getBestRefPair()
{
- if (customizer == null )
- {
- return null;
- }
- Collection<RefPair<T>> refs = customizer.getRefs( new AtomicInteger( ) );
+ Collection<RefPair<T>> refs = m_customizer.getRefs( new AtomicInteger( ) );
if (refs.isEmpty())
{
return null;
@@ -1220,7 +1218,7 @@ public class DependencyManager<S, T> imp
*/
T[] getServices()
{
- Collection<RefPair<T>> refs = customizer.getRefs( new AtomicInteger( ) );
+ Collection<RefPair<T>> refs = m_customizer.getRefs( new AtomicInteger( ) );
List<T> services = new ArrayList<T>( refs.size() );
for ( RefPair<T> ref: refs)
{
@@ -1243,7 +1241,7 @@ public class DependencyManager<S, T> imp
*/
public ServiceReference<T>[] getServiceReferences()
{
- Collection<RefPair<T>> bound = customizer.getRefs( new AtomicInteger( ) );
+ Collection<RefPair<T>> bound = m_customizer.getRefs( new AtomicInteger( ) );
if ( bound.isEmpty() )
{
return null;
@@ -1280,7 +1278,7 @@ public class DependencyManager<S, T> imp
private RefPair<T> getRefPair( ServiceReference<T> serviceReference )
{
AtomicInteger trackingCount = new AtomicInteger( );
- return trackerRef.get().getTracked( null, trackingCount ).get( serviceReference );
+ return m_tracker.getTracked( null, trackingCount ).get( serviceReference );
}
@@ -1369,7 +1367,7 @@ public class DependencyManager<S, T> imp
*/
public boolean isSatisfied()
{
- return customizer.isSatisfied();
+ return m_customizer.isSatisfied();
}
@@ -1391,7 +1389,7 @@ public class DependencyManager<S, T> imp
boolean prebind()
{
- return customizer.prebind();
+ return m_customizer.prebind();
}
/**
@@ -1429,9 +1427,9 @@ public class DependencyManager<S, T> imp
AtomicInteger trackingCount = new AtomicInteger( );
Collection<RefPair<T>> refs;
CountDownLatch openLatch = new CountDownLatch(1);
- synchronized ( trackerRef.get().tracked() )
+ synchronized ( m_tracker.tracked() )
{
- refs = customizer.getRefs( trackingCount );
+ refs = m_customizer.getRefs( trackingCount );
edgeInfo.setOpen( trackingCount.get() );
edgeInfo.setOpenLatch( openLatch );
}
@@ -1470,9 +1468,9 @@ public class DependencyManager<S, T> imp
AtomicInteger trackingCount = new AtomicInteger();
Collection<RefPair<T>> refPairs;
CountDownLatch latch = new CountDownLatch( 1 );
- synchronized ( trackerRef.get().tracked() )
+ synchronized ( m_tracker.tracked() )
{
- refPairs = customizer.getRefs( trackingCount );
+ refPairs = m_customizer.getRefs( trackingCount );
edgeInfo.setClose( trackingCount.get() );
edgeInfo.setCloseLatch( latch );
}
@@ -1500,7 +1498,7 @@ public class DependencyManager<S, T> imp
}
if ( !isMultiple() )
{
- Collection<RefPair<T>> refs = customizer.getRefs( new AtomicInteger( ) );
+ Collection<RefPair<T>> refs = m_customizer.getRefs( new AtomicInteger( ) );
if (refs.isEmpty())
{
return;
@@ -1513,7 +1511,7 @@ public class DependencyManager<S, T> imp
}
}
//TODO dynamic reluctant
- RefPair<T> refPair = trackerRef.get().getService( ref );
+ RefPair<T> refPair = m_tracker.getService( ref );
if (refPair.getServiceObject() != null)
{
m_componentManager.log( LogService.LOG_DEBUG,
@@ -1552,7 +1550,7 @@ public class DependencyManager<S, T> imp
// null. This is valid for both immediate and delayed components
if ( componentInstance != null )
{
- synchronized ( trackerRef.get().tracked() )
+ synchronized ( m_tracker.tracked() )
{
if (info.outOfRange( trackingCount ) )
{
@@ -1603,15 +1601,7 @@ public class DependencyManager<S, T> imp
// null. This is valid for both immediate and delayed components
if ( componentInstance != null )
{
- if (refPair == null)
- {
-
- //TODO should this be possible? If so, reduce or eliminate logging
- m_componentManager.log( LogService.LOG_WARNING,
- "DependencyManager : invokeUpdatedMethod : Component set, but reference not present", null );
- return;
- }
- synchronized ( trackerRef.get().tracked() )
+ synchronized ( m_tracker.tracked() )
{
if (info.outOfRange( trackingCount ) )
{
@@ -1742,13 +1732,6 @@ public class DependencyManager<S, T> imp
//ignore
}
- if (refPair == null)
- {
- //TODO should this be possible? If so, reduce or eliminate logging
- m_componentManager.log( LogService.LOG_WARNING,
- "DependencyManager : invokeUnbindMethod : Component set, but reference not present {0}", new Object[] {getName()}, null );
- return;
- }
if ( !getServiceObject( m_bindMethods.getUnbind(), refPair ))
{
m_componentManager.log( LogService.LOG_WARNING,
@@ -1863,15 +1846,7 @@ public class DependencyManager<S, T> imp
*/
void setTargetFilter( Dictionary<String, Object> properties )
{
- try
- {
- setTargetFilter( ( String ) properties.get( m_dependencyMetadata.getTargetPropertyName() ) );
- }
- catch ( InvalidSyntaxException e )
- {
- // this should not occur. The only choice would be if the filter for the object class was invalid,
- // but we already set this once when we enabled.
- }
+ setTargetFilter( ( String ) properties.get( m_dependencyMetadata.getTargetPropertyName() ) );
}
@@ -1885,7 +1860,7 @@ public class DependencyManager<S, T> imp
* @param target The new target filter to be set. This may be
* <code>null</code> if no target filtering is to be used.
*/
- private void setTargetFilter( String target) throws InvalidSyntaxException
+ private void setTargetFilter( String target)
{
// if configuration does not set filter, use the value from metadata
if (target == null)
@@ -1896,8 +1871,8 @@ public class DependencyManager<S, T> imp
if ( ( m_target == null && target == null ) || ( m_target != null && m_target.equals( target ) ) )
{
m_componentManager.log( LogService.LOG_DEBUG, "No change in target property for dependency {0}: currently registered: {1}", new Object[]
- {getName(), registered}, null );
- if (registered)
+ {getName(), m_tracker != null}, null );
+ if (m_tracker != null)
{
return;
}
@@ -1909,31 +1884,24 @@ public class DependencyManager<S, T> imp
filterString = "(&" + filterString + m_target + ")";
}
- SortedMap<ServiceReference<T>, RefPair<T>> refMap;
- if ( registered )
- {
- refMap = unregisterServiceListener();
- }
- else
- {
- refMap = new TreeMap<ServiceReference<T>, RefPair<T>>(Collections.reverseOrder());
- }
+ final ServiceTracker<T, RefPair<T>> oldTracker = m_tracker;
+ SortedMap<ServiceReference<T>, RefPair<T>> refMap = unregisterServiceListener();
m_componentManager.log( LogService.LOG_DEBUG, "Setting target property for dependency {0} to {1}", new Object[]
{getName(), target}, null );
BundleContext bundleContext = m_componentManager.getBundleContext();
if ( bundleContext != null )
{
- try
- {
+ try
+ {
m_targetFilter = bundleContext.createFilter( filterString );
- }
- catch ( InvalidSyntaxException ise )
- {
- m_componentManager.log( LogService.LOG_ERROR, "Invalid syntax in target property for dependency {0} to {1}", new Object[]
- {getName(), target}, null );
- // TODO this is an error, how do we recover?
- return; //avoid an NPE
- }
+ }
+ catch ( InvalidSyntaxException ise )
+ {
+ m_componentManager.log( LogService.LOG_ERROR, "Invalid syntax in target property for dependency {0} to {1}", new Object[]
+ {getName(), target}, null );
+ // TODO this is an error, how do we recover?
+ return; //avoid an NPE
+ }
}
else
{
@@ -1942,21 +1910,15 @@ public class DependencyManager<S, T> imp
return;
}
- registerServiceListener( bundleContext, refMap );
- }
-
- private void registerServiceListener( BundleContext bundleContext, SortedMap<ServiceReference<T>, RefPair<T>> refMap ) throws InvalidSyntaxException
- {
- final ServiceTracker<T, RefPair<T>> oldTracker = trackerRef.get();
- customizer.setPreviousRefMap( refMap );
+ m_customizer.setPreviousRefMap( refMap );
boolean initialActive = oldTracker != null && oldTracker.isActive();
- m_componentManager.log( LogService.LOG_INFO, "New service tracker for {0}, initial active: {1}", new Object[]
- {getName(), initialActive}, null );
- ServiceTracker<T, RefPair<T>> tracker = new ServiceTracker<T, RefPair<T>>( bundleContext, m_targetFilter, customizer, initialActive );
- customizer.setTracker( tracker );
- registered = true;
+ m_componentManager.log( LogService.LOG_INFO, "New service tracker for {0}, initial active: {1}, previous references: {2}", new Object[]
+ {getName(), initialActive, refMap}, null );
+ ServiceTracker<T, RefPair<T>> tracker = new ServiceTracker<T, RefPair<T>>( bundleContext, m_targetFilter, m_customizer, initialActive );
+ m_customizer.setTracker( tracker );
+ // m_registered = true;
tracker.open( m_componentManager.getTrackingCount() );
- customizer.setTrackerOpened();
+ m_customizer.setTrackerOpened();
if ( oldTracker != null )
{
oldTracker.completeClose( refMap );
@@ -2013,20 +1975,22 @@ public class DependencyManager<S, T> imp
SortedMap<ServiceReference<T>, RefPair<T>> unregisterServiceListener()
{
SortedMap<ServiceReference<T>, RefPair<T>> refMap;
- ServiceTracker<T, RefPair<T>> tracker = trackerRef.get();
-// trackerRef.set( null ); //???
+ ServiceTracker<T, RefPair<T>> tracker = m_tracker;
if ( tracker != null )
{
AtomicInteger trackingCount = new AtomicInteger( );
refMap = tracker.close( trackingCount );
+ m_tracker = null;
+ m_componentManager.log( LogService.LOG_DEBUG, "unregistering service listener for dependency {0}", new Object[]
+ {getName()}, null );
}
else
{
refMap = new TreeMap<ServiceReference<T>, RefPair<T>>(Collections.reverseOrder());
+ m_componentManager.log( LogService.LOG_DEBUG, " No existing service listener to unregister for dependency {0}", new Object[]
+ {getName()}, null );
}
- registered = false;
- m_componentManager.log( LogService.LOG_DEBUG, "unregistering service listener for dependency {0}", new Object[]
- {getName()}, null );
+// m_registered = false;
return refMap;
}
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=1522046&r1=1522045&r2=1522046&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 Wed Sep 11 21:13:50 2013
@@ -21,6 +21,7 @@ package org.apache.felix.scr.impl.manage
import java.util.Dictionary;
import java.util.Hashtable;
+import java.util.concurrent.CountDownLatch;
import java.util.concurrent.atomic.AtomicInteger;
import org.apache.felix.scr.impl.BundleComponentActivator;
@@ -528,81 +529,115 @@ public class ImmediateComponentManager<S
*/
public void reconfigure( Dictionary<String, Object> configuration, long changeCount, TargetedPID targetedPID )
{
- if ( targetedPID == null || !targetedPID.equals( m_targetedPID ) )
- {
- m_targetedPID = targetedPID;
- m_changeCount = -1;
- }
- if ( configuration != null )
+ CountDownLatch enableLatch = null;
+ try
{
- if ( changeCount <= m_changeCount )
+ enableLatch = enableLatchWait();
+ if ( targetedPID == null || !targetedPID.equals( m_targetedPID ) )
{
- log( LogService.LOG_DEBUG,
- "ImmediateComponentHolder out of order configuration updated for pid {0} with existing count {1}, new count {2}",
- new Object[] { getConfigurationPid(), m_changeCount, changeCount }, null );
+ m_targetedPID = targetedPID;
+ m_changeCount = -1;
+ }
+ if ( configuration != null )
+ {
+ if ( changeCount <= m_changeCount )
+ {
+ log( LogService.LOG_DEBUG,
+ "ImmediateComponentHolder out of order configuration updated for pid {0} with existing count {1}, new count {2}",
+ new Object[] { getConfigurationPid(), m_changeCount, changeCount }, null );
+ return;
+ }
+ m_changeCount = changeCount;
+ }
+ else
+ {
+ m_changeCount = -1;
+ }
+ // nothing to do if there is no configuration (see FELIX-714)
+ if ( configuration == null && m_configurationProperties == null )
+ {
+ log( LogService.LOG_DEBUG, "No configuration provided (or deleted), nothing to do", null );
return;
}
- m_changeCount = changeCount;
- }
- else
- {
- m_changeCount = -1;
- }
- // nothing to do if there is no configuration (see FELIX-714)
- if ( configuration == null && m_configurationProperties == null )
- {
- log( LogService.LOG_DEBUG, "No configuration provided (or deleted), nothing to do", null );
- return;
- }
- // store the properties
- m_configurationProperties = configuration;
+ // store the properties
+ m_configurationProperties = configuration;
- // clear the current properties to force using the configuration data
- m_properties = null;
+ // clear the current properties to force using the configuration data
+ m_properties = null;
- // 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 );
- updateTargets( getProperties() );
- activateInternal( getTrackingCount().get() );
- return;
- }
+ // 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() );
+ return;
+ }
- // reactivate the component to ensure it is provided with the
- // configuration data
- if ( ( getState() & ( STATE_ACTIVE | STATE_FACTORY | STATE_REGISTERED ) ) == 0 )
- {
- // 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 );
- updateTargets( getProperties() );
- return;
- }
+ // reactivate the component to ensure it is provided with the
+ // configuration data
+ if ( ( getState() & ( STATE_DISPOSED | STATE_DISABLED ) ) != 0 )
+ {
+ // 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;
+ }
- // if the configuration has been deleted but configuration is required
- // this component must be deactivated
- if ( configuration == null && getComponentMetadata().isConfigurationRequired() )
+ // if the configuration has been deleted but configuration is required
+ // this component must be deactivated
+ if ( configuration == null && getComponentMetadata().isConfigurationRequired() )
+ {
+ //deactivate and remove service listeners
+ deactivateInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED, true, getTrackingCount().get() );
+ //do not reset targets as that will reinstall the service listeners which may activate the component.
+ //when a configuration arrives the properties will get set based on the new configuration.
+ return;
+ }
+ if ( !modify() )
+ {
+ // SCR 112.7.1 - deactivate if configuration is deleted or no modified method declared
+ log( LogService.LOG_DEBUG, "Deactivating and Activating to reconfigure from configuration", null );
+ int reason = ( configuration == null ) ? ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED
+ : ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_MODIFIED;
+
+ // FELIX-2368: cycle component immediately, reconfigure() is
+ // called through ConfigurationListener API which itself is
+ // called asynchronously by the Configuration Admin Service
+ deactivateInternal( reason, false, getTrackingCount().get() );
+ //do not allow reactivation before all targets are reset
+ m_internalEnabled = false;
+ try
+ {
+ updateTargets( getProperties() );
+ }
+ finally
+ {
+ m_internalEnabled = true;
+ }
+ activateInternal( getTrackingCount().get() );
+ }
+ }
+ catch ( InterruptedException e )
{
- deactivateInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED, true, getTrackingCount().get() );
- updateTargets( getProperties() );
- return;
+ Thread.currentThread().interrupt();
}
- if ( !modify() )
+ finally
{
- // 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() );
- updateTargets( getProperties() );
- activateInternal( getTrackingCount().get() );
+ enableLatch.countDown();
}
}
@@ -640,6 +675,7 @@ public class ImmediateComponentManager<S
obtainWriteLock( "ImmediateComponentManager.modify" );
try
{
+ //cf 112.5.12 where invoking modified method before updating target services is specified.
final MethodResult result = invokeModifiedMethod();
updateTargets( props );
if ( result == null )