FELIX-4585 fix component factory disable
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1615283 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java b/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java
index c87f146..80043dd 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/config/ConfigurableComponentHolder.java
@@ -19,6 +19,7 @@
package org.apache.felix.scr.impl.config;
+import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -142,6 +143,9 @@
* enabled. Otherwise they are not enabled immediately.
*/
private volatile boolean m_enabled;
+ private final Object enableLock = new Object();
+ private Promise<Void> m_enablePromise;
+ private Promise<Void> m_disablePromise = Promises.resolved(null);
private final ComponentMethods m_componentMethods;
@@ -589,7 +593,7 @@
{
synchronized ( m_components )
{
- return getComponentManagers( false );
+ return getComponentManagers( );
}
}
@@ -598,55 +602,106 @@
return m_enabled;
}
+
+ private void wait(Promise<Void> promise)
+ {
+ boolean waited = false;
+ boolean interrupted = false;
+ while ( !waited )
+ {
+ try
+ {
+ promise.getValue();
+ waited = true;
+ }
+ catch ( InterruptedException e )
+ {
+ interrupted = true;
+ }
+ catch (InvocationTargetException e)
+ {
+ //this is not going to happen
+ }
+ }
+ if ( interrupted )
+ {
+ Thread.currentThread().interrupt();
+ }
+ }
+
public Promise<Void> enableComponents( final boolean async )
{
- List<AbstractComponentManager<S>> cms = new ArrayList<AbstractComponentManager<S>>();
- synchronized ( m_components )
+ synchronized (enableLock)
{
- if ( isSatisfied() )
+ if ( m_enablePromise != null)
{
- if ( m_factoryPidIndex == null || m_componentMetadata.isObsoleteFactoryComponentFactory())
+ return m_enablePromise;
+ }
+ wait( m_disablePromise );
+
+
+ List<AbstractComponentManager<S>> cms = new ArrayList<AbstractComponentManager<S>>();
+ synchronized ( m_components )
+ {
+ if ( isSatisfied() )
{
- m_singleComponent = createComponentManager(false);
- cms.add( m_singleComponent );
- m_singleComponent.reconfigure(mergeProperties( null ), false);
- }
- if ( m_factoryPidIndex != null)
- {
- for (String pid: m_factoryConfigurations.keySet()) {
- AbstractComponentManager<S> scm = createComponentManager(true);
- m_components.put(pid, scm);
- scm.reconfigure( mergeProperties( pid ), false);
- cms.add( scm );
+ if ( m_factoryPidIndex == null || m_componentMetadata.isObsoleteFactoryComponentFactory())
+ {
+ m_singleComponent = createComponentManager(false);
+ cms.add( m_singleComponent );
+ m_singleComponent.reconfigure(mergeProperties( null ), false);
+ }
+ if ( m_factoryPidIndex != null)
+ {
+ for (String pid: m_factoryConfigurations.keySet()) {
+ AbstractComponentManager<S> scm = createComponentManager(true);
+ m_components.put(pid, scm);
+ scm.reconfigure( mergeProperties( pid ), false);
+ cms.add( scm );
+ }
}
}
+ m_enabled = true;
}
- m_enabled = true;
+ List<Promise<Void>> promises = new ArrayList<Promise<Void>>();
+ for ( AbstractComponentManager<S> cm : cms )
+ {
+ promises.add(cm.enable( async ));
+ }
+ m_enablePromise = new Deferred<List<Void>>().resolveWith(Promises.all(promises));
+ m_disablePromise = null;
+ return m_enablePromise;
}
- List<Promise<Void>> promises = new ArrayList<Promise<Void>>();
- for ( AbstractComponentManager<S> cm : cms )
- {
- promises.add(cm.enable( async ));
- }
- return new Deferred().resolveWith(Promises.all(promises));
}
public Promise<Void> disableComponents( final boolean async )
{
- List<AbstractComponentManager<S>> cms;
- synchronized ( m_components )
+ synchronized (enableLock)
{
- m_enabled = false;
+ if ( m_disablePromise != null)
+ {
+ return m_disablePromise;
+ }
+ wait( m_enablePromise );
- cms = getComponentManagers( true );
+ List<AbstractComponentManager<S>> cms;
+ synchronized ( m_components )
+ {
+ m_enabled = false;
+
+ cms = getDirectComponentManagers( );
+ clearComponents();
+ }
+ List<Promise<Void>> promises = new ArrayList<Promise<Void>>();
+ for ( AbstractComponentManager<S> cm : cms )
+ {
+ promises.add(cm.disable( async ));
+ }
+ m_disablePromise = new Deferred<List<Void>>().resolveWith(Promises.all(promises));
+ m_enablePromise = null;
+ return m_disablePromise;
}
- List<Promise<Void>> promises = new ArrayList<Promise<Void>>();
- for ( AbstractComponentManager<S> cm : cms )
- {
- promises.add(cm.disable( async ));
- }
- return new Deferred().resolveWith(Promises.all(promises));
}
@@ -655,7 +710,8 @@
List<AbstractComponentManager<S>> cms;
synchronized ( m_components )
{
- cms = getComponentManagers( true );
+ cms = getDirectComponentManagers( );
+ clearComponents();
}
for ( AbstractComponentManager<S> cm : cms )
{
@@ -741,40 +797,38 @@
* Returns all component managers from the map and the single component manager, optionally also removing them
* from the map. If there are no component managers, <code>null</code>
* is returned. Must be called synchronized on m_components.
- *
- * @param clear If true, clear the map and the single component manager.
*/
- List<AbstractComponentManager<S>> getComponentManagers( final boolean clear )
+ List<AbstractComponentManager<S>> getComponentManagers( )
{
- List<AbstractComponentManager<S>> cms;
- if ( m_components.isEmpty() )
+ List<AbstractComponentManager<S>> cms = new ArrayList<AbstractComponentManager<S>>();
+ if ( m_singleComponent != null)
{
- if ( m_singleComponent != null)
- {
- cms = new ArrayList<AbstractComponentManager<S>>();
- m_singleComponent.getComponentManagers(cms);
- }
- else
- {
- cms = Collections.emptyList();
- }
+ m_singleComponent.getComponentManagers(cms);
}
- else
+ for (AbstractComponentManager<S> cm: m_components.values())
{
- cms = new ArrayList<AbstractComponentManager<S>>();
- for (AbstractComponentManager<S> cm: m_components.values())
- {
- cm.getComponentManagers(cms);
- }
- }
- if ( clear )
- {
- m_components.clear();
- m_singleComponent = null;
+ cm.getComponentManagers(cms);
}
return cms;
}
+
+ List<AbstractComponentManager<S>> getDirectComponentManagers( )
+ {
+ List<AbstractComponentManager<S>> cms = new ArrayList<AbstractComponentManager<S>>();
+ if ( m_singleComponent != null)
+ {
+ cms.add(m_singleComponent);
+ }
+ cms.addAll(m_components.values());
+ return cms;
+ }
+
+ void clearComponents()
+ {
+ m_components.clear();
+ m_singleComponent = null;
+ }
public boolean isLogEnabled( int level )
{
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 d7891a1..99de1cb 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
@@ -109,7 +109,6 @@
*/
private final AtomicReference< Deferred<Void>> m_enabledLatchRef = new AtomicReference<Deferred<Void>>( new Deferred<Void>() );
- protected volatile boolean m_enabled;
protected volatile boolean m_internalEnabled;
private volatile boolean m_satisfied;
@@ -393,10 +392,6 @@
public final Promise<Void> enable( final boolean async )
{
- if (m_enabled)
- {
- return Promises.resolved(null);
- }
Deferred<Void> enableLatch = null;
try
{
@@ -413,7 +408,6 @@
{
enableLatch.resolve(null);
}
- m_enabled = true;
}
if ( async )
@@ -487,24 +481,8 @@
return newEnabledLatch;
}
- /**
- * Disables this component and - if active - first deactivates it. The
- * component may be reenabled by calling the {@link #enable()} method.
- * <p>
- * This method deactivates and disables the component immediately.
- */
- public final void disable()
- {
- disable( true );
- }
-
-
public final Promise<Void> disable( final boolean async )
{
- if (!m_enabled)
- {
- return Promises.resolved(null);
- }
Deferred<Void> enableLatch = null;
try
{
@@ -521,7 +499,6 @@
{
enableLatch.resolve(null);
}
- m_enabled = false;
}
if ( async )
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 93d355c..b1368b5 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
@@ -322,14 +322,13 @@
return null;
}
- //TODO shouldn't something be calling this?
/**
* Disposes off all components ever created by this component holder. This
* method is called if either the Declarative Services runtime is stopping
* or if the owning bundle is stopped. In both cases all components created
* by this holder must be disposed off.
*/
- public void disposeComponents( int reason )
+ public void dispose( int reason )
{
List<AbstractComponentManager<S>> cms = new ArrayList<AbstractComponentManager<S>>( );
getComponentManagers( m_componentInstances, cms );
@@ -344,7 +343,7 @@
}
// finally dispose the component factory itself
- dispose( reason );
+ super.dispose( reason );
}
@@ -389,11 +388,6 @@
}
- public boolean isEnabled() {
- return isInternalEnabled();
- }
-
-
@Override
public void reconfigure(Map<String, Object> configuration, boolean configurationDeleted) {
m_configuration = configuration;
diff --git a/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java b/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java
index 4215670..f3ab4bf 100644
--- a/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java
+++ b/scr/src/test/java/org/apache/felix/scr/impl/config/ConfiguredComponentHolderTest.java
@@ -182,7 +182,7 @@
private static List<SingleComponentManager> getComponentManagers( ConfigurableComponentHolder holder )
{
- return holder.getComponentManagers(false);
+ return holder.getComponentManagers();
}
private static class TestingConfiguredComponentHolder extends ConfigurableComponentHolder
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/ComponentEnableTest.java b/scr/src/test/java/org/apache/felix/scr/integration/ComponentEnableTest.java
new file mode 100644
index 0000000..00b6304
--- /dev/null
+++ b/scr/src/test/java/org/apache/felix/scr/integration/ComponentEnableTest.java
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.felix.scr.integration;
+
+
+import java.lang.reflect.Field;
+import java.util.Collection;
+import java.util.Map;
+
+import junit.framework.Assert;
+import junit.framework.TestCase;
+
+import org.apache.felix.scr.integration.components.EnableComponent;
+import org.apache.felix.scr.integration.components.SimpleComponent;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.junit.JUnit4TestRunner;
+import org.osgi.service.component.ComponentContext;
+import org.osgi.service.component.runtime.dto.ComponentConfigurationDTO;
+
+
+@RunWith(JUnit4TestRunner.class)
+public class ComponentEnableTest extends ComponentTestBase
+{
+ static
+ {
+ // uncomment to enable debugging of this test class
+ // paxRunnerVmOption = DEBUG_VM_OPTION;
+ descriptorFile = "/integration_test_enable.xml";
+ }
+
+
+ @Test
+ public void test_Component_Enable() throws Exception
+ {
+ final String enable = "org.apache.felix.scr.integration.components.enable";
+ final String name = "org.apache.felix.scr.integration.components.SimpleComponent";
+
+ ComponentConfigurationDTO dto = findComponentConfigurationByName(enable, ComponentConfigurationDTO.SATISFIED);
+
+ EnableComponent ec = getServiceFromConfiguration(dto, EnableComponent.class);
+
+ TestCase.assertEquals(0, SimpleComponent.INSTANCES.size());
+
+ ec.enable(name);
+ delay();
+ TestCase.assertEquals(1, SimpleComponent.INSTANCES.size());
+ ec.enable(name);
+ delay();
+ TestCase.assertEquals(1, SimpleComponent.INSTANCES.size());
+
+ }
+
+
+}
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java b/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java
index 76d540b..fa9abbb 100644
--- a/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java
+++ b/scr/src/test/java/org/apache/felix/scr/integration/ComponentFactoryTest.java
@@ -167,20 +167,15 @@
checkConfigurationCount(componentname, 1, ComponentConfigurationDTO.ACTIVE);
- // delete config, ensure factory is not active anymore and component instance not changed
+ // delete config, ensure factory is not active anymore and component instance gone
+ //(configuration required >> dispose of instance. Also for pre-1.3 components, removing config unconditionally
+ //deactivates component.
deleteConfig( componentname );
delay();
checkNoFactory(componentfactory);
- TestCase.assertNotNull( instance.getInstance() );
- TestCase.assertEquals( SimpleComponent.INSTANCE, instance.getInstance() );
- TestCase.assertEquals( instanceObject, instance.getInstance() );
- TestCase.assertEquals( PROP_NAME_FACTORY, SimpleComponent.INSTANCE.getProperty( PROP_NAME_FACTORY ) );
- TestCase.assertEquals( PROP_NAME, SimpleComponent.INSTANCE.getProperty( PROP_NAME ) );
-
- instance.dispose();
- TestCase.assertNull( SimpleComponent.INSTANCE ); // component is deactivated
- TestCase.assertNull( instance.getInstance() ); // SCR 112.12.6.2
+ TestCase.assertNull( instance.getInstance() );
+ TestCase.assertNull( SimpleComponent.INSTANCE );
// with removal of the factory, the created instance should also be removed
checkConfigurationCount(componentname, 0, ComponentConfigurationDTO.ACTIVE);
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/PersistentComponentFactoryTest.java b/scr/src/test/java/org/apache/felix/scr/integration/PersistentComponentFactoryTest.java
index e435e57..7c41a50 100644
--- a/scr/src/test/java/org/apache/felix/scr/integration/PersistentComponentFactoryTest.java
+++ b/scr/src/test/java/org/apache/felix/scr/integration/PersistentComponentFactoryTest.java
@@ -163,23 +163,16 @@
checkConfigurationCount(componentname, 1, ComponentConfigurationDTO.ACTIVE);
- // delete config, ensure factory is not active anymore and component instance not changed
+ // delete config, ensure factory is not active anymore and component instance dropped (config required)
deleteConfig( componentname );
delay();
checkNoFactory(componentfactory);
- TestCase.assertNotNull( instance.getInstance() );
- TestCase.assertEquals( SimpleComponent.INSTANCE, instance.getInstance() );
- TestCase.assertEquals( instanceObject, instance.getInstance() );
- TestCase.assertEquals( PROP_NAME_FACTORY, SimpleComponent.INSTANCE.getProperty( PROP_NAME_FACTORY ) );
- TestCase.assertEquals( PROP_NAME, SimpleComponent.INSTANCE.getProperty( PROP_NAME ) );
-
- instance.dispose();
- TestCase.assertNull( SimpleComponent.INSTANCE ); // component is deactivated
- TestCase.assertNull( instance.getInstance() ); // SCR 112.12.6.2
+ TestCase.assertNull( instance.getInstance() );
+ TestCase.assertNull( SimpleComponent.INSTANCE );
// with removal of the factory, the created instance should also be removed
- checkConfigurationCount(componentname, 0, ComponentConfigurationDTO.ACTIVE);
+ checkConfigurationCount(componentname, 0, ComponentConfigurationDTO.UNSATISFIED);
}
diff --git a/scr/src/test/java/org/apache/felix/scr/integration/components/EnableComponent.java b/scr/src/test/java/org/apache/felix/scr/integration/components/EnableComponent.java
new file mode 100644
index 0000000..cf4c7ab
--- /dev/null
+++ b/scr/src/test/java/org/apache/felix/scr/integration/components/EnableComponent.java
@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.felix.scr.integration.components;
+
+import org.osgi.service.component.ComponentContext;
+
+public class EnableComponent
+{
+
+ private ComponentContext cc;
+
+ protected void activate(ComponentContext cc)
+ {
+ this.cc = cc;
+ }
+
+ protected void deactivate()
+ {
+ cc = null;
+ }
+
+ public void enable( String component )
+ {
+ cc.enableComponent(component);
+ }
+
+ public void disable( String component )
+ {
+ cc.disableComponent(component);
+ }
+
+}
diff --git a/scr/src/test/resources/integration_test_enable.xml b/scr/src/test/resources/integration_test_enable.xml
new file mode 100644
index 0000000..78ef87a
--- /dev/null
+++ b/scr/src/test/resources/integration_test_enable.xml
@@ -0,0 +1,30 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
+ license agreements. See the NOTICE file distributed with this work for additional
+ information regarding copyright ownership. The ASF licenses this file to
+ you under the Apache License, Version 2.0 (the "License"); you may not use
+ this file except in compliance with the License. You may obtain a copy of
+ the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required
+ by applicable law or agreed to in writing, software distributed under the
+ License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS
+ OF ANY KIND, either express or implied. See the License for the specific
+ language governing permissions and limitations under the License. -->
+<components xmlns:scr="http://www.osgi.org/xmlns/scr/v1.1.0">
+ <scr:component
+ name='org.apache.felix.scr.integration.components.enable'
+ >
+ <implementation
+ class='org.apache.felix.scr.integration.components.EnableComponent' />
+ <service>
+ <provide interface='org.apache.felix.scr.integration.components.EnableComponent' />
+ </service>
+ </scr:component>
+
+ <scr:component
+ name='org.apache.felix.scr.integration.components.SimpleComponent'
+ enabled="false">
+ <implementation
+ class='org.apache.felix.scr.integration.components.SimpleComponent' />
+ </scr:component>
+
+</components>
diff --git a/scr/src/test/resources/integration_test_persistent_factory_components.xml b/scr/src/test/resources/integration_test_persistent_factory_components.xml
index 816e0b1..141820c 100644
--- a/scr/src/test/resources/integration_test_persistent_factory_components.xml
+++ b/scr/src/test/resources/integration_test_persistent_factory_components.xml
@@ -33,7 +33,8 @@
felix:persistentFactoryComponent="true"
enabled="false"
configuration-policy="require"
- factory="factory.component.factory.configuration" >
+ factory="factory.component.factory.configuration"
+ modified="modified">
<implementation class="org.apache.felix.scr.integration.components.SimpleComponent" />
</scr:component>
diff --git a/scr/src/test/resources/integration_test_simple_factory_components.xml b/scr/src/test/resources/integration_test_simple_factory_components.xml
index 6ee6611..e8e9576 100644
--- a/scr/src/test/resources/integration_test_simple_factory_components.xml
+++ b/scr/src/test/resources/integration_test_simple_factory_components.xml
@@ -31,7 +31,8 @@
<scr:component name="factory.component.configuration"
enabled="false"
configuration-policy="require"
- factory="factory.component.factory.configuration" >
+ factory="factory.component.factory.configuration"
+ modified="modified" >
<implementation class="org.apache.felix.scr.integration.components.SimpleComponent" />
</scr:component>