FELIX-4011 fix a lot of potential NPEs during bundle shutdown when BundleComponentActivator and BundleContext may not be available
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1465203 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/pom.xml b/scr/pom.xml
index 6d90324..d971a7b 100644
--- a/scr/pom.xml
+++ b/scr/pom.xml
@@ -205,7 +205,7 @@
<dependency>
<groupId>org.codehaus.mojo</groupId>
<artifactId>animal-sniffer-annotations</artifactId>
- <version>1.10-SNAPSHOT</version>
+ <version>1.9</version>
<scope>compile</scope>
</dependency>
</dependencies>
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/ScrConfiguration.java b/scr/src/main/java/org/apache/felix/scr/impl/config/ScrConfiguration.java
index c1adcee..3318c46 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/ScrConfiguration.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/ScrConfiguration.java
@@ -66,6 +66,8 @@
public static final String PROP_LOCK_TIMEOUT = "ds.lock.timeout.milliseconds";
+ public static final long DEFAULT_LOCK_TIMEOUT_MILLISECONDS = 5000;
+
public static final String PROP_LOGLEVEL = "ds.loglevel";
private static final String LOG_LEVEL_DEBUG = "debug";
@@ -88,7 +90,7 @@
private boolean infoAsService;
- private long lockTimeout = 5000;//milliseconds
+ private long lockTimeout = DEFAULT_LOCK_TIMEOUT_MILLISECONDS;
private BundleContext bundleContext;
@@ -143,7 +145,7 @@
factoryEnabled = false;
keepInstances = false;
infoAsService = false;
- lockTimeout = 5000;
+ lockTimeout = DEFAULT_LOCK_TIMEOUT_MILLISECONDS;
}
else
{
@@ -161,7 +163,7 @@
keepInstances = VALUE_TRUE.equalsIgnoreCase( String.valueOf( config.get( PROP_DELAYED_KEEP_INSTANCES ) ) );
infoAsService = VALUE_TRUE.equalsIgnoreCase( String.valueOf( config.get( PROP_INFO_SERVICE) ) );
Long timeout = ( Long ) config.get( PROP_LOCK_TIMEOUT );
- lockTimeout = timeout == null? 5000: timeout;
+ lockTimeout = timeout == null? DEFAULT_LOCK_TIMEOUT_MILLISECONDS: timeout;
}
if ( scrCommand != null )
{
@@ -227,7 +229,7 @@
String val = bundleContext.getProperty( PROP_LOCK_TIMEOUT);
if ( val == null)
{
- return 5000;
+ return DEFAULT_LOCK_TIMEOUT_MILLISECONDS;
}
return Long.parseLong( val );
}
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 d629b4a..519023f 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
@@ -41,6 +41,7 @@
import org.apache.felix.scr.Component;
import org.apache.felix.scr.Reference;
import org.apache.felix.scr.impl.BundleComponentActivator;
+import org.apache.felix.scr.impl.config.ScrConfiguration;
import org.apache.felix.scr.impl.helper.ComponentMethods;
import org.apache.felix.scr.impl.helper.MethodResult;
import org.apache.felix.scr.impl.helper.SimpleLogger;
@@ -192,7 +193,12 @@
private long getLockTimeout()
{
- return getActivator().getConfiguration().lockTimeout();
+ BundleComponentActivator activator = getActivator();
+ if ( activator != null )
+ {
+ return activator.getConfiguration().lockTimeout();
+ }
+ return ScrConfiguration.DEFAULT_LOCK_TIMEOUT_MILLISECONDS;
}
final void releaseWriteLock( String source )
@@ -490,6 +496,15 @@
disposed = true;
disposeInternal( reason );
}
+
+ <T> void registerMissingDependency( DependencyManager<S, T> dm, ServiceReference<T> ref, int trackingCount)
+ {
+ BundleComponentActivator activator = getActivator();
+ if ( activator != null )
+ {
+ activator.registerMissingDependency( dm, ref, trackingCount );
+ }
+ }
//---------- Component interface ------------------------------------------
@@ -509,25 +524,30 @@
*/
public Bundle getBundle()
{
+ final BundleContext context = getBundleContext();
+ if ( context != null )
+ {
+ try
+ {
+ return context.getBundle();
+ }
+ catch ( IllegalStateException ise )
+ {
+ // if the bundle context is not valid any more
+ }
+ }
+ // already disposed off component or bundle context is invalid
+ return null;
+ }
+
+ BundleContext getBundleContext()
+ {
final BundleComponentActivator activator = getActivator();
if ( activator != null )
{
- final BundleContext context = activator.getBundleContext();
- if ( context != null )
- {
- try
- {
- return context.getBundle();
- }
- catch ( IllegalStateException ise )
- {
- // if the bundle context is not valid any more
- }
- }
+ return activator.getBundleContext();
}
-
- // already disposed off component or bundle context is invalid
- return null;
+ return null;
}
@@ -728,8 +748,13 @@
@Override
ServiceRegistration<S> register(String[] services)
{
+ BundleContext bundleContext = getBundleContext();
+ if (bundleContext == null)
+ {
+ return null;
+ }
final Dictionary<String, Object> serviceProperties = getServiceProperties();
- ServiceRegistration<S> serviceRegistration = ( ServiceRegistration<S> ) getActivator().getBundleContext()
+ ServiceRegistration<S> serviceRegistration = ( ServiceRegistration<S> ) bundleContext
.registerService( services, getService(), serviceProperties );
return serviceRegistration;
}
@@ -792,10 +817,16 @@
{
return true;
}
+ final Bundle bundle = getBundle();
+ if (bundle == null)
+ {
+ log( LogService.LOG_ERROR, "bundle shut down while trying to load implementation object class", null );
+ return false;
+ }
Class<?> implementationObjectClass;
try
{
- implementationObjectClass = getActivator().getBundleContext().getBundle().loadClass(
+ implementationObjectClass = bundle.loadClass(
getComponentMetadata().getImplementationClassName() );
}
catch ( ClassNotFoundException e )
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
index cda6030..58dd806 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
@@ -23,6 +23,7 @@
import java.util.Dictionary;
import org.apache.felix.scr.component.ExtComponentContext;
+import org.apache.felix.scr.impl.BundleComponentActivator;
import org.apache.felix.scr.impl.helper.ReadOnlyDictionary;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
@@ -112,7 +113,7 @@
public BundleContext getBundleContext()
{
- return m_componentManager.getActivator().getBundleContext();
+ return m_componentManager.getBundleContext();
}
@@ -130,13 +131,21 @@
public void enableComponent( String name )
{
- m_componentManager.getActivator().enableComponent( name );
+ BundleComponentActivator activator = m_componentManager.getActivator();
+ if ( activator != null )
+ {
+ activator.enableComponent( name );
+ }
}
public void disableComponent( String name )
{
- m_componentManager.getActivator().disableComponent( name );
+ BundleComponentActivator activator = m_componentManager.getActivator();
+ if ( activator != null )
+ {
+ activator.disableComponent( name );
+ }
}
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 e41c26a..92398a7 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
@@ -38,6 +38,7 @@
import org.apache.felix.scr.Component;
import org.apache.felix.scr.Reference;
import org.apache.felix.scr.impl.BundleComponentActivator;
+import org.apache.felix.scr.impl.helper.BindMethod;
import org.apache.felix.scr.impl.helper.BindMethods;
import org.apache.felix.scr.impl.helper.MethodResult;
import org.apache.felix.scr.impl.metadata.ReferenceMetadata;
@@ -214,7 +215,11 @@
if ( ref.getServiceObject() != null )
{
ref.setServiceObject( null );
- m_componentManager.getActivator().getBundleContext().ungetService( ref.getRef() );
+ BundleContext bundleContext = m_componentManager.getBundleContext();
+ if ( bundleContext != null )
+ {
+ bundleContext.ungetService( ref.getRef() );
+ }
}
}
}
@@ -291,7 +296,7 @@
}
if (isActive())
{
- m_bindMethods.getBind().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext(), m_componentManager );
+ getServiceObject( m_bindMethods.getBind(), refPair );
}
return refPair;
}
@@ -309,7 +314,7 @@
m_componentManager.invokeBindMethod( DependencyManager.this, refPair, trackingCount );
}
else {
- m_componentManager.getActivator().registerMissingDependency( DependencyManager.this, serviceReference, trackingCount );
+ m_componentManager.registerMissingDependency( DependencyManager.this, serviceReference, trackingCount );
}
}
else if ( isTrackerOpened() && !isOptional() )
@@ -376,13 +381,13 @@
{
synchronized (refPair)
{
- if (m_bindMethods.getBind().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext(), m_componentManager ))
+ if (getServiceObject( m_bindMethods.getBind(), refPair ))
{
success = true;
}
else
{
- m_componentManager.getActivator().registerMissingDependency( DependencyManager.this, refPair.getRef(), trackingCount.get() );
+ m_componentManager.registerMissingDependency( DependencyManager.this, refPair.getRef(), trackingCount.get() );
}
}
}
@@ -427,7 +432,7 @@
RefPair<T> refPair = new RefPair<T>( serviceReference );
if (isActive())
{
- m_bindMethods.getBind().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext(), m_componentManager );
+ getServiceObject( m_bindMethods.getBind(), refPair );
}
return refPair;
}
@@ -491,7 +496,7 @@
{
synchronized (refPair)
{
- success |= m_bindMethods.getBind().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext(), m_componentManager );
+ success |= getServiceObject( m_bindMethods.getBind(), refPair );
}
}
return success;
@@ -584,7 +589,7 @@
{
synchronized (refPair)
{
- success |= m_bindMethods.getBind().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext(), m_componentManager );
+ success |= getServiceObject( m_bindMethods.getBind(), refPair );
}
}
return success;
@@ -596,7 +601,7 @@
{
synchronized (refPair)
{
- success |= m_bindMethods.getBind().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext(), m_componentManager );
+ success |= getServiceObject( m_bindMethods.getBind(), refPair );
}
refs.add(refPair) ;
}
@@ -665,7 +670,7 @@
{
synchronized ( refPair )
{
- m_bindMethods.getBind().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext(), m_componentManager );
+ getServiceObject( m_bindMethods.getBind(), refPair );
}
if ( !refPair.isFailed() )
{
@@ -678,7 +683,7 @@
}
else
{
- m_componentManager.getActivator().registerMissingDependency( DependencyManager.this, serviceReference, trackingCount );
+ m_componentManager.registerMissingDependency( DependencyManager.this, serviceReference, trackingCount );
}
this.refPair = refPair;
}
@@ -725,7 +730,7 @@
nextRefPair = tracked.values().iterator().next();
synchronized ( nextRefPair )
{
- if (!m_bindMethods.getBind().getServiceObject( nextRefPair, m_componentManager.getActivator().getBundleContext(), m_componentManager ))
+ if (!getServiceObject( m_bindMethods.getBind(), nextRefPair ))
{
//TODO error???
}
@@ -780,11 +785,11 @@
RefPair<T> refPair = tracked.values().iterator().next();
synchronized ( refPair )
{
- success |= m_bindMethods.getBind().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext(), m_componentManager );
+ success |= getServiceObject( m_bindMethods.getBind(), refPair );
}
if (refPair.isFailed())
{
- m_componentManager.getActivator().registerMissingDependency( DependencyManager.this, refPair.getRef(), trackingCount.get() );
+ m_componentManager.registerMissingDependency( DependencyManager.this, refPair.getRef(), trackingCount.get() );
}
this.refPair = refPair;
}
@@ -891,7 +896,7 @@
RefPair<T> refPair = tracked.values().iterator().next();
synchronized ( refPair )
{
- success |= m_bindMethods.getBind().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext(), m_componentManager );
+ success |= getServiceObject( m_bindMethods.getBind(), refPair );
}
this.refPair = refPair;
}
@@ -1225,9 +1230,17 @@
}
T serviceObject;
// otherwise acquire the service
+ final BundleContext bundleContext = m_componentManager.getBundleContext();
+ if (bundleContext == null)
+ {
+ m_componentManager.log( LogService.LOG_ERROR, "Bundle shut down while getting service {0} ({1}/{2,number,#})", new Object[]
+ { getName(), m_dependencyMetadata.getInterface(),
+ refPair.getRef().getProperty( Constants.SERVICE_ID ) }, null );
+ return null;
+ }
try
{
- serviceObject = m_componentManager.getActivator().getBundleContext().getService( refPair.getRef() );
+ serviceObject = bundleContext.getService( refPair.getRef() );
}
catch ( Exception e )
{
@@ -1427,7 +1440,7 @@
//something else got the reference and may be binding it.
return;
}
- m_bindMethods.getBind().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext(), m_componentManager );
+ getServiceObject( m_bindMethods.getBind(), refPair );
}
m_componentManager.invokeBindMethod( this, refPair, trackingCount );
}
@@ -1526,7 +1539,7 @@
return;
}
}
- if ( !m_bindMethods.getUpdated().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext(), m_componentManager ))
+ if ( !getServiceObject( m_bindMethods.getUpdated(), refPair ))
{
m_componentManager.log( LogService.LOG_WARNING,
"DependencyManager : invokeUpdatedMethod : Service not available from service registry for ServiceReference {0} for reference {1}",
@@ -1598,7 +1611,7 @@
"DependencyManager : invokeUnbindMethod : Component set, but reference not present", null );
return;
}
- if ( !m_bindMethods.getUnbind().getServiceObject( refPair, m_componentManager.getActivator().getBundleContext(), m_componentManager ))
+ if ( !getServiceObject( m_bindMethods.getUnbind(), refPair ))
{
m_componentManager.log( LogService.LOG_WARNING,
"DependencyManager : invokeUnbindMethod : Service not available from service registry for ServiceReference {0} for reference {1}",
@@ -1764,29 +1777,39 @@
}
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
{
- m_targetFilter = m_componentManager.getActivator().getBundleContext().createFilter( filterString );
+ 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?
- m_targetFilter = null;
+ return; //avoid an NPE
+ }
+ }
+ else
+ {
+ m_componentManager.log( LogService.LOG_ERROR, "Bundle is shut down for dependency {0} to {1}", new Object[]
+ {getName(), target}, null );
+ return;
}
- registerServiceListener( refMap );
+ registerServiceListener( bundleContext, refMap );
}
- private void registerServiceListener( SortedMap<ServiceReference<T>, RefPair<T>> refMap ) throws InvalidSyntaxException
+ private void registerServiceListener( BundleContext bundleContext, SortedMap<ServiceReference<T>, RefPair<T>> refMap ) throws InvalidSyntaxException
{
final ServiceTracker<T, RefPair<T>> oldTracker = trackerRef.get();
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>>( m_componentManager.getActivator().getBundleContext(), m_targetFilter, customizer, initialActive );
+ ServiceTracker<T, RefPair<T>> tracker = new ServiceTracker<T, RefPair<T>>( bundleContext, m_targetFilter, customizer, initialActive );
customizer.setTracker( tracker );
registered = true;
tracker.open( m_componentManager.getTrackingCount() );
@@ -1882,4 +1905,18 @@
{
return "DependencyManager: Component [" + m_componentManager + "] reference [" + getName() + "]";
}
+
+ boolean getServiceObject(BindMethod bindMethod, RefPair<T> refPair)
+ {
+ BundleContext bundleContext = m_componentManager.getBundleContext();
+ if ( bundleContext != null )
+ {
+ return bindMethod.getServiceObject( refPair, bundleContext, m_componentManager );
+ }
+ else
+ {
+ refPair.setFailed();
+ return false;
+ }
+ }
}
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
index 7ddc962..0c17fd5 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
@@ -136,7 +136,11 @@
log( LogService.LOG_DEBUG, "Set implementation object for component {0}", new Object[] { getName() }, null );
//notify that component was successfully created so any optional circular dependencies can be retried
- getActivator().missingServicePresent( getServiceReference() );
+ BundleComponentActivator activator = getActivator();
+ if ( activator != null )
+ {
+ activator.missingServicePresent( getServiceReference() );
+ }
}
return true;
}
@@ -216,10 +220,16 @@
// 1. Load the component implementation class
// 2. Create the component instance and component context
// If the component is not immediate, this is not done at this moment
+ Bundle bundle = getBundle();
+ if (bundle == null)
+ {
+ log( LogService.LOG_WARNING, "Bundle shut down during instantiation of the implementation object", null);
+ return null;
+ }
try
{
// 112.4.4 The class is retrieved with the loadClass method of the component's bundle
- implementationObjectClass = (Class<S>) getActivator().getBundleContext().getBundle().loadClass(
+ implementationObjectClass = (Class<S>) bundle.loadClass(
getComponentMetadata().getImplementationClassName() ) ;
// 112.4.4 The class must be public and have a public constructor without arguments so component instances
@@ -754,7 +764,7 @@
// unget the service instance if no bundle is using it
// any longer unless delayed component instances have to
// be kept (FELIX-3039)
- if ( useCount == 0 && !isImmediate() && !getActivator().getConfiguration().keepInstances() )
+ if ( useCount == 0 && !isImmediate() && !keepInstances() )
{
obtainWriteLock( "ImmediateComponentManager.ungetService.1" );
try
@@ -773,6 +783,11 @@
}
}
+ private boolean keepInstances()
+ {
+ return getActivator() != null && getActivator().getConfiguration().keepInstances();
+ }
+
public long getChangeCount()
{
return m_changeCount;