You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by cl...@apache.org on 2014/01/06 11:42:50 UTC

svn commit: r1555708 - /felix/trunk/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedService.java

Author: clement
Date: Mon Jan  6 10:42:50 2014
New Revision: 1555708

URL: http://svn.apache.org/r1555708
Log:
Fix FELIX-4374

Change synchronization protocol in the provided service class.

Modified:
    felix/trunk/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedService.java

Modified: felix/trunk/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedService.java
URL: http://svn.apache.org/viewvc/felix/trunk/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedService.java?rev=1555708&r1=1555707&r2=1555708&view=diff
==============================================================================
--- felix/trunk/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedService.java (original)
+++ felix/trunk/ipojo/runtime/core/src/main/java/org/apache/felix/ipojo/handlers/providedservice/ProvidedService.java Mon Jan  6 10:42:50 2014
@@ -18,23 +18,18 @@
  */
 package org.apache.felix.ipojo.handlers.providedservice;
 
-import java.lang.reflect.InvocationHandler;
-import java.lang.reflect.Method;
-import java.lang.reflect.Proxy;
-import java.util.*;
-
 import org.apache.felix.ipojo.*;
 import org.apache.felix.ipojo.util.Callback;
 import org.apache.felix.ipojo.util.Property;
 import org.apache.felix.ipojo.util.SecurityHelper;
-import org.osgi.framework.Bundle;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.Constants;
-import org.osgi.framework.ServiceFactory;
-import org.osgi.framework.ServiceReference;
-import org.osgi.framework.ServiceRegistration;
+import org.osgi.framework.*;
 import org.osgi.service.cm.ConfigurationAdmin;
 
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.lang.reflect.Proxy;
+import java.util.*;
+
 /**
  * Provided Service represent a provided service by the component.
  *
@@ -144,11 +139,11 @@ public class ProvidedService implements 
     /**
      * Creates a provided service object.
      *
-     * @param handler the the provided service handler.
-     * @param specification the specifications provided by this provided service
-     * @param factoryPolicy the service providing policy
+     * @param handler               the the provided service handler.
+     * @param specification         the specifications provided by this provided service
+     * @param factoryPolicy         the service providing policy
      * @param creationStrategyClass the customized service object creation strategy.
-     * @param conf the instance configuration.
+     * @param conf                  the instance configuration.
      */
     public ProvidedService(ProvidedServiceHandler handler, String[] specification, int factoryPolicy, Class creationStrategyClass, Dictionary conf) {
         CreationStrategy strategy;
@@ -242,6 +237,7 @@ public class ProvidedService implements 
 
     /**
      * Add properties to the provided service.
+     *
      * @param props : the properties to attached to the service registration
      */
     protected void setProperties(Property[] props) {
@@ -272,6 +268,7 @@ public class ProvidedService implements 
 
     /**
      * Get the service reference of the service registration.
+     *
      * @return the service reference of the provided service (null if the
      * service is not published).
      */
@@ -285,11 +282,12 @@ public class ProvidedService implements 
 
     /**
      * Returns a service object for the dependency.
-     * @see org.osgi.framework.ServiceFactory#getService(org.osgi.framework.Bundle, org.osgi.framework.ServiceRegistration)
-     * @param bundle : the bundle
+     *
+     * @param bundle       : the bundle
      * @param registration : the service registration of the registered service
      * @return a new service object or a already created service object (in the case of singleton) or <code>null</code>
      * if the instance is no more valid.
+     * @see org.osgi.framework.ServiceFactory#getService(org.osgi.framework.Bundle, org.osgi.framework.ServiceRegistration)
      */
     public Object getService(Bundle bundle, ServiceRegistration registration) {
         if (getInstanceManager().getState() == InstanceManager.VALID) {
@@ -302,11 +300,11 @@ public class ProvidedService implements 
     /**
      * The unget method.
      *
+     * @param bundle       : bundle
+     * @param registration : service registration
+     * @param service      : service object
      * @see org.osgi.framework.ServiceFactory#ungetService(org.osgi.framework.Bundle,
      * org.osgi.framework.ServiceRegistration, java.lang.Object)
-     * @param bundle : bundle
-     * @param registration : service registration
-     * @param service : service object
      */
     public void ungetService(Bundle bundle, ServiceRegistration registration, Object service) {
         m_strategy.ungetService(bundle, registration, service);
@@ -320,25 +318,18 @@ public class ProvidedService implements 
     public void registerService() {
         ServiceRegistration reg = null;
         Properties serviceProperties = null;
+        // Do not have to be in the synchronized block, immutable.
+        final BundleContext bc = m_handler.getInstanceManager().getContext();
+
         synchronized (this) {
             if (m_serviceRegistration != null) {
                 return;
             } else {
                 if (m_handler.getInstanceManager().getState() == ComponentInstance.VALID && isAtLeastAServiceControllerValid()) {
-                    // Build the service properties list
-
-                    BundleContext bc = m_handler.getInstanceManager().getContext();
                     // Security check
                     if (SecurityHelper.hasPermissionToRegisterServices(
-                            m_serviceSpecifications, bc)  && SecurityHelper.canRegisterService(bc)) {
+                            m_serviceSpecifications, bc) && SecurityHelper.canRegisterService(bc)) {
                         serviceProperties = getServiceProperties();
-                        m_strategy.onPublication(getInstanceManager(),
-                                getServiceSpecificationsToRegister(),
-                                serviceProperties);
-                        m_serviceRegistration = bc.registerService(
-                                getServiceSpecificationsToRegister(), this,
-                                (Dictionary) serviceProperties);
-                        reg = m_serviceRegistration; // Stack confinement
                     } else {
                         throw new SecurityException("The bundle "
                                 + bc.getBundle().getBundleId()
@@ -346,21 +337,48 @@ public class ProvidedService implements 
                                 + " permission to register the services "
                                 + Arrays.asList(m_serviceSpecifications));
                     }
+                } else {
+                    // We don't have to do anything.
+                    return;
                 }
             }
         }
 
-        // An update may happen during the registration, re-check and apply.
-        // This must be call outside the synchronized block.
-        // If the registration is null, the security helper returns false.
-        if (m_wasUpdated  && SecurityHelper.canUpdateService(reg)) {
-            Properties updated = getServiceProperties();
-            reg.setProperties((Dictionary) updated);
-            m_publishedProperties = updated;
-            m_wasUpdated = false;
+        // Registration must be done outside of the synchronized block.
+        m_strategy.onPublication(getInstanceManager(),
+                getServiceSpecificationsToRegister(),
+                serviceProperties);
+        reg = bc.registerService(
+                getServiceSpecificationsToRegister(), this,
+                (Dictionary) serviceProperties);
+
+        boolean update = false;
+        synchronized (this) {
+            if (m_serviceRegistration != null) {
+                // Oh oh the service was registered twice. Unregister the last one
+                reg.unregister();
+                return;
+            } else {
+                m_serviceRegistration = reg;
+            }
+
+            // An update may happen during the registration, re-check and apply.
+            // This must be call outside the synchronized block.
+            // If the registration is null, the security helper returns false.
+            if (m_wasUpdated && SecurityHelper.canUpdateService(reg)) {
+                serviceProperties = getServiceProperties();
+                update = true;
+            }
+        }
+
+        if (update) {
+            reg.setProperties(serviceProperties);
         }
 
         synchronized (this) {
+            m_publishedProperties = serviceProperties;
+            m_wasUpdated = false;
+
             // Call the post-registration callback in the same thread holding
             // the monitor lock.
             // This allows to be sure that the callback is called once per
@@ -369,8 +387,8 @@ public class ProvidedService implements 
             if (m_postRegistration != null) {
                 try {
                     m_postRegistration
-                            .call(new Object[] { m_serviceRegistration
-                                    .getReference() });
+                            .call(new Object[]{m_serviceRegistration
+                                    .getReference()});
                 } catch (Exception e) {
                     m_handler.error(
                             "Cannot invoke the post-registration callback "
@@ -403,9 +421,9 @@ public class ProvidedService implements 
             // Call the post-unregistration callback in the same thread holding the monitor lock.
             // This allows to be sure that the callback is called once per unregistration.
             // But the callback must take care to not create a deadlock
-            if (m_postUnregistration != null   && ref != null) {
+            if (m_postUnregistration != null && ref != null) {
                 try {
-                    m_postUnregistration.call(new Object[] { ref });
+                    m_postUnregistration.call(new Object[]{ref});
                 } catch (Exception e) {
                     m_handler.error("Cannot invoke the post-unregistration callback " + m_postUnregistration.getMethod(), e);
                 }
@@ -421,6 +439,7 @@ public class ProvidedService implements 
 
     /**
      * Get the current provided service state.
+     *
      * @return The state of the provided service.
      */
     public int getState() {
@@ -438,6 +457,8 @@ public class ProvidedService implements 
     /**
      * Return the list of properties attached to this service. This list
      * contains only property where a value are assigned.
+     * <p/>
+     * This method is called while holding the monitor lock.
      *
      * @return the properties attached to the provided service.
      */
@@ -446,7 +467,7 @@ public class ProvidedService implements 
         Properties serviceProperties = new Properties();
         for (Property p : m_properties.values()) {
             final Object value = p.getValue();
-            if (value != null  && value != Property.NO_VALUE) {
+            if (value != null && value != Property.NO_VALUE) {
                 serviceProperties.put(p.getName(), value);
             }
         }
@@ -455,6 +476,7 @@ public class ProvidedService implements 
 
     /**
      * Get the list of properties attached to the service registration.
+     *
      * @return the properties attached to the provided service.
      */
     public synchronized Property[] getProperties() {
@@ -500,14 +522,14 @@ public class ProvidedService implements 
                     // Check changes
                     Enumeration keys = oldProps.keys();
                     boolean hasChanged = false;
-                    while (! hasChanged  && keys.hasMoreElements()) {
+                    while (!hasChanged && keys.hasMoreElements()) {
                         String k = (String) keys.nextElement();
                         Object val = oldProps.get(k);
-                        if (! val.equals(updated.get(k))) {
+                        if (!val.equals(updated.get(k))) {
                             hasChanged = true;
                         }
                     }
-                    if (hasChanged  && SecurityHelper.canUpdateService(m_serviceRegistration)) {
+                    if (hasChanged && SecurityHelper.canUpdateService(m_serviceRegistration)) {
                         m_handler.info("Updating Registration : " + updated);
                         m_publishedProperties = updated;
                         m_serviceRegistration.setProperties(updated);
@@ -527,6 +549,7 @@ public class ProvidedService implements 
 
     /**
      * Add properties to the list.
+     *
      * @param props : properties to add
      */
     protected void addProperties(Dictionary props) {
@@ -536,11 +559,16 @@ public class ProvidedService implements 
             String key = (String) keys.nextElement();
             Object value = props.get(key);
 
-            Property property  = m_properties.get(key);
+            // m_properties can be modified by another thread, we need to make a stack confinement
+            Property property;
+            synchronized (this) {
+                property = m_properties.get(key);
+            }
+
             if (property != null) {
                 // Already existing property
-                if (property.getValue() == null  || ! value.equals(property.getValue())) {
-                    m_properties.get(key).setValue(value);
+                if (property.getValue() == null || !value.equals(property.getValue())) {
+                    property.setValue(value);
                     updated = true;
                 }
             } else {
@@ -563,6 +591,7 @@ public class ProvidedService implements 
 
     /**
      * Remove properties from the list.
+     *
      * @param props : properties to remove
      */
     protected void deleteProperties(Dictionary props) {
@@ -581,6 +610,7 @@ public class ProvidedService implements 
 
     /**
      * Get the published service specifications.
+     *
      * @return the list of provided service specifications (i.e. java
      * interface).
      */
@@ -590,6 +620,7 @@ public class ProvidedService implements 
 
     /**
      * Get the service registration.
+     *
      * @return the service registration of this service.
      */
     public ServiceRegistration getServiceRegistration() {
@@ -598,10 +629,11 @@ public class ProvidedService implements 
 
     /**
      * Sets the service controller on this provided service.
-     * @param field the field attached to this controller
-     * @param value the value the initial value
+     *
+     * @param field         the field attached to this controller
+     * @param value         the value the initial value
      * @param specification the target specification, if <code>null</code>
-     * affect all specifications.
+     *                      affect all specifications.
      */
     public void setController(String field, boolean value, String specification) {
         if (specification == null) {
@@ -614,12 +646,13 @@ public class ProvidedService implements 
 
     /**
      * Gets the service controller attached to the given field.
+     *
      * @param field the field name
      * @return the service controller, {@code null} if there is no service controller attached to the given field
      * name.
      */
     public ServiceController getController(String field) {
-        for (ServiceController controller: m_controllers.values()) {
+        for (ServiceController controller : m_controllers.values()) {
             if (field.equals(controller.m_field)) {
                 return controller;
             }
@@ -629,6 +662,7 @@ public class ProvidedService implements 
 
     /**
      * Gets the service controller handling the service publishing the given specification.
+     *
      * @param spec the specification (qualified class name)
      * @return the service controller, {@code null} if there is no service controller handling the service publishing
      * the given service specification
@@ -639,6 +673,7 @@ public class ProvidedService implements 
 
     /**
      * Checks if at least one service controller is valid.
+     *
      * @return <code>true</code> if one service controller at least
      * is valid.
      */
@@ -721,7 +756,7 @@ public class ProvidedService implements 
      * Remove the given listener from the provided service handler's list of listeners.
      *
      * @param listener the {@code ProvidedServiceListener} object to be removed
-     * @throws NullPointerException if {@code listener} is {@code null}
+     * @throws NullPointerException   if {@code listener} is {@code null}
      * @throws NoSuchElementException if {@code listener} wasn't present the in provided service handler's list of listeners
      */
     public void removeListener(ProvidedServiceListener listener) {
@@ -732,7 +767,7 @@ public class ProvidedService implements 
             // We definitely cannot rely on listener's equals method...
             // ...so we need to manually search for the listener, using ==.
             int i = -1;
-            for(int j = m_listeners.size() -1; j>=0 ; j--) {
+            for (int j = m_listeners.size() - 1; j >= 0; j--) {
                 if (m_listeners.get(j) == listener) {
                     // Found!
                     i = j;
@@ -804,6 +839,7 @@ public class ProvidedService implements 
 
         /**
          * Creates a ServiceController.
+         *
          * @param field the field
          * @param value the initial value
          */
@@ -818,6 +854,7 @@ public class ProvidedService implements 
 
         /**
          * Gets the value.
+         *
          * @return the value
          */
         public boolean getValue() {
@@ -828,6 +865,7 @@ public class ProvidedService implements 
 
         /**
          * Sets the value.
+         *
          * @param value the value
          */
         public void setValue(Boolean value) {
@@ -856,22 +894,27 @@ public class ProvidedService implements 
 
         /**
          * The service is going to be registered.
-         * @param instance the instance manager
+         *
+         * @param instance   the instance manager
          * @param interfaces the published interfaces
-         * @param props the properties
+         * @param props      the properties
          * @see org.apache.felix.ipojo.handlers.providedservice.CreationStrategy#onPublication(InstanceManager, java.lang.String[], java.util.Properties)
          */
         public void onPublication(InstanceManager instance, String[] interfaces,
-                Properties props) { }
+                                  Properties props) {
+        }
 
         /**
          * The service was unpublished.
+         *
          * @see org.apache.felix.ipojo.handlers.providedservice.CreationStrategy#onUnpublication()
          */
-        public void onUnpublication() { }
+        public void onUnpublication() {
+        }
 
         /**
          * A service object is required.
+         *
          * @param arg0 the bundle requiring the service object.
          * @param arg1 the service registration.
          * @return the first pojo object.
@@ -883,13 +926,14 @@ public class ProvidedService implements 
 
         /**
          * A service object is released.
-         * @param arg0  the bundle
+         *
+         * @param arg0 the bundle
          * @param arg1 the service registration
          * @param arg2 the get service object.
          * @see org.osgi.framework.ServiceFactory#ungetService(org.osgi.framework.Bundle, org.osgi.framework.ServiceRegistration, java.lang.Object)
          */
         public void ungetService(Bundle arg0, ServiceRegistration arg1,
-                Object arg2) {
+                                 Object arg2) {
         }
 
     }
@@ -902,24 +946,29 @@ public class ProvidedService implements 
 
         /**
          * The service is going to be registered.
-         * @param instance the instance manager
+         *
+         * @param instance   the instance manager
          * @param interfaces the published interfaces
-         * @param props the properties
+         * @param props      the properties
          * @see org.apache.felix.ipojo.handlers.providedservice.CreationStrategy#onPublication(InstanceManager, java.lang.String[], java.util.Properties)
          */
         public void onPublication(InstanceManager instance, String[] interfaces,
-                Properties props) { }
+                                  Properties props) {
+        }
 
         /**
          * The service is unpublished.
+         *
          * @see org.apache.felix.ipojo.handlers.providedservice.CreationStrategy#onUnpublication()
          */
-        public void onUnpublication() { }
+        public void onUnpublication() {
+        }
 
         /**
          * OSGi Service Factory getService method.
          * Returns a new service object per asking bundle.
          * This object is then cached by the framework.
+         *
          * @param arg0 the bundle requiring the service
          * @param arg1 the service registration
          * @return the service object for the asking bundle
@@ -932,13 +981,14 @@ public class ProvidedService implements 
         /**
          * OSGi Service Factory unget method.
          * Deletes the created object for the asking bundle.
+         *
          * @param arg0 the asking bundle
          * @param arg1 the service registration
          * @param arg2 the created service object returned for this bundle
          * @see org.osgi.framework.ServiceFactory#ungetService(org.osgi.framework.Bundle, org.osgi.framework.ServiceRegistration, java.lang.Object)
          */
         public void ungetService(Bundle arg0, ServiceRegistration arg1,
-                Object arg2) {
+                                 Object arg2) {
             m_handler.getInstanceManager().deletePojoObject(arg2);
         }
     }
@@ -964,6 +1014,7 @@ public class ProvidedService implements 
          * the service object is unget (i.e. removed from the map and deleted).
          * In all other cases, a {@link UnsupportedOperationException} is thrown as this policy
          * requires to use  the {@link IPOJOServiceFactory} interaction pattern.
+         *
          * @param arg0 the proxy object
          * @param arg1 the called method
          * @param arg2 the arguments
@@ -983,7 +1034,7 @@ public class ProvidedService implements 
             }
 
             // Regular methods from java.lang.Object : equals and hashCode
-            if (arg1.getName().equals("equals")  && arg2 != null  && arg2.length == 1) {
+            if (arg1.getName().equals("equals") && arg2 != null && arg2.length == 1) {
                 return this.equals(arg2[0]);
             }
 
@@ -999,6 +1050,7 @@ public class ProvidedService implements 
         /**
          * A service object is required.
          * This policy returns a service object per asking instance.
+         *
          * @param instance the instance requiring the service object
          * @return the service object for this instance
          * @see org.apache.felix.ipojo.IPOJOServiceFactory#getService(org.apache.felix.ipojo.ComponentInstance)
@@ -1015,7 +1067,8 @@ public class ProvidedService implements 
         /**
          * A service object is unget.
          * The service object is removed from the map and deleted.
-         * @param instance the instance releasing the service
+         *
+         * @param instance  the instance releasing the service
          * @param svcObject the service object
          * @see org.apache.felix.ipojo.IPOJOServiceFactory#ungetService(org.apache.felix.ipojo.ComponentInstance, java.lang.Object)
          */
@@ -1026,17 +1079,20 @@ public class ProvidedService implements 
 
         /**
          * The service is going to be registered.
-         * @param instance the instance manager
+         *
+         * @param instance   the instance manager
          * @param interfaces the published interfaces
-         * @param props the properties
+         * @param props      the properties
          * @see org.apache.felix.ipojo.handlers.providedservice.CreationStrategy#onPublication(InstanceManager, java.lang.String[], java.util.Properties)
          */
         public void onPublication(InstanceManager instance, String[] interfaces,
-                Properties props) { }
+                                  Properties props) {
+        }
 
         /**
          * The service is going to be unregistered.
          * The instance map is cleared. Created object are disposed.
+         *
          * @see org.apache.felix.ipojo.handlers.providedservice.CreationStrategy#onUnpublication()
          */
         public void onUnpublication() {
@@ -1050,6 +1106,7 @@ public class ProvidedService implements 
 
         /**
          * OSGi Service Factory getService method.
+         *
          * @param arg0 the asking bundle
          * @param arg1 the service registration
          * @return a proxy implementing the {@link IPOJOServiceFactory}
@@ -1064,19 +1121,22 @@ public class ProvidedService implements 
         /**
          * OSGi Service factory unget method.
          * Does nothing.
+         *
          * @param arg0 the asking bundle
          * @param arg1 the service registration
          * @param arg2 the service object created for this bundle.
          * @see org.osgi.framework.ServiceFactory#ungetService(org.osgi.framework.Bundle, org.osgi.framework.ServiceRegistration, java.lang.Object)
          */
         public void ungetService(Bundle arg0, ServiceRegistration arg1,
-                Object arg2) { }
+                                 Object arg2) {
+        }
 
         /**
          * Utility method returning the class array of provided service
          * specification and the {@link IPOJOServiceFactory} interface.
+         *
          * @param specs the published service interface
-         * @param bc the bundle context, used to load classes
+         * @param bc    the bundle context, used to load classes
          * @return the class array containing provided service specification and
          * the {@link IPOJOServiceFactory} class.
          */