Return-Path: X-Original-To: apmail-felix-commits-archive@www.apache.org Delivered-To: apmail-felix-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 787B21085A for ; Mon, 21 Oct 2013 22:12:08 +0000 (UTC) Received: (qmail 27534 invoked by uid 500); 21 Oct 2013 22:12:07 -0000 Delivered-To: apmail-felix-commits-archive@felix.apache.org Received: (qmail 27349 invoked by uid 500); 21 Oct 2013 22:12:05 -0000 Mailing-List: contact commits-help@felix.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@felix.apache.org Delivered-To: mailing list commits@felix.apache.org Received: (qmail 27303 invoked by uid 99); 21 Oct 2013 22:12:04 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 21 Oct 2013 22:12:04 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 21 Oct 2013 22:11:59 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id E8FEC238899C; Mon, 21 Oct 2013 22:11:37 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1534395 - in /felix/trunk/scr/src/main/java/org/apache/felix/scr/impl: BundleComponentActivator.java manager/AbstractComponentManager.java manager/ComponentFactoryImpl.java manager/DependencyManager.java manager/SingleComponentManager.java Date: Mon, 21 Oct 2013 22:11:37 -0000 To: commits@felix.apache.org From: djencks@apache.org X-Mailer: svnmailer-1.0.9 Message-Id: <20131021221137.E8FEC238899C@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: djencks Date: Mon Oct 21 22:11:37 2013 New Revision: 1534395 URL: http://svn.apache.org/r1534395 Log: FELIX-4287 fix NPE when dispose called after bundle stopped, simplify deactivate method calls Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.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/SingleComponentManager.java 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=1534395&r1=1534394&r2=1534395&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 Mon Oct 21 22:11:37 2013 @@ -54,25 +54,25 @@ import org.osgi.util.tracker.ServiceTrac 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 m_managers = new ArrayList(); // 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 @@ public class BundleComponentActivator im */ 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 @@ public class BundleComponentActivator im 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; } 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=1534395&r1=1534394&r2=1534395&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 Mon Oct 21 22:11:37 2013 @@ -513,7 +513,7 @@ public abstract class AbstractComponentM 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 @@ public abstract class AbstractComponentM { try { - deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED, true, m_trackingCount.get() ); + deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED, true, false ); } finally { @@ -572,8 +572,7 @@ public abstract class AbstractComponentM */ public void dispose( int reason ) { - m_disposed = true; - disposeInternal( reason ); + deactivateInternal( reason, true, true ); } void registerMissingDependency( DependencyManager dm, ServiceReference ref, int trackingCount) @@ -834,16 +833,22 @@ public abstract class AbstractComponentM } } - 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 ) - { - return; - } - if ( m_factoryInstance ) + synchronized ( this ) { - disposeInternal( reason ); - return; + if ( m_disposed ) + { + return; + } + m_disposed = dispose; } log( LogService.LOG_DEBUG, "Deactivating component", null ); @@ -852,45 +857,20 @@ public abstract class AbstractComponentM 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. - *

- * 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 @@ public abstract class AbstractComponentM } } + final void disableInternal() + { + m_internalEnabled = false; + if ( m_disposed ) + { + throw new IllegalStateException( "Cannot disable a disposed component " + getName() ); + } + unregisterComponentId(); + } + final ServiceReference getServiceReference() { ServiceRegistration reg = getServiceRegistration(); 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=1534395&r1=1534394&r2=1534395&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 Mon Oct 21 22:11:37 2013 @@ -129,7 +129,7 @@ public class ComponentFactoryImpl ext 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 @@ public class ComponentFactoryImpl ext 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 @@ public class ComponentFactoryImpl ext { 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; } } 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=1534395&r1=1534394&r2=1534395&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 Mon Oct 21 22:11:37 2013 @@ -263,7 +263,7 @@ public class DependencyManager imp { 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 @@ public class DependencyManager imp 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 @@ public class DependencyManager imp 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 @@ public class DependencyManager imp 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 @@ public class DependencyManager imp 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 @@ public class DependencyManager imp 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 @@ public class DependencyManager imp 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 @@ public class DependencyManager imp 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 @@ public class DependencyManager imp } 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 @@ public class DependencyManager imp } 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 ); Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java?rev=1534395&r1=1534394&r2=1534395&view=diff ============================================================================== --- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java (original) +++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java Mon Oct 21 22:11:37 2013 @@ -587,7 +587,7 @@ public class SingleComponentManager e 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 @@ public class SingleComponentManager e // 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 {