FELIX-4287 fix NPE when dispose called after bundle stopped, simplify deactivate method calls
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1534395 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java b/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
index 1b086d1..315137c 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
@@ -54,25 +54,25 @@
public class BundleComponentActivator implements Logger
{
// global component registration
- private ComponentRegistry m_componentRegistry;
+ private final ComponentRegistry m_componentRegistry;
// The bundle context owning the registered component
- private BundleContext m_context = null;
+ private final BundleContext m_context;
// This is a list of component instance managers that belong to a particular bundle
private List<ComponentHolder> m_managers = new ArrayList<ComponentHolder>();
// The Configuration Admin tracker providing configuration for components
- private ServiceTracker m_logService;
+ private final ServiceTracker m_logService;
// thread acting upon configurations
- private ComponentActorThread m_componentActor;
+ private final ComponentActorThread m_componentActor;
// true as long as the dispose method is not called
private boolean m_active;
// the configuration
- private ScrConfiguration m_configuration;
+ private final ScrConfiguration m_configuration;
/**
@@ -316,14 +316,15 @@
*/
void dispose( int reason )
{
- if ( m_context == null )
+ synchronized ( this )
{
- return;
+ if ( !m_active )
+ {
+ return;
+ }
+ // mark instance inactive (no more component activations)
+ m_active = false;
}
-
- // mark instance inactive (no more component activations)
- m_active = false;
-
log( LogService.LOG_DEBUG, "BundleComponentActivator : Bundle [{0}] will destroy {1} instances", new Object[]
{ m_context.getBundle().getBundleId(), m_managers.size() }, null, null, null );
@@ -351,14 +352,8 @@
log( LogService.LOG_DEBUG, "BundleComponentActivator : Bundle [{0}] STOPPED", new Object[]
{m_context.getBundle().getBundleId()}, null, null, null );
- if (m_logService != null) {
- m_logService.close();
- m_logService = null;
- }
+ m_logService.close();
- m_componentActor = null;
- m_componentRegistry = null;
- m_context = null;
}
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 e812dc2..a8c895e 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
@@ -513,7 +513,7 @@
enableLatch = enableLatchWait();
if ( !async )
{
- deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED, true, m_trackingCount.get() );
+ deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED, true, false );
}
disableInternal();
}
@@ -538,7 +538,7 @@
{
try
{
- deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED, true, m_trackingCount.get() );
+ deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED, true, false );
}
finally
{
@@ -572,8 +572,7 @@
*/
public void dispose( int reason )
{
- m_disposed = true;
- disposeInternal( reason );
+ deactivateInternal( reason, true, true );
}
<T> void registerMissingDependency( DependencyManager<S, T> dm, ServiceReference<T> ref, int trackingCount)
@@ -834,16 +833,22 @@
}
}
- final void deactivateInternal( int reason, boolean disable, int trackingCount )
+ /**
+ * Handles deactivating, disabling, and disposing a component manager. Deactivating a factory instance
+ * always disables and disposes it. Deactivating a factory disposes it.
+ * @param reason reason for action
+ * @param disable whether to also disable the manager
+ * @param dispose whether to also dispose of the manager
+ */
+ final void deactivateInternal( int reason, boolean disable, boolean dispose )
{
- if ( m_disposed )
+ synchronized ( this )
{
- return;
- }
- if ( m_factoryInstance )
- {
- disposeInternal( reason );
- return;
+ if ( m_disposed )
+ {
+ return;
+ }
+ m_disposed = dispose;
}
log( LogService.LOG_DEBUG, "Deactivating component", null );
@@ -852,45 +857,20 @@
obtainActivationReadLock( "deactivateInternal" );
try
{
- doDeactivate( reason, disable );
+ doDeactivate( reason, disable || m_factoryInstance );
}
finally
{
releaseActivationReadLock( "deactivateInternal" );
}
- if ( isFactory() )
+ if ( isFactory() || m_factoryInstance || dispose )
{
+ log( LogService.LOG_DEBUG, "Disposing component (reason: " + reason + ")", null );
clear();
}
}
- final void disableInternal()
- {
- m_internalEnabled = false;
- if ( m_disposed )
- {
- throw new IllegalStateException( "Cannot disable a disposed component " + getName() );
- }
- unregisterComponentId();
- }
-
- /**
- * Disposes off this component deactivating and disabling it first as
- * required. After disposing off the component, it may not be used anymore.
- * <p>
- * This method unlike the other state change methods immediately takes
- * action and disposes the component. The reason for this is, that this
- * method has to actually complete before other actions like bundle stopping
- * may continue.
- */
- final void disposeInternal( int reason )
- {
- log( LogService.LOG_DEBUG, "Disposing component (reason: " + reason + ")", null );
- doDeactivate( reason, true );
- clear();
- }
-
- final void doDeactivate( int reason, boolean disable )
+ private void doDeactivate( int reason, boolean disable )
{
try
{
@@ -924,6 +904,16 @@
}
}
+ final void disableInternal()
+ {
+ m_internalEnabled = false;
+ if ( m_disposed )
+ {
+ throw new IllegalStateException( "Cannot disable a disposed component " + getName() );
+ }
+ unregisterComponentId();
+ }
+
final ServiceReference<S> getServiceReference()
{
ServiceRegistration<S> reg = getServiceRegistration();
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
index 65e4d96..c8f20a1 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
@@ -129,7 +129,7 @@
if ( instance == null || instance.getInstance() == null )
{
// activation failed, clean up component manager
- cm.disposeInternal( ComponentConstants.DEACTIVATION_REASON_DISPOSED );
+ cm.dispose( ComponentConstants.DEACTIVATION_REASON_DISPOSED );
throw new ComponentException( "Failed activating component" );
}
@@ -313,7 +313,7 @@
if ( ( getState() & STATE_DISPOSED ) == 0 && getComponentMetadata().isConfigurationRequired() )
{
log( LogService.LOG_DEBUG, "Deactivating component factory (required configuration has gone)", null );
- deactivateInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED, true, getTrackingCount().get() );
+ deactivateInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED, true, false );
}
}
else
@@ -381,7 +381,7 @@
{
log( LogService.LOG_DEBUG,
"Component Factory target filters not satisfied anymore: deactivating", null );
- deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, getTrackingCount().get() );
+ deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
return false;
}
}
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 a1aa66f..f75f41c 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
@@ -263,7 +263,7 @@
{
if (getTracker().isEmpty())
{
- m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, trackingCount );
+ m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
}
}
}
@@ -362,7 +362,7 @@
lastRefPair = refPair;
lastRefPairTrackingCount = trackingCount;
tracked( trackingCount );
- m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, trackingCount );
+ m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
lastRefPair = null;
m_componentManager.log( LogService.LOG_DEBUG, "dm {0} tracking {1} MultipleDynamic removed (deactivate) {2}", new Object[] {getName(), trackingCount, serviceReference}, null );
}
@@ -440,7 +440,7 @@
m_componentManager.log( LogService.LOG_DEBUG,
"Dependency Manager: Static dependency on {0}/{1} is broken", new Object[]
{getName(), m_dependencyMetadata.getInterface()}, null );
- m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, trackingCount );
+ m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
m_componentManager.activateInternal( trackingCount );
}
@@ -472,7 +472,7 @@
m_componentManager.log( LogService.LOG_DEBUG,
"Dependency Manager: Static dependency on {0}/{1} is broken", new Object[]
{getName(), m_dependencyMetadata.getInterface()}, null );
- m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, trackingCount );
+ m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
//try to reactivate after ref is no longer tracked.
m_componentManager.activateInternal( trackingCount );
}
@@ -481,7 +481,7 @@
m_componentManager.log( LogService.LOG_DEBUG,
"Dependency Manager: Static dependency on {0}/{1} is broken", new Object[]
{getName(), m_dependencyMetadata.getInterface()}, null );
- m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, trackingCount );
+ m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
}
//This is unlikely
ungetService( refPair );
@@ -572,7 +572,7 @@
m_componentManager.log( LogService.LOG_DEBUG,
"Dependency Manager: Static dependency on {0}/{1} is broken", new Object[]
{ getName(), m_dependencyMetadata.getInterface() }, null );
- m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, trackingCount );
+ m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
// FELIX-2368: immediately try to reactivate
m_componentManager.activateInternal( trackingCount );
@@ -584,7 +584,7 @@
m_componentManager.log( LogService.LOG_DEBUG,
"Dependency Manager: Static dependency on {0}/{1} is broken", new Object[]
{getName(), m_dependencyMetadata.getInterface()}, null );
- m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, trackingCount );
+ m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
}
ungetService( refPair );
m_componentManager.log( LogService.LOG_DEBUG, "dm {0} tracking {1} MultipleStaticReluctant removed {2} (exit)", new Object[] {getName(), trackingCount, serviceReference}, null );
@@ -792,7 +792,7 @@
this.trackingCount = trackingCount;
tracked( trackingCount );
untracked = false;
- m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, trackingCount );
+ m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
}
if ( oldRefPair != null )
{
@@ -894,7 +894,7 @@
}
if ( reactivate )
{
- m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, trackingCount );
+ m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
m_componentManager.activateInternal( trackingCount );
}
else
@@ -942,7 +942,7 @@
}
if ( reactivate )
{
- m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, trackingCount );
+ m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false, false );
m_componentManager.activateInternal( trackingCount );
}
m_componentManager.log( LogService.LOG_DEBUG, "dm {0} tracking {1} SingleStatic removed {2} (exit)", new Object[] {getName(), trackingCount, serviceReference}, null );
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java
index ffedf46..dc72b8c 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java
@@ -587,7 +587,7 @@
if ( configuration == null && getComponentMetadata().isConfigurationRequired() )
{
//deactivate and remove service listeners
- deactivateInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED, true, getTrackingCount().get() );
+ deactivateInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED, true, false );
//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;
@@ -619,7 +619,7 @@
// called through ConfigurationListener API which itself is
// called asynchronously by the Configuration Admin Service
releaseActivationWriteeLock( "reconfigure.modified.1" );;
- deactivateInternal( reason, false, getTrackingCount().get() );
+ deactivateInternal( reason, false, false );
obtainActivationWriteLock( "reconfigure.deactivate.activate" );
try
{