Fix a synchronization bug in the instance manager.
Refactor the factory classes.
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@655622 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/ComponentFactory.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/ComponentFactory.java
index 8be6408..2a28084 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/ComponentFactory.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/ComponentFactory.java
@@ -49,28 +49,32 @@
/**
* Tracker used to track required handler factories.
+ * Immutable once set.
*/
protected Tracker m_tracker;
/**
* Class loader to delegate loading.
+ * Immutable once set.
*/
- private FactoryClassloader m_classLoader = null;
+ private FactoryClassloader m_classLoader;
/**
* Component Implementation class.
*/
- private byte[] m_clazz = null;
+ private byte[] m_clazz;
/**
* Component Implementation Class Name.
+ * Immutable once set.
*/
- private String m_classname = null;
+ private String m_classname;
/**
* Manipulation Metadata of the internal POJO.
+ * Immutable once set.
*/
- private PojoMetadata m_manipulation = null;
+ private PojoMetadata m_manipulation;
/**
* Create a instance manager factory. The class is given in parameter. The
@@ -101,8 +105,9 @@
}
/**
- * Check method : allow a factory to check if given element are correct.
+ * Check method : allows a factory to check if given element is well-formed.
* A component factory metadata are correct if they contain the 'classname' attribute.
+ * As this method is called from the (single-thread) constructor, no synchronization is needed.
* @param element : the metadata
* @throws ConfigurationException occurs when the element describing the factory is malformed.
*/
@@ -111,12 +116,19 @@
if (m_classname == null) { throw new ConfigurationException("A component needs a class name : " + element); }
}
+ /**
+ * Gets the class name.
+ * No synchronization needed, the classname is immutable.
+ * @return the class name.
+ * @see org.apache.felix.ipojo.IPojoFactory#getClassName()
+ */
public String getClassName() {
return m_classname;
}
/**
- * Create a primitive instance.
+ * Creates a primitive instance.
+ * This method is called when holding the lock.
* @param config : instance configuration
* @param context : service context.
* @param handlers : handler to use
@@ -139,13 +151,14 @@
}
/**
- * Define a class.
+ * Defines a class.
+ * This method need to be synchronized to avoid that the classloader is created twice.
* @param name : qualified name of the class
* @param clazz : byte array of the class
* @param domain : protection domain of the class
* @return the defined class object
*/
- public Class defineClass(String name, byte[] clazz, ProtectionDomain domain) {
+ public synchronized Class defineClass(String name, byte[] clazz, ProtectionDomain domain) {
if (m_classLoader == null) {
m_classLoader = new FactoryClassloader();
}
@@ -153,63 +166,64 @@
}
/**
- * Return the URL of a resource.
+ * Returns the URL of a resource.
* @param resName : resource name
* @return the URL of the resource
*/
public URL getResource(String resName) {
+ //No synchronization needed, the context is immutable and the call is managed by the underlying framework.
return m_context.getBundle().getResource(resName);
}
/**
- * Load a class.
+ * Loads a class.
* @param className : name of the class to load
* @return the resulting Class object
* @throws ClassNotFoundException
* @throws ClassNotFoundException : happen when the class is not found
*/
public Class loadClass(String className) throws ClassNotFoundException {
- if (m_clazz != null && className.equals(m_classname)) {
- // Used the factory classloader to load the component implementation
- // class
- if (m_classLoader == null) {
- m_classLoader = new FactoryClassloader();
- }
- return m_classLoader.defineClass(m_classname, m_clazz, null);
+ if (m_clazz != null && m_classname.equals(className)) { // Immutable fields.
+ return defineClass(className, m_clazz, null);
}
return m_context.getBundle().loadClass(className);
}
/**
- * Start the factory.
+ * Starts the factory.
+ * This method is called with the lock.
*/
- public synchronized void starting() {
- if (m_requiredHandlers.size() != 0) {
- try {
- String filter = "(&(" + Handler.HANDLER_TYPE_PROPERTY + "=" + PrimitiveHandler.HANDLER_TYPE + ")" + "(factory.state=1)" + ")";
- m_tracker = new Tracker(m_context, m_context.createFilter(filter), this);
- m_tracker.open();
- } catch (InvalidSyntaxException e) {
- m_logger.log(Logger.ERROR, "A factory filter is not valid: " + e.getMessage());
- stop();
+ public void starting() {
+ if (m_tracker != null) {
+ return; // Already started
+ } else {
+ if (m_requiredHandlers.size() != 0) {
+ try {
+ String filter = "(&(" + Handler.HANDLER_TYPE_PROPERTY + "=" + PrimitiveHandler.HANDLER_TYPE + ")" + "(factory.state=1)" + ")";
+ m_tracker = new Tracker(m_context, m_context.createFilter(filter), this);
+ m_tracker.open();
+ } catch (InvalidSyntaxException e) {
+ m_logger.log(Logger.ERROR, "A factory filter is not valid: " + e.getMessage()); //Holding the lock should not be an issue here.
+ stop();
+ }
}
}
}
/**
- * Stop all the instance managers.
+ * Stops all the instance managers.
+ * This method is called with the lock.
*/
- public synchronized void stopping() {
+ public void stopping() {
if (m_tracker != null) {
m_tracker.close();
m_tracker = null;
}
- m_classLoader = null;
- m_clazz = null;
}
/**
- * Compute the factory name.
+ * Computes the factory name.
+ * This method does not manipulate any non-immutable fields, so does not need to be synchronized.
* @return the factory name.
*/
public String getFactoryName() {
@@ -224,7 +238,8 @@
}
/**
- * Compute required handlers.
+ * Computes required handlers.
+ * This method does not manipulate any non-immutable fields, so does not need to be synchronized.
* @return the required handler list.
*/
public List getRequiredHandlerList() {
@@ -259,11 +274,12 @@
/**
* A new handler factory is detected.
* Test if the factory can be used or not.
+ * This method need to be synchronized as it accesses to the content of required handlers.
* @param reference : the new service reference.
* @return true if the given factory reference match with a required handler.
* @see org.apache.felix.ipojo.util.TrackerCustomizer#addingService(org.osgi.framework.ServiceReference)
*/
- public boolean addingService(ServiceReference reference) {
+ public synchronized boolean addingService(ServiceReference reference) {
for (int i = 0; i < m_requiredHandlers.size(); i++) {
RequiredHandler req = (RequiredHandler) m_requiredHandlers.get(i);
if (req.getReference() == null && match(req, reference)) {
@@ -271,6 +287,7 @@
req.setReference(reference);
// If the priority has changed, sort the list.
if (oldP != req.getLevel()) {
+ // Manipulate the list.
Collections.sort(m_requiredHandlers);
}
return true;
@@ -280,11 +297,12 @@
}
/**
- * A matching service has been added to the tracker, we can no compute the factory state.
+ * A matching service has been added to the tracker, we can no compute the factory state. This method is synchronized to avoid concurrent calls to
+ * method modifying the factory state.
* @param reference : added reference.
* @see org.apache.felix.ipojo.util.TrackerCustomizer#addedService(org.osgi.framework.ServiceReference)
*/
- public void addedService(ServiceReference reference) {
+ public synchronized void addedService(ServiceReference reference) {
if (m_state == INVALID) {
computeFactoryState();
}
@@ -292,11 +310,12 @@
/**
* A used factory disappears.
+ * This method is synchronized to avoid concurrent calls to method modifying the factory state.
* @param reference : service reference.
* @param service : factory object.
* @see org.apache.felix.ipojo.util.TrackerCustomizer#removedService(org.osgi.framework.ServiceReference, java.lang.Object)
*/
- public void removedService(ServiceReference reference, Object service) {
+ public synchronized void removedService(ServiceReference reference, Object service) {
// Look for the implied reference and invalid the handler identifier
for (int i = 0; i < m_requiredHandlers.size(); i++) {
RequiredHandler req = (RequiredHandler) m_requiredHandlers.get(i);
@@ -324,8 +343,10 @@
* @return manipulation metadata of this component type.
*/
public PojoMetadata getPojoMetadata() {
- if (m_manipulation == null) {
- m_manipulation = new PojoMetadata(m_componentMetadata);
+ synchronized (this) {
+ if (m_manipulation == null) {
+ m_manipulation = new PojoMetadata(m_componentMetadata);
+ }
}
return m_manipulation;
}
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/HandlerFactory.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/HandlerFactory.java
index 08bff14..9a10c73 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/HandlerFactory.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/HandlerFactory.java
@@ -19,7 +19,6 @@
package org.apache.felix.ipojo;
import java.util.Dictionary;
-import java.util.Properties;
import org.apache.felix.ipojo.architecture.ComponentTypeDescription;
import org.apache.felix.ipojo.metadata.Element;
@@ -41,24 +40,23 @@
/**
* Handler type (composite|primitive).
- * Default: handler.
*/
- private String m_type = "primitive";
+ private final String m_type;
/**
- * Default iPOJO Namespace.
+ * iPOJO Handler Namespace.
+ * (Set the the iPOJO default namespace is not specified)
*/
- private String m_namespace = IPOJO_NAMESPACE;
+ private final String m_namespace;
/**
- * Get the handler start level.
- * Lower level are priority are configured and started before higher level, and are stopped after.
- *
+ * Handler start level.
+ * Lower level are priority are configured and started before higher level, and are stopped after.
*/
- private int m_level = Integer.MAX_VALUE;
+ private final int m_level;
/**
- * Create a composite factory.
+ * Creates a handler factory.
* @param context : bundle context
* @param metadata : metadata of the component to create
* @throws ConfigurationException occurs when the element describing the factory is malformed.
@@ -74,17 +72,23 @@
String type = metadata.getAttribute("type");
if (type != null) {
m_type = type;
+ } else {
+ m_type = "primitive"; // Set to primitive if not specified.
}
String level = metadata.getAttribute("level");
if (level != null) {
m_level = new Integer(level).intValue();
+ } else {
+ m_level = Integer.MAX_VALUE; // Set to max if not specified.
}
// Get the namespace
String namespace = metadata.getAttribute("namespace");
if (namespace != null) {
m_namespace = namespace.toLowerCase();
+ } else {
+ m_namespace = IPOJO_NAMESPACE; // Set to the iPOJO default namespace if not specified.
}
}
@@ -109,11 +113,12 @@
}
/**
- * Stop the factory.
+ * Stops the factory.
* This method does not disposed created instances.
* These instances will be disposed by the instance managers.
+ * This method is called with the lock.
*/
- public synchronized void stopping() {
+ public void stopping() {
if (m_tracker != null) {
m_tracker.close();
m_tracker = null;
@@ -121,20 +126,8 @@
}
/**
- * Compute factory service properties.
- * This method add three mandatory handler factory properties (name, namespace and type)
- * @return the properties.
- * @see org.apache.felix.ipojo.ComponentFactory#getProperties()
- */
- protected Properties getProperties() {
- Properties props = new Properties();
-
- return props;
- }
-
- /**
- * Create an instance. The given configuration needs to contain the 'name'
- * property.
+ * Creates an instance. The given configuration needs to contain the 'name'
+ * property. This method is called when holding the lock.
* @param configuration : configuration of the created instance.
* @param context : the service context to push for this instance.
* @param handlers : handler array to used.
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/HandlerManager.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/HandlerManager.java
index e2f4003..b0f0c09 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/HandlerManager.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/HandlerManager.java
@@ -18,7 +18,9 @@
*/
package org.apache.felix.ipojo;
+import java.util.ArrayList;
import java.util.Dictionary;
+import java.util.List;
import org.apache.felix.ipojo.metadata.Element;
import org.osgi.framework.BundleContext;
@@ -32,6 +34,7 @@
/**
* Handler object (contained).
+ * Immutable once set.
*/
private Handler m_handler;
@@ -74,6 +77,7 @@
/**
* Create the handler object.
* This method does nothing if the object is already created.
+ * This method does not need locking protocol as only one thread (the creator thread) can create an instance.
*/
private void createHandlerObject() {
if (m_handler != null) { return; }
@@ -83,14 +87,20 @@
/**
* Start the instance manager.
*/
- public synchronized void start() {
- if (m_state != STOPPED) { return; } // Instance already started
+ public void start() {
+ synchronized (this) {
+ if (m_state != STOPPED) {
+ return; // Instance already started
+ } else {
+ m_state = -2; // Temporary starting state, avoiding concurrent starts.
+ }
+ }
for (int i = 0; i < m_handlers.length; i++) {
m_handlers[i].addInstanceStateListener(this);
m_handlers[i].start();
}
-
+
m_handler.start(); // Call the handler start method.
for (int i = 0; i < m_handlers.length; i++) {
@@ -104,13 +114,21 @@
} else {
setState(INVALID);
}
+
+ // Now, the state is necessary different from the temporary state.
}
/**
* Stop the instance manager.
*/
- public synchronized void stop() {
- if (m_state == STOPPED) { return; } // Instance already stopped
+ public void stop() {
+ synchronized (this) {
+ if (m_state == STOPPED) {
+ return; // Instance already stopped
+ } else {
+ m_state = -2; // Temporary state avoiding concurrent stopping.
+ }
+ }
setState(INVALID);
@@ -124,10 +142,17 @@
m_handlers[i].stop();
}
- m_state = STOPPED;
- if (m_listeners != null) {
- for (int i = 0; i < m_listeners.size(); i++) {
- ((InstanceStateListener) m_listeners.get(i)).stateChanged(this, STOPPED);
+ List listeners = null;
+ synchronized (this) {
+ m_state = STOPPED;
+ if (m_listeners != null) {
+ listeners = new ArrayList(m_listeners); // Stack confinement.
+ }
+ }
+
+ if (listeners != null) {
+ for (int i = 0; i < listeners.size(); i++) {
+ ((InstanceStateListener) listeners.get(i)).stateChanged(this, STOPPED);
}
}
}
@@ -136,7 +161,7 @@
* Dispose the instance.
* @see org.apache.felix.ipojo.ComponentInstance#dispose()
*/
- public synchronized void dispose() {
+ public void dispose() {
super.dispose();
m_handler = null;
}
@@ -157,16 +182,22 @@
* @param newState : new state
* @see org.apache.felix.ipojo.InstanceStateListener#stateChanged(org.apache.felix.ipojo.ComponentInstance, int)
*/
- public synchronized void stateChanged(ComponentInstance instance, int newState) {
- if (m_state <= STOPPED) { return; }
-
+ public void stateChanged(ComponentInstance instance, int newState) {
+ int state;
+ synchronized (this) {
+ if (m_state <= STOPPED) {
+ return;
+ } else {
+ state = m_state; // Stack confinement
+ }
+ }
// Update the component state if necessary
- if (newState == INVALID && m_state == VALID) {
+ if (newState == INVALID && state == VALID) {
// Need to update the state to UNRESOLVED
setState(INVALID);
return;
}
- if (newState == VALID && m_state == INVALID) {
+ if (newState == VALID && state == INVALID) {
// An handler becomes valid => check if all handlers are valid
if (!m_handler.getValidity()) { return; }
for (int i = 0; i < m_handlers.length; i++) {
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java
index 44ae438..87cf4a9 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/IPojoFactory.java
@@ -21,6 +21,7 @@
import java.util.ArrayList;
import java.util.Dictionary;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Properties;
@@ -40,6 +41,10 @@
* @author <a href="mailto:dev@felix.apache.org">Felix Project Team</a>
*/
public abstract class IPojoFactory implements Factory, ManagedServiceFactory {
+ /*
+ * TODO there is potentially an issue when calling FactoryStateListener callbacks with the lock
+ * It should be called by a separate thread dispatching events to listeners.
+ */
/**
* List of the managed instance name. This list is shared by all factories.
@@ -54,20 +59,21 @@
/**
* List of the managed instance managers. The key of this map is the name (i.e. instance names) of the created instance
*/
- protected Map m_componentInstances = new HashMap();
+ protected final Map m_componentInstances = new HashMap();
/**
* Component Type provided by this factory.
*/
- protected Element m_componentMetadata;
+ protected final Element m_componentMetadata;
/**
* The bundle context reference.
*/
- protected BundleContext m_context = null;
+ protected final BundleContext m_context;
/**
* Factory Name. Could be the component class name if the factory name is not set.
+ * Immutable once set.
*/
protected String m_factoryName;
@@ -79,17 +85,17 @@
/**
* List of listeners.
*/
- protected List m_listeners = new ArrayList(2);
+ protected List m_listeners = new ArrayList(1);
/**
* Logger for the factory (and all component instance).
*/
- protected Logger m_logger;
+ protected final Logger m_logger;
/**
* Is the factory public (expose as a service).
*/
- protected boolean m_isPublic;
+ protected final boolean m_isPublic;
/**
* Service Registration of this factory (Factory & ManagedServiceFactory).
@@ -124,7 +130,7 @@
String fac = metadata.getAttribute("factory");
m_isPublic = fac == null || !fac.equalsIgnoreCase("false");
m_logger = new Logger(m_context, m_factoryName);
- m_requiredHandlers = getRequiredHandlerList();
+ m_requiredHandlers = getRequiredHandlerList(); // Call sub-class to get the list of required handlers.
}
public ComponentTypeDescription getComponentTypeDescription() {
@@ -132,18 +138,18 @@
}
/**
- * Add a factory listener.
+ * Adds a factory listener.
* @param listener : the factory listener to add.
* @see org.apache.felix.ipojo.Factory#addFactoryStateListener(org.apache.felix.ipojo.FactoryStateListener)
*/
public void addFactoryStateListener(FactoryStateListener listener) {
- synchronized (m_listeners) {
+ synchronized (this) {
m_listeners.add(listener);
}
}
/**
- * Get the logger used by instances of he current factory.
+ * Gets the logger used by instances of he current factory.
* @return the factory logger.
*/
public Logger getLogger() {
@@ -151,19 +157,20 @@
}
/**
- * Compute the factory name.
+ * Computes the factory name.
* @return the factory name.
*/
public abstract String getFactoryName();
/**
- * Compute the required handler list.
+ * Computes the required handler list.
* @return the required handler list
*/
public abstract List getRequiredHandlerList();
/**
- * Create an instance.
+ * Creates an instance.
+ * This method is called with the lock.
* @param config : instance configuration
* @param context : ipojo context to use
* @param handlers : handler array to use
@@ -174,7 +181,7 @@
throws ConfigurationException;
/**
- * Create an instance. The given configuration needs to contain the 'name' property.
+ * Creates an instance. The given configuration needs to contain the 'name' property.
* @param configuration : configuration of the created instance.
* @return the created component instance.
* @throws UnacceptableConfiguration : occurs if the given configuration is not consistent with the component type of this factory.
@@ -188,7 +195,9 @@
}
/**
- * Create an instance. The given configuration needs to contain the 'name' property.
+ * Creates an instance. The given configuration needs to contain the 'name' property.
+ * This method is synchronized to assert the validity of the factory during the creation.
+ * Callbacks to sub-class and to create instance need to be aware that they are holding the lock.
* @param configuration : configuration of the created instance.
* @param serviceContext : the service context to push for this instance.
* @return the created component instance.
@@ -197,25 +206,29 @@
* @throws org.apache.felix.ipojo.ConfigurationException : when the instance configuration failed.
* @see org.apache.felix.ipojo.Factory#createComponentInstance(java.util.Dictionary)
*/
- public ComponentInstance createComponentInstance(Dictionary configuration, ServiceContext serviceContext) throws UnacceptableConfiguration, // NOPMD
+ public synchronized ComponentInstance createComponentInstance(Dictionary configuration, ServiceContext serviceContext) throws UnacceptableConfiguration, // NOPMD
MissingHandlerException, ConfigurationException {
if (configuration == null) {
configuration = new Properties();
}
+
+ IPojoContext context = null;
+ if (serviceContext == null) {
+ context = new IPojoContext(m_context);
+ } else {
+ context = new IPojoContext(m_context, serviceContext);
+ }
try {
checkAcceptability(configuration);
} catch (UnacceptableConfiguration e) {
m_logger.log(Logger.ERROR, "The configuration is not acceptable : " + e.getMessage());
throw new UnacceptableConfiguration("The configuration "
- + configuration
- + " is not acceptable for "
- + m_factoryName
- + ": "
- + e.getMessage());
+ + configuration + " is not acceptable for " + m_factoryName
+ + ": " + e.getMessage());
}
- String name = null;
+ String name;
if (configuration.get("name") == null) {
name = generateName();
configuration.put("name", name);
@@ -226,21 +239,15 @@
throw new UnacceptableConfiguration("Name already used : " + name);
}
}
-
- IPojoContext context = null;
- if (serviceContext == null) {
- context = new IPojoContext(m_context);
- } else {
- context = new IPojoContext(m_context, serviceContext);
- }
-
+ // Here we are sure to be valid until the end of the method.
HandlerManager[] handlers = new HandlerManager[m_requiredHandlers.size()];
for (int i = 0; i < handlers.length; i++) {
RequiredHandler req = (RequiredHandler) m_requiredHandlers.get(i);
handlers[i] = getHandler(req, serviceContext);
}
+
try {
- ComponentInstance instance = createInstance(configuration, context, handlers);
+ ComponentInstance instance = createInstance(configuration, context, handlers); // This method is called with the lock.
m_instancesName.add(name);
m_componentInstances.put(name, instance);
return instance;
@@ -255,26 +262,27 @@
}
/**
- * Get the factory class name.
- * @return the factory classname.
+ * Gets the factory class name.
+ * @return the factory class name.
* @see org.apache.felix.ipojo.Factory#getClassName()
*/
public abstract String getClassName();
/**
- * Get the component type description.
+ * Gets the component type description.
* @return the component type description object. Null if not already computed.
*/
- public ComponentTypeDescription getComponentDescription() {
+ public synchronized ComponentTypeDescription getComponentDescription() {
return m_componentDesc;
}
/**
- * Get the component type description (Element-Attribute form).
+ * Gets the component type description (Element-Attribute form).
* @return the component type description.
* @see org.apache.felix.ipojo.Factory#getDescription()
*/
- public Element getDescription() {
+ public synchronized Element getDescription() {
+ // Can be null, if not already computed.
if (m_componentDesc == null) {
return new Element("No description available for " + m_factoryName, "");
}
@@ -282,7 +290,7 @@
}
/**
- * Compute the list of missing handlers.
+ * Computes the list of missing handlers. This method is called with the lock.
* @return list of missing handlers.
* @see org.apache.felix.ipojo.Factory#getMissingHandlers()
*/
@@ -297,16 +305,24 @@
return list;
}
+ /**
+ * Gets the factory name.
+ * This name is immutable once set.
+ * @return the factory name.
+ * @see org.apache.felix.ipojo.Factory#getName()
+ */
public String getName() {
return m_factoryName;
}
/**
- * Get the list of required handlers.
+ * Gets the list of required handlers.
+ * This method is synchronized to avoid the concurrent modification
+ * of the required handlers.
* @return list of required handlers.
* @see org.apache.felix.ipojo.Factory#getRequiredHandlers()
*/
- public List getRequiredHandlers() {
+ public synchronized List getRequiredHandlers() {
List list = new ArrayList();
for (int i = 0; i < m_requiredHandlers.size(); i++) {
RequiredHandler req = (RequiredHandler) m_requiredHandlers.get(i);
@@ -315,12 +331,18 @@
return list;
}
- public int getState() {
+ /**
+ * Gets the actual factory state.
+ * Must be synchronized as this state is dependent of handler availability.
+ * @return the actual factory state.
+ * @see org.apache.felix.ipojo.Factory#getState()
+ */
+ public synchronized int getState() {
return m_state;
}
/**
- * Check if the configuration is acceptable.
+ * Checks if the configuration is acceptable.
* @param conf : the configuration to test.
* @return true if the configuration is acceptable.
* @see org.apache.felix.ipojo.Factory#isAcceptable(java.util.Dictionary)
@@ -337,27 +359,29 @@
}
/**
- * Check if the configuration is acceptable.
+ * Checks if the configuration is acceptable.
* @param conf : the configuration to test.
* @throws UnacceptableConfiguration occurs if the configuration is unacceptable.
* @throws MissingHandlerException occurs if an handler is missing.
*/
public void checkAcceptability(Dictionary conf) throws UnacceptableConfiguration, MissingHandlerException {
- if (m_state == Factory.INVALID) {
- throw new MissingHandlerException(getMissingHandlers());
+ PropertyDescription[] props;
+ synchronized (this) {
+ if (m_state == Factory.INVALID) {
+ throw new MissingHandlerException(getMissingHandlers());
+ }
+ props = m_componentDesc.getProperties(); // Stack confinement.
+ // The property list is up to date, as the factory is valid.
}
// Check that the configuration does not override immutable properties.
- PropertyDescription[] props = m_componentDesc.getProperties();
+
for (int i = 0; i < props.length; i++) {
// Is the property immutable
if (props[i].isImmutable() && conf.get(props[i].getName()) != null) {
- throw new UnacceptableConfiguration("The property " + props[i] + " cannot be overide : immutable property"); // The instance
- // configuration try
- // to override an
- // immutable property.
+ throw new UnacceptableConfiguration("The property " + props[i] + " cannot be overide : immutable property"); // The instance configuration tries to override an immutable property.
}
- // Is the property required.
+ // Is the property required ?
if (props[i].getValue() == null && conf.get(props[i].getName()) == null) {
throw new UnacceptableConfiguration("The property " + props[i].getName() + " is missing"); // The property must be set.
}
@@ -365,7 +389,8 @@
}
/**
- * Reconfigure an existing instance.
+ * Reconfigures an existing instance.
+ * This method is synchronized to assert the validity of the factory during the reconfiguration.
* @param properties : the new configuration to push.
* @throws UnacceptableConfiguration : occurs if the new configuration is not consistent with the component type.
* @throws MissingHandlerException : occurs if the current factory is not valid.
@@ -376,55 +401,63 @@
throw new UnacceptableConfiguration("The configuration does not contains the \"name\" property");
}
String name = (String) properties.get("name");
+
ComponentInstance instance = (ComponentInstance) m_componentInstances.get(name);
-
- if (instance == null) {
- return; // The instance does not exist.
- } else {
- checkAcceptability(properties); // Test if the configuration is acceptable
- instance.reconfigure(properties); // re-configure the component
+ if (instance == null) { // The instance does not exists.
+ return;
}
+
+ checkAcceptability(properties); // Test if the configuration is acceptable
+ instance.reconfigure(properties); // re-configure the instance
}
/**
- * Remove a factory listener.
+ * Removes a factory listener.
* @param listener : the factory listener to remove.
* @see org.apache.felix.ipojo.Factory#removeFactoryStateListener(org.apache.felix.ipojo.FactoryStateListener)
*/
public void removeFactoryStateListener(FactoryStateListener listener) {
- synchronized (m_listeners) {
+ synchronized (this) {
m_listeners.remove(listener);
}
}
/**
* Stopping method. This method is call when the factory is stopping.
+ * This method is called when holding the lock on the factory.
*/
public abstract void stopping();
/**
- * Stop all the instance managers.
+ * Stops all the instance managers.
*/
public synchronized void stop() {
+ ComponentInstance[] instances;
if (m_sr != null) {
m_sr.unregister();
m_sr = null;
}
+ stopping(); // Method called when holding the lock.
+ m_state = INVALID; // Set here to avoid to create instances during the stops.
- stopping();
+ Set col = m_componentInstances.keySet();
+ Iterator it = col.iterator();
+ instances = new ComponentInstance[col.size()]; // Stack confinement
+ int index = 0;
+ while (it.hasNext()) {
+ instances[index] = (ComponentInstance) (m_componentInstances.get(it.next()));
+ index++;
+ }
if (m_state == VALID) {
for (int i = 0; i < m_listeners.size(); i++) {
((FactoryStateListener) m_listeners.get(i)).stateChanged(this, INVALID);
}
}
- m_state = INVALID;
// Dispose created instances.
- Set col = m_componentInstances.keySet();
- String[] keys = (String[]) col.toArray(new String[col.size()]);
- for (int i = 0; i < keys.length; i++) {
- ComponentInstance instance = (ComponentInstance) m_componentInstances.get(keys[i]);
+ for (int i = 0; i < instances.length; i++) {
+ ComponentInstance instance = instances[i];
if (instance.getState() != ComponentInstance.DISPOSED) {
instance.dispose();
}
@@ -434,33 +467,27 @@
for (int i = 0; i < m_requiredHandlers.size(); i++) {
((RequiredHandler) m_requiredHandlers.get(i)).unRef();
}
-
m_described = false;
m_componentDesc = null;
m_componentInstances.clear();
}
/**
- * Destroy the factory. The factory cannot be restarted.
- * Only the extender can call this method.
+ * Destroys the factory. The factory cannot be restarted. Only the extender can call this method.
*/
- void dispose() {
- stop();
- m_componentMetadata = null;
- m_componentInstances = null;
- m_context = null;
+ synchronized void dispose() {
+ stop(); // Does not hold the lock.
m_requiredHandlers = null;
m_listeners = null;
- m_logger = null;
}
/**
- * Starting method. This method is called when the factory is starting.
+ * Starting method. This method is called when the factory is starting. This method is called when holding the lock on the factory.
*/
public abstract void starting();
/**
- * Start the factory.
+ * Starts the factory.
*/
public synchronized void start() {
if (m_described) { // Already started.
@@ -482,17 +509,22 @@
}
/**
- * Create of update an instance.
+ * Creates or updates an instance.
* @param name : name of the instance
* @param properties : configuration of the instance
* @throws org.osgi.service.cm.ConfigurationException : if the configuration is not consistent for this component type
* @see org.osgi.service.cm.ManagedServiceFactory#updated(java.lang.String, java.util.Dictionary)
*/
- public synchronized void updated(String name, Dictionary properties) throws org.osgi.service.cm.ConfigurationException {
- InstanceManager instance = (InstanceManager) m_componentInstances.get(name);
+ public void updated(String name, Dictionary properties) throws org.osgi.service.cm.ConfigurationException {
+ InstanceManager instance;
+ synchronized (this) {
+ instance = (InstanceManager) m_componentInstances.get(name);
+ }
+
if (instance == null) {
try {
properties.put("name", name); // Add the name in the configuration
+ // If an instance with this name was created before, this creation will failed.
createComponentInstance(properties);
} catch (UnacceptableConfiguration e) {
m_logger.log(Logger.ERROR, "The configuration is not acceptable : " + e.getMessage());
@@ -519,7 +551,7 @@
}
/**
- * Delete an instance.
+ * Deletes an instance.
* @param name : name of the instance to delete
* @see org.osgi.service.cm.ManagedServiceFactory#deleted(java.lang.String)
*/
@@ -537,12 +569,15 @@
*/
public void disposed(ComponentInstance instance) {
String name = instance.getInstanceName();
- m_instancesName.remove(name);
- m_componentInstances.remove(name);
+ synchronized (this) {
+ m_instancesName.remove(name);
+ m_componentInstances.remove(name);
+ }
}
/**
- * Compute the component type description. The factory must be valid when calling this method.
+ * Computes the component type description. The factory must be valid when calling this method.
+ * This method is called with the lock.
*/
protected void computeDescription() {
for (int i = 0; i < m_requiredHandlers.size(); i++) {
@@ -562,7 +597,8 @@
}
/**
- * Compute factory state.
+ * Computes factory state.
+ * This method is call when holding the lock on the current factory.
*/
protected void computeFactoryState() {
boolean isValid = true;
@@ -624,7 +660,8 @@
}
/**
- * Check if the given handler identifier and the service reference can match.
+ * Checks if the given handler identifier and the service reference can match.
+ * Does not need to be synchronized as the method does not use any fields.
* @param req : the handler identifier.
* @param ref : the service reference.
* @return true if the service reference can fulfill the handler requirement
@@ -639,7 +676,8 @@
}
/**
- * Return the handler object for the given required handler. The handler is instantiated in the given service context.
+ * Returns the handler object for the given required handler. The handler is instantiated in the given service context.
+ * This method is called with the lock.
* @param req : handler to create.
* @param context : service context in which create the handler (instance context).
* @return the Handler object.
@@ -670,9 +708,10 @@
/**
* Helping method generating a new unique name.
+ * This method is call when holding the lock to assert generated name unicity.
* @return an non already used name
*/
- protected synchronized String generateName() {
+ protected String generateName() {
String name = m_factoryName + "-" + m_index;
while (m_instancesName.contains(name)) {
m_index = m_index + 1;
@@ -683,6 +722,8 @@
/**
* Structure storing required handlers.
+ * Access to this class must mostly be with the lock on the factory.
+ * (except to access final fields)
*/
protected class RequiredHandler implements Comparable {
/**
@@ -693,7 +734,7 @@
/**
* Handler name.
*/
- private String m_name;
+ private final String m_name;
/**
* Handler start level.
@@ -703,7 +744,7 @@
/**
* Handler namespace.
*/
- private String m_namespace;
+ private final String m_namespace;
/**
* Service Reference of the handler factory.
@@ -741,7 +782,8 @@
}
/**
- * Get the factory object used for this handler. The object is get when used for the first time.
+ * Gets the factory object used for this handler. The object is get when used for the first time.
+ * This method is called with the lock avoiding concurrent modification and on a valid factory.
* @return the factory object.
*/
public HandlerFactory getFactory() {
@@ -784,10 +826,10 @@
/**
* Release the reference of the used factory.
+ * This method is called with the lock on the current factory.
*/
public void unRef() {
if (m_reference != null) {
- // m_context.ungetService(m_reference); // Will be unget automatically
m_factory = null;
m_reference = null;
}
@@ -795,6 +837,7 @@
/**
* Set the service reference. If the new service reference is null, it unget the used factory (if already get).
+ * This method is called with the lock on the current factory.
* @param ref : new service reference.
*/
public void setReference(ServiceReference ref) {
@@ -807,6 +850,7 @@
/**
* Start level Comparison. This method is used to sort the handler array.
+ * This method is called with the lock.
* @param object : object on which compare.
* @return -1, 0, +1 according to the comparison of their start level.
* @see java.lang.Comparable#compareTo(java.lang.Object)
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
index 82dbd94..173f2fa 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/InstanceManager.java
@@ -54,7 +54,7 @@
/**
* Handler list.
*/
- protected HandlerManager[] m_handlers = null;
+ protected final HandlerManager[] m_handlers;
/**
* Component state (STOPPED at the beginning).
@@ -69,25 +69,28 @@
/**
* Parent factory (ComponentFactory).
*/
- private ComponentFactory m_factory;
+ private final ComponentFactory m_factory;
/**
* The context of the component.
*/
- private BundleContext m_context;
+ private final BundleContext m_context;
/**
* Map [field, field interceptor list] storing handlers interested by the field.
+ * Once configured, this map can't change.
*/
private Map m_fieldRegistration;
/**
* Map [method identifier, method interceptor list] storing handlers interested by the method.
+ * Once configure this map can't change.
*/
private Map m_methodRegistration;
/**
* Manipulated class.
+ * Once set, this field doesn't change.
*/
private Class m_clazz;
@@ -98,6 +101,7 @@
/**
* Factory method. Contains the name of the static method used to create POJO objects.
+ * Once set, this field is immutable.
*/
private String m_factoryMethod = null;
@@ -164,12 +168,14 @@
InstanceDescription desc =
new InstanceDescription(m_name, componentState, getContext().getBundle().getBundleId(), m_factory.getComponentDescription());
- if (m_pojoObjects != null) {
- String[] objects = new String[m_pojoObjects.size()];
- for (int i = 0; i < m_pojoObjects.size(); i++) {
- objects[i] = m_pojoObjects.get(i).toString();
+ synchronized (this) { // Must be synchronized, it access to the m_pojoObjects list.
+ if (m_pojoObjects != null) {
+ String[] objects = new String[m_pojoObjects.size()];
+ for (int i = 0; i < m_pojoObjects.size(); i++) {
+ objects[i] = m_pojoObjects.get(i).toString();
+ }
+ desc.setCreatedObjects(objects);
}
- desc.setCreatedObjects(objects);
}
Handler[] handlers = getRegistredHandlers();
@@ -181,6 +187,7 @@
/**
* Get the list of handlers plugged on the instance.
+ * This method does not need a synchronized block as the handler set is constant.
* @return the handler array of plugged handlers.
*/
public Handler[] getRegistredHandlers() {
@@ -193,6 +200,7 @@
/**
* Return a specified handler.
+ * This must does not need a synchronized block as the handler set is constant.
* @param name : class name of the handler to find or its qualified name (namespace:name)
* @return : the handler, or null if not found
*/
@@ -262,11 +270,15 @@
/**
* Start the instance manager.
*/
- public synchronized void start() {
- if (m_state != STOPPED) { // Instance already started
- return;
+ public void start() {
+ synchronized (this) {
+ if (m_state != STOPPED) { // Instance already started
+ return;
+ } else {
+ m_state = -2; // Temporary state.
+ }
}
-
+
for (int i = 0; i < m_handlers.length; i++) {
m_handlers[i].addInstanceStateListener(this);
try {
@@ -277,13 +289,12 @@
throw e;
}
}
-
+
for (int i = 0; i < m_handlers.length; i++) {
if (m_handlers[i].getState() != VALID) {
setState(INVALID);
return;
}
-
}
setState(VALID);
}
@@ -291,26 +302,35 @@
/**
* Stop the instance manager.
*/
- public synchronized void stop() {
- if (m_state == STOPPED) {
- return;
- } // Instance already stopped
-
- setState(INVALID);
-
- m_state = STOPPED;
+ public void stop() {
+ List listeners = null;
+ synchronized (this) {
+ if (m_state == STOPPED) { // Instance already stopped
+ return;
+ }
+ m_stateQueue.clear();
+ m_inTransition = false;
+ }
+
+ setState(INVALID); // Must be called outside a synchronized block.
// Stop all the handlers
for (int i = m_handlers.length - 1; i > -1; i--) {
m_handlers[i].removeInstanceStateListener(this);
m_handlers[i].stop();
}
+
+ synchronized (this) {
+ m_state = STOPPED;
+ if (m_listeners != null) {
+ listeners = new ArrayList(m_listeners); // Stack confinement
+ }
+ m_pojoObjects = null;
+ }
- m_pojoObjects = null;
-
- if (m_listeners != null) {
- for (int i = 0; i < m_listeners.size(); i++) {
- ((InstanceStateListener) m_listeners.get(i)).stateChanged(this, STOPPED);
+ if (listeners != null) {
+ for (int i = 0; i < listeners.size(); i++) {
+ ((InstanceStateListener) listeners.get(i)).stateChanged(this, STOPPED);
}
}
}
@@ -319,29 +339,40 @@
* Dispose the instance.
* @see org.apache.felix.ipojo.ComponentInstance#dispose()
*/
- public synchronized void dispose() {
- if (m_state > STOPPED) { // Valid or invalid
- stop();
+ public void dispose() {
+ List listeners = null;
+ int state = -2;
+ synchronized (this) {
+ state = m_state; // Stack confinement
+ if (m_listeners != null) {
+ listeners = new ArrayList(m_listeners); // Stack confinement
+ }
+ m_listeners = null;
+ }
+
+ if (state > STOPPED) { // Valid or invalid
+ stop(); // Does not hold the lock.
+ }
+
+ synchronized (this) {
+ m_state = DISPOSED;
}
- m_state = DISPOSED;
-
- for (int i = 0; m_listeners != null && i < m_listeners.size(); i++) {
- ((InstanceStateListener) m_listeners.get(i)).stateChanged(this, DISPOSED);
+ for (int i = 0; listeners != null && i < listeners.size(); i++) {
+ ((InstanceStateListener) listeners.get(i)).stateChanged(this, DISPOSED);
}
- m_listeners = null;
for (int i = m_handlers.length - 1; i > -1; i--) {
m_handlers[i].dispose();
}
- m_handlers = new HandlerManager[0];
- m_factory.disposed(this);
- m_fields.clear();
- m_fieldRegistration = new HashMap();
- m_methodRegistration = new HashMap();
- m_clazz = null;
- m_inTransition = false;
+ synchronized (this) {
+ m_factory.disposed(this);
+ m_fields.clear();
+ m_fieldRegistration = new HashMap();
+ m_methodRegistration = new HashMap();
+ m_clazz = null;
+ }
}
/**
@@ -350,54 +381,64 @@
* finished.
* @param state : the new state
*/
- public synchronized void setState(int state) {
- if (m_inTransition) {
- m_stateQueue.add(new Integer(state));
- return;
+ public void setState(int state) {
+ int originalState = -2;
+ List listeners = null;
+ synchronized (this) {
+ if (m_inTransition) {
+ m_stateQueue.add(new Integer(state));
+ return;
+ }
+
+ if (m_state != state) {
+ m_inTransition = true;
+ originalState = m_state; // Stack confinement.
+ m_state = state;
+ if (m_listeners != null) {
+ listeners = new ArrayList(m_listeners); // Stack confinement.
+ }
+ }
}
- if (m_state != state) {
- m_inTransition = true;
-
- if (state > m_state) {
+ // This section can be executed only by one thread at the same time. The m_inTransition pseudo semaphore block access to this section.
+ if (m_inTransition) { // Check that we are really changing.
+ if (state > originalState) {
// The state increases (Stopped = > IV, IV => V) => invoke handlers from the higher priority to the lower
- m_state = state;
try {
for (int i = 0; i < m_handlers.length; i++) {
m_handlers[i].getHandler().stateChanged(state);
}
} catch (IllegalStateException e) {
// When an illegal state exception happens, the instance manager must be stopped immediately.
- m_stateQueue.clear();
stop();
return;
}
} else {
// The state decreases (V => IV, IV = > Stopped, Stopped => Disposed)
- m_state = state;
try {
for (int i = m_handlers.length - 1; i > -1; i--) {
m_handlers[i].getHandler().stateChanged(state);
}
} catch (IllegalStateException e) {
// When an illegal state exception happens, the instance manager must be stopped immediately.
- m_stateQueue.clear();
stop();
return;
}
}
+ }
- if (m_listeners != null) {
- for (int i = 0; i < m_listeners.size(); i++) {
- ((InstanceStateListener) m_listeners.get(i)).stateChanged(this, state);
- }
+ if (listeners != null) {
+ for (int i = 0; i < listeners.size(); i++) {
+ ((InstanceStateListener) listeners.get(i)).stateChanged(this, state);
}
}
- m_inTransition = false;
- if (!m_stateQueue.isEmpty()) {
- int newState = ((Integer) (m_stateQueue.remove(0))).intValue();
- setState(newState);
+ synchronized (this) {
+ m_inTransition = false;
+ if (!m_stateQueue.isEmpty()) {
+ int newState = ((Integer) (m_stateQueue.remove(0))).intValue();
+ setState(newState);
+ }
}
}
@@ -406,7 +447,7 @@
* @return the actual state of the component instance.
* @see org.apache.felix.ipojo.ComponentInstance#getState()
*/
- public int getState() {
+ public synchronized int getState() {
return m_state;
}
@@ -415,7 +456,7 @@
* @return true if the instance is started.
* @see org.apache.felix.ipojo.ComponentInstance#isStarted()
*/
- public boolean isStarted() {
+ public synchronized boolean isStarted() {
return m_state > STOPPED;
}
@@ -428,9 +469,7 @@
if (m_listeners == null) {
m_listeners = new ArrayList();
}
- synchronized (m_listeners) {
- m_listeners.add(listener);
- }
+ m_listeners.add(listener);
}
/**
@@ -440,11 +479,9 @@
*/
public synchronized void removeInstanceStateListener(InstanceStateListener listener) {
if (m_listeners != null) {
- synchronized (m_listeners) {
- m_listeners.remove(listener);
- if (m_listeners.isEmpty()) {
- m_listeners = null;
- }
+ m_listeners.remove(listener);
+ if (m_listeners.isEmpty()) {
+ m_listeners = null;
}
}
}
@@ -475,7 +512,7 @@
* Get the array of object created by the instance.
* @return the created instance of the component instance.
*/
- public Object[] getPojoObjects() {
+ public synchronized Object[] getPojoObjects() {
if (m_pojoObjects == null) {
return null;
}
@@ -491,8 +528,8 @@
load();
}
+ // The following code doesn't need to be synchronized as is deal only with immutable fields.
Object instance = null;
-
if (m_factoryMethod == null) {
// No factory-method, we use the constructor.
try {
@@ -627,19 +664,21 @@
}
}
+
+
- // Register the new instance in not already present.
- if (m_pojoObjects == null) {
- m_pojoObjects = new ArrayList(1);
- }
- if (!m_pojoObjects.contains(instance)) {
- m_pojoObjects.add(instance);
- // Call createInstance on Handlers :
- for (int i = 0; i < m_handlers.length; i++) {
- ((PrimitiveHandler) m_handlers[i].getHandler()).onCreation(instance);
+ // Add the new instance in the instance list.
+ synchronized (this) {
+ if (m_pojoObjects == null) {
+ m_pojoObjects = new ArrayList(1);
}
+ m_pojoObjects.add(instance);
}
-
+ // Call createInstance on Handlers :
+ for (int i = 0; i < m_handlers.length; i++) {
+ ((PrimitiveHandler) m_handlers[i].getHandler()).onCreation(instance);
+ }
+
return instance;
}
@@ -647,15 +686,25 @@
* Get the first object created by the instance. If no object created, create and return one object.
* @return the instance of the component instance to use for singleton component
*/
- public synchronized Object getPojoObject() {
- if (m_pojoObjects == null) {
- return createPojoObject();
+ public Object getPojoObject() {
+ Object pojo = null;
+ synchronized (this) {
+ if (m_pojoObjects != null) {
+ pojo = m_pojoObjects.get(0); // Stack confinement
+ }
}
- return m_pojoObjects.get(0);
+
+ if (pojo == null) {
+ return createPojoObject(); // This method must be called without the lock.
+ } else {
+ return pojo;
+ }
}
/**
* Get the manipulated class.
+ * The method does not need to be synchronized.
+ * Reassigning the internal class will use the same class object.
* @return the manipulated class
*/
public Class getClazz() {
@@ -743,13 +792,17 @@
* @param fieldName : the field name on which the GETFIELD instruction is called
* @return the value decided by the last asked handler (throw a warning if two fields decide two different values)
*/
- public Object onGet(Object pojo, String fieldName) {
- Object initialValue = m_fields.get(fieldName);
+ public Object onGet(Object pojo, String fieldName) {
+ Object initialValue = null;
+ synchronized (this) { // Stack confinement.
+ initialValue = m_fields.get(fieldName);
+ }
Object result = initialValue;
// Get the list of registered handlers
- FieldInterceptor[] list = (FieldInterceptor[]) m_fieldRegistration.get(fieldName);
+ FieldInterceptor[] list = (FieldInterceptor[]) m_fieldRegistration.get(fieldName); // Immutable list.
for (int i = 0; list != null && i < list.length; i++) {
+ // Call onGet outside of a synchronized block.
Object handlerResult = list[i].onGet(null, fieldName, initialValue);
if (handlerResult == initialValue) {
continue; // Non-binding case (default implementation).
@@ -768,7 +821,10 @@
if ((result != null && !result.equals(initialValue)) || (result == null && initialValue != null)) {
// A change occurs => notify the change
- m_fields.put(fieldName, result);
+ synchronized (this) {
+ m_fields.put(fieldName, result);
+ }
+ // Call onset outside of a synchronized block.
for (int i = 0; list != null && i < list.length; i++) {
list[i].onSet(null, fieldName, result);
}
@@ -784,13 +840,13 @@
* @param args : argument array
*/
public void onEntry(Object pojo, String methodId, Object[] args) {
- if (m_methodRegistration == null) {
+ if (m_methodRegistration == null) { // Immutable field.
return;
}
MethodInterceptor[] list = (MethodInterceptor[]) m_methodRegistration.get(methodId);
Method method = getMethodById(methodId);
for (int i = 0; list != null && i < list.length; i++) {
- list[i].onEntry(pojo, method, args);
+ list[i].onEntry(pojo, method, args); // Outside a synchronized block.
}
}
@@ -842,6 +898,7 @@
* @return : the method object or null if the method cannot be found.
*/
private Method getMethodById(String methodId) {
+ // Not necessary synchronized as recomputing the methodID will give the same Method twice.
Method method = (Method) m_methods.get(methodId);
if (method == null) {
Method[] mets = m_clazz.getDeclaredMethods();
@@ -873,12 +930,17 @@
* @param objectValue : the value of the field
*/
public void onSet(Object pojo, String fieldName, Object objectValue) {
- Object value = m_fields.get(fieldName);
+ Object value = null; // Stack confinement
+ synchronized (this) {
+ value = m_fields.get(fieldName);
+ }
if ((value != null && !value.equals(objectValue)) || (value == null && objectValue != null)) {
- m_fields.put(fieldName, objectValue);
+ synchronized (this) {
+ m_fields.put(fieldName, objectValue);
+ }
FieldInterceptor[] list = (FieldInterceptor[]) m_fieldRegistration.get(fieldName);
for (int i = 0; list != null && i < list.length; i++) {
- list[i].onSet(null, fieldName, objectValue);
+ list[i].onSet(null, fieldName, objectValue); // Outside a synchronized block.
}
}
}
@@ -889,15 +951,15 @@
* @see org.apache.felix.ipojo.ComponentInstance#getContext()
*/
public BundleContext getContext() {
- return m_context;
+ return m_context; // Immutable
}
public BundleContext getGlobalContext() {
- return ((IPojoContext) m_context).getGlobalContext();
+ return ((IPojoContext) m_context).getGlobalContext(); // Immutable
}
public ServiceContext getLocalServiceContext() {
- return ((IPojoContext) m_context).getServiceContext();
+ return ((IPojoContext) m_context).getServiceContext(); // Immutable
}
/**
@@ -906,7 +968,7 @@
* @see org.apache.felix.ipojo.ComponentInstance#getInstanceName()
*/
public String getInstanceName() {
- return m_name;
+ return m_name; // Immutable
}
/**
@@ -918,19 +980,24 @@
for (int i = 0; i < m_handlers.length; i++) {
m_handlers[i].getHandler().reconfigure(configuration);
}
- if (m_state == INVALID) {
- // Try to revalidate the instance ofter reconfiguration
- for (int i = 0; i < m_handlers.length; i++) {
- if (m_handlers[i].getState() != VALID) {
- return;
+ // We synchronized the state computation.
+ synchronized (this) {
+ if (m_state == INVALID) {
+ // Try to revalidate the instance after reconfiguration
+ for (int i = 0; i < m_handlers.length; i++) {
+ if (m_handlers[i].getState() != VALID) {
+ return;
+ }
}
+ setState(VALID);
}
- setState(VALID);
}
}
/**
* Get the implementation class of the component type.
+ * This method does not need to be synchronized as the
+ * class name is constant once set.
* @return the class name of the component type.
*/
public String getClassName() {
@@ -943,18 +1010,23 @@
* @param newState : new state
* @see org.apache.felix.ipojo.InstanceStateListener#stateChanged(org.apache.felix.ipojo.ComponentInstance, int)
*/
- public synchronized void stateChanged(ComponentInstance instance, int newState) {
- if (m_state <= STOPPED) {
- return;
+ public void stateChanged(ComponentInstance instance, int newState) {
+ int state;
+ synchronized (this) {
+ if (m_state <= STOPPED) {
+ return;
+ } else {
+ state = m_state; // Stack confinement
+ }
}
// Update the component state if necessary
- if (newState == INVALID && m_state == VALID) {
+ if (newState == INVALID && state == VALID) {
// Need to update the state to UNRESOLVED
setState(INVALID);
return;
}
- if (newState == VALID && m_state == INVALID) {
+ if (newState == VALID && state == INVALID) {
// An handler becomes valid => check if all handlers are valid
for (int i = 0; i < m_handlers.length; i++) {
if (m_handlers[i].getState() != VALID) {
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java
index feb86ba..39081d9 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/architecture/ComponentTypeDescription.java
@@ -47,7 +47,7 @@
/**
* Represented factory.
*/
- private Factory m_factory;
+ private final Factory m_factory;
/**
* Constructor.
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java
index 0bb6ab3..73cdb04 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/configuration/ConfigurationHandler.java
@@ -321,7 +321,7 @@
* @param configuration : the new configuration
* @see org.apache.felix.ipojo.Handler#reconfigure(java.util.Dictionary)
*/
- public synchronized void reconfigure(Dictionary configuration) {
+ public synchronized void reconfigure(Dictionary configuration) {
Properties props = reconfigureProperties(configuration);
propagate(props, m_propagatedFromInstance);
m_propagatedFromInstance = props;
diff --git a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/Dependency.java b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/Dependency.java
index 274a092..82a242a 100644
--- a/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/Dependency.java
+++ b/ipojo/core/src/main/java/org/apache/felix/ipojo/handlers/dependency/Dependency.java
@@ -393,8 +393,8 @@
* @return the service object or a nullable / default implementation if defined.
* @see org.apache.felix.ipojo.FieldInterceptor#onGet(java.lang.Object, java.lang.String, java.lang.Object)
*/
- public Object onGet(Object pojo, String fieldName, Object value) {
- // Initialize the thread local object is not already touched.
+ public Object onGet(Object pojo, String fieldName, Object value) {
+ // Initialize the thread local object is not already touched.
Usage usage = (Usage) m_usage.get();
if (usage.m_stack == 0) { // uninitialized usage.
ServiceReference[] refs = super.getServiceReferences();
@@ -441,7 +441,7 @@
* @param value : set value.
* @see org.apache.felix.ipojo.FieldInterceptor#onSet(java.lang.Object, java.lang.String, java.lang.Object)
*/
- public void onSet(Object pojo, String fieldName, Object value) {
+ public void onSet(Object pojo, String fieldName, Object value) {
// Nothing to do.
}
@@ -482,8 +482,7 @@
* @see org.apache.felix.ipojo.MethodInterceptor#onExit(java.lang.Object, java.lang.reflect.Method, java.lang.Object)
*/
public void onExit(Object pojo, Method method, Object returnedObj) {
- // Nothing to do : wait onFinally
-
+ // Nothing to do : wait onFinally
}
/**