FELIX-5336: fixed bug where a prototype scope component is instantiated unexpectedly. Do not instantiate prototype instance in case the component does not have an init method
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk/dependencymanager@1844112 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/ScopedAspectAdaptersServiceTest.java b/org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/ScopedAspectAdaptersServiceTest.java
index 9b19a4d..ea2dce2 100644
--- a/org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/ScopedAspectAdaptersServiceTest.java
+++ b/org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/ScopedAspectAdaptersServiceTest.java
@@ -48,8 +48,7 @@
Component aspect = m.createAspectComponent()
.setAspect(Service.class, null, 10)
.setScope(ServiceScope.PROTOTYPE)
- .setFactory(this, "createAspectService")
- .setCallbacks(null, null, null, null);
+ .setFactory(this, "createAspectService");
m.add(provider);
m_e.waitForStep(1, 5000); // provider started
diff --git a/org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/ScopedServiceUnexpectedlyInstantiatedTest.java b/org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/ScopedServiceUnexpectedlyInstantiatedTest.java
new file mode 100644
index 0000000..6350ebe
--- /dev/null
+++ b/org.apache.felix.dependencymanager.itest/src/org/apache/felix/dm/itest/api/ScopedServiceUnexpectedlyInstantiatedTest.java
@@ -0,0 +1,86 @@
+/*
+ * 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.dm.itest.api;
+
+import org.apache.felix.dm.Component;
+import org.apache.felix.dm.Component.ServiceScope;
+import org.apache.felix.dm.DependencyManager;
+import org.apache.felix.dm.itest.util.Ensure;
+import org.apache.felix.dm.itest.util.TestBase;
+
+/**
+ * Make sure a scoped service does not instantiate
+ */
+public class ScopedServiceUnexpectedlyInstantiatedTest extends TestBase {
+ final static Ensure m_e = new Ensure();
+
+ public void testPrototypeComponent() {
+ DependencyManager m = getDM();
+
+ Component s1 = m.createComponent()
+ .setScope(ServiceScope.PROTOTYPE)
+ .setImplementation(S1Impl.class)
+ .setInterface(S1.class.getName(), null)
+ .add(m.createServiceDependency().setRequired(true).setService(S2.class).setCallbacks("bind", null));
+
+ Component s2 = m.createComponent()
+ .setInterface(S2.class.getName(), null)
+ .setScope(ServiceScope.PROTOTYPE)
+ .setImplementation(S2Impl.class);
+
+ Component consumer1 = m.createComponent()
+ .setImplementation(S1Consumer.class)
+ .add(m.createServiceDependency().setService(S1.class).setRequired(true).setCallbacks("bind", null));
+
+ m.add(s1);
+ m.add(s2);
+ m.add(consumer1);
+ m_e.waitForStep(3, 5000);
+
+ m.clear();
+ }
+
+ public interface S1 {
+ }
+
+ public interface S2 {
+ }
+
+ public static class S1Impl implements S1 {
+ void bind(S2 s2) {
+ m_e.step(2);
+ }
+
+ void start() {
+ m_e.step(3);
+ }
+ }
+
+ public static class S2Impl implements S2 {
+ void start() {
+ m_e.step(1);
+ }
+ }
+
+ public static class S1Consumer {
+ public void bind(S1 service) {
+ m_e.step(4);
+ }
+ }
+}
diff --git a/org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/components/ScopedServiceWithInitAnnotation.java b/org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/components/ScopedServiceWithInitAnnotation.java
new file mode 100644
index 0000000..e850e9b
--- /dev/null
+++ b/org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/components/ScopedServiceWithInitAnnotation.java
@@ -0,0 +1,154 @@
+/*
+ * 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.dm.runtime.itest.components;
+
+import java.util.Map;
+
+import org.apache.felix.dm.DependencyManager;
+import org.apache.felix.dm.annotation.api.Component;
+import org.apache.felix.dm.annotation.api.Init;
+import org.apache.felix.dm.annotation.api.Inject;
+import org.apache.felix.dm.annotation.api.PropertyType;
+import org.apache.felix.dm.annotation.api.ServiceDependency;
+import org.apache.felix.dm.annotation.api.ServiceScope;
+import org.apache.felix.dm.annotation.api.Start;
+import org.apache.felix.dm.annotation.api.Stop;
+import org.apache.felix.dm.itest.util.Ensure;
+import org.junit.Assert;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceObjects;
+import org.osgi.framework.ServiceRegistration;
+
+public class ScopedServiceWithInitAnnotation {
+
+ public final static String ENSURE = "ScopedServiceWithInitAnnotation.ENSURE";
+
+
+ public interface PrototypeService {
+ void ensure();
+ }
+
+ @PropertyType
+ public @interface MyConf {
+ public String foo() default "bar";
+ }
+
+ @Component(scope=ServiceScope.PROTOTYPE)
+ @MyConf(foo="bar2")
+ public static class PrototypeServiceImpl implements PrototypeService {
+ @ServiceDependency(filter = "(name=" + ENSURE + ")")
+ protected volatile Ensure m_sequencer;
+
+ Bundle m_clientBundle;
+ ServiceRegistration m_registration;
+
+ volatile ExtraDendency m_extra;
+
+ public PrototypeServiceImpl(Bundle b, ServiceRegistration sr) {
+ m_clientBundle = b;
+ m_registration = sr;
+ }
+
+ @Init
+ void init(org.apache.felix.dm.Component c) {
+ DependencyManager dm = c.getDependencyManager();
+ c.add(dm.createServiceDependency().setService(ExtraDendency.class).setRequired(true));
+ }
+
+ @Start
+ void start() {
+ Assert.assertNotNull(m_clientBundle);
+ Assert.assertNotNull(m_registration);
+ Assert.assertNotNull(m_extra);
+ m_sequencer.step();
+ }
+
+ @Stop
+ void stop() {
+ m_sequencer.step();
+ }
+
+ public void ensure() {
+ Assert.assertNotNull(m_clientBundle);
+ Assert.assertNotNull(m_registration);
+ Assert.assertNotNull(m_extra);
+ }
+ }
+
+ @Component
+ public static class Consumer {
+ @ServiceDependency(filter = "(name=" + ENSURE + ")")
+ protected volatile Ensure m_sequencer;
+
+ private ServiceObjects<PrototypeService> m_so;
+ private PrototypeService m_service1;
+ private PrototypeService m_service2;
+
+ @ServiceDependency
+ void bind(PrototypeService service, Map<String, Object> props) {
+ m_service1 = service;
+ Assert.assertEquals("bar2", props.get("foo"));
+ m_service1.ensure();
+ }
+
+ @ServiceDependency
+ void bind2(ServiceObjects<PrototypeService> so) {
+ m_so = so;
+ m_service2 = m_so.getService();
+ }
+
+ @Start
+ void start() {
+ Assert.assertNotNull(m_service1);
+ Assert.assertNotNull(m_service2);
+ Assert.assertNotEquals(m_service1, m_service2);
+ m_sequencer.step();
+ }
+
+ @Stop
+ void stop() {
+ m_so.ungetService(m_service2);
+ m_sequencer.step();
+ }
+ }
+
+ @Component
+ public static class ExtraDendency {
+ @Inject
+ BundleContext m_bc;
+
+ @Start
+ void start() {
+ Thread t = new Thread(() -> {
+ try {
+ Thread.sleep(1000);
+ System.out.println("register ExtraDendency" );
+ m_bc.registerService(ExtraDendency.class, this, null);
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
+ });
+ t.start();
+
+ }
+ }
+
+
+}
diff --git a/org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/tests/ScopedServiceAnnotationTest.java b/org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/tests/ScopedServiceAnnotationTest.java
index d77ed59..19a5ba8 100644
--- a/org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/tests/ScopedServiceAnnotationTest.java
+++ b/org.apache.felix.dependencymanager.runtime.itest/src/org/apache/felix/dm/runtime/itest/tests/ScopedServiceAnnotationTest.java
@@ -21,6 +21,7 @@
import org.apache.felix.dm.itest.util.Ensure;
import org.apache.felix.dm.itest.util.TestBase;
import org.apache.felix.dm.runtime.itest.components.ScopedServiceAnnotation;
+import org.apache.felix.dm.runtime.itest.components.ScopedServiceWithInitAnnotation;
import org.osgi.framework.ServiceRegistration;
/**
@@ -48,4 +49,21 @@
e.ensure();
}
+ public void testScopedServiceWithInitMethod() throws Throwable {
+ Ensure e = new Ensure();
+ ServiceRegistration seq = register(e, ScopedServiceWithInitAnnotation.ENSURE);
+ // the consumer defines two dependencies, hence two prototype instances are created
+ e.waitForStep(2, 5000);
+
+ // make sure Consumer has started
+ e.waitForStep(3, 5000);
+
+ // Deactivate service provider
+ seq.unregister();
+
+ // make sure the two prototypes and the consumer have stopped
+ e.waitForStep(6, 5000);
+ e.ensure();
+ }
+
}
diff --git a/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java b/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java
index ce65781..ead049d 100644
--- a/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java
+++ b/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ComponentImpl.java
@@ -363,7 +363,7 @@
m_listeners.forEach(clone::add);
int ctorIndex = clone.instantiateComponent(createScopedComponentConstructorArgs(bundle, registration));
// ctorIndex == -1 means we have used a factory to create the component
- // if ctorIndex == 0 mneas we have used the first constructor (the one used to inject service registration and bundles
+ // if ctorIndex == 0 means we have used the first constructor (the one used to inject service registration and bundles
if (ctorIndex != 0) {
// we don't have injected the bundle and service registration in the constructor, so inject them in the class fields
autoConfigField(clone, ServiceRegistration.class, registration);
@@ -1018,27 +1018,6 @@
return m_stopwatch;
}
- /**
- * Instantiates the component. The method signatures used when using a component constructor or a factory "create" method
- * depends on the component scope:
- *
- * 1) For SINGLETON scope:
- *
- * When no factory is used, we always use the component default public constructor: ()
- * When a factory "create" method is used, we try the factory.create with "create()" or "create(Component)" signatures
- *
- * 2) For other scopes (PROTOTYPE,BUNDLE):
- *
- * When no factory is used, we try to instantiate the component using the following constructors:
- * (Bundle, ServiceRegistration)
- * (Bundle, ServiceRegistration, BundleContext)
- * ()
- *
- * When a factory.create method is used, we try the following factory "create" method signatures:
- * create(Component, Bundle, ServiceRegistration)
- * create(Component)
- * create()
- */
@Override
public ComponentContext instantiateComponent() {
CallbackTypeDef ctorArgs = null;
@@ -1048,23 +1027,19 @@
// When a component scope is singleton, we'll use the following constructor or factory "create" method signatures:
ctorArgs = VOID;
break;
-
+
case PROTOTYPE:
case BUNDLE:
- // for scoped components, If no init callback is defined, we don't instantiate the prototype instance with the real component class,
- // we use a dymmy prototype instance object because there is no need to instantiate the real component prototype object (since there is no init method to invoke).
- if (m_callbackInit == null) {
- m_componentInstance = PROTOTYPE_INSTANCE;
- m_injectionDisabled = true;
- return this;
- }
-
- // When a component scope is BUNDLE|PROTOTYPE, we'll use the following constructor or factory "create" method signatures:
- ctorArgs = createScopedComponentConstructorArgs(null, null);
+ if (m_injectionDisabled) {
+ m_componentInstance = PROTOTYPE_INSTANCE;
+ return this;
+ } else {
+ ctorArgs = createScopedComponentConstructorArgs(null, null);
+ }
break;
-
- default:
- throw new IllegalStateException("Invalid scope: " + m_scope);
+
+ default:
+ throw new IllegalStateException("Invalid scope: " + m_scope);
}
// Create the factory "create" method signatures types
@@ -1237,6 +1212,7 @@
**/
private boolean performTransition(ComponentState oldState, ComponentState newState) {
if (oldState == ComponentState.INACTIVE && newState == ComponentState.WAITING_FOR_REQUIRED) {
+ initPrototype();
startDependencies(m_dependencies);
notifyListeners(newState);
return true;
@@ -1972,4 +1948,45 @@
}
}
+ private final static Class[][] INIT_SIGNATUTES = new Class[][] { { Component.class }, {} };
+ private final static Class[][] FACTORY_CREATE_SIGNATURES = new Class[][] { {}, { Component.class } };
+
+ private boolean hasInitMethodWhenNotSingleton() {
+ if (m_callbackInit == null) {
+ return false;
+ }
+ if (m_componentDefinition instanceof Class) {
+ if (null != InvocationUtil.getCallbackMethod((Class<?>) m_componentDefinition, m_callbackInit, INIT_SIGNATUTES)) {
+ return true;
+ }
+ } else if (m_instanceFactoryCreateMethod != null) {
+ if (m_instanceFactory != null) {
+ Class<?> factoryClass = null;
+
+ if (m_instanceFactory instanceof Class) {
+ factoryClass = (Class<?>) m_instanceFactory;
+ } else if (m_instanceFactory != null) {
+ factoryClass = m_instanceFactory.getClass();
+ }
+ Method create = InvocationUtil.getCallbackMethod(factoryClass, m_instanceFactoryCreateMethod, FACTORY_CREATE_SIGNATURES);
+ Class<?> componentType = create.getReturnType();
+ if (componentType != null && InvocationUtil.getCallbackMethod(componentType, m_callbackInit, INIT_SIGNATUTES) != null) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ private void initPrototype() {
+ switch (m_scope) {
+ case PROTOTYPE:
+ case BUNDLE:
+ if (m_callbackInit == null || ! hasInitMethodWhenNotSingleton()) {
+ // when no init callback is defined, or if the component implementation does not actually has an init method, we disable injection for the component prototype instance.
+ m_injectionDisabled = true;
+ }
+ break;
+ }
+ }
}
diff --git a/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/FactoryConfigurationAdapterImpl.java b/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/FactoryConfigurationAdapterImpl.java
index 8e77cac..17f9a33 100644
--- a/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/FactoryConfigurationAdapterImpl.java
+++ b/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/FactoryConfigurationAdapterImpl.java
@@ -292,7 +292,7 @@
}
private void invokeUpdated(Component service, CallbackTypeDef callbackInfo) throws Exception {
- if (m_component.injectionDisabled()) {
+ if (((ComponentContext) service).injectionDisabled()) {
return;
}
boolean callbackFound = false;
diff --git a/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/InvocationUtil.java b/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/InvocationUtil.java
index 66e80e3..76d3cd0 100644
--- a/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/InvocationUtil.java
+++ b/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/InvocationUtil.java
@@ -76,7 +76,7 @@
public final Object m_instance;
/**
- * The index of the consutror used when creating the component instance
+ * The index of the constructor used when creating the component instance
*/
public final int m_ctorIndex;
diff --git a/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ServiceDependencyImpl.java b/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ServiceDependencyImpl.java
index 58e5c59..01408e6 100644
--- a/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ServiceDependencyImpl.java
+++ b/org.apache.felix.dependencymanager/src/org/apache/felix/dm/impl/ServiceDependencyImpl.java
@@ -157,6 +157,7 @@
m_autoConfigInstance = prototype.m_autoConfigInstance;
m_defaultImplementation = prototype.m_defaultImplementation;
m_autoConfig = prototype.m_autoConfig;
+ m_obtainServiceBeforeInjection = prototype.m_obtainServiceBeforeInjection;
}
// --- CREATION
@@ -248,7 +249,7 @@
public Object addingService(@SuppressWarnings("rawtypes") ServiceReference reference) {
try {
ServiceEventImpl event = new ServiceEventImpl(m_component, reference, null);
- if (m_obtainServiceBeforeInjection) {
+ if (obtainServiceBeforeInjecting()) {
Object service = event.getEvent(); // will dereference the service object.
if (service == null) {
// service concurrently removed, ignore
@@ -293,7 +294,7 @@
ServiceEventImpl evt = (ServiceEventImpl) service;
ServiceEventImpl newEvt = (ServiceEventImpl) newService;
- if (m_obtainServiceBeforeInjection) {
+ if (obtainServiceBeforeInjecting()) {
try {
newEvt.getEvent();
} catch (IllegalStateException e) {
@@ -598,5 +599,9 @@
m_component.getLogger().err("Could not invoke swap callback", e);
}
}
+
+ private boolean obtainServiceBeforeInjecting() {
+ return m_obtainServiceBeforeInjection && ! m_component.injectionDisabled();
+ }
}