You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@aries.apache.org by Mark Nuttall <mn...@apache.org> on 2012/07/24 11:33:26 UTC

Re: svn commit: r1364914 - in /aries/trunk/blueprint/blueprint-cm/src: main/java/org/apache/aries/blueprint/compendium/cm/ test/java/org/apache/aries/blueprint/compendium/cm/

Hi Guillaume,
Perhaps you've noticed all ready, but this change plus the three before it
is causing
org.apache.aries.blueprint.itests.TestConfigAdmin.testManagedServiceFactory
to fail, and so breaking the Hudson build. We saw this first in
https://builds.apache.org/job/Aries/1592/changes ; it can be seen again in
https://builds.apache.org/job/Aries/1593/ .

Please would you revert these changes, or fix the resulting test failure?
Thank you.

Regards,
Mark

On 24 July 2012 07:35, <gn...@apache.org> wrote:

> Author: gnodet
> Date: Tue Jul 24 06:35:34 2012
> New Revision: 1364914
>
> URL: http://svn.apache.org/viewvc?rev=1364914&view=rev
> Log:
> [ARIES-584] Refactor ManagedServiceFactory to avoid holding lock while
> registering/unregistering services and have a safer destruction mechanism
>
> Added:
>
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java
> Modified:
>
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java
>
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java
>
> aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java
>
> Added:
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java
> URL:
> http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java?rev=1364914&view=auto
>
> ==============================================================================
> ---
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java
> (added)
> +++
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java
> Tue Jul 24 06:35:34 2012
> @@ -0,0 +1,233 @@
> +/**
> + * 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.aries.blueprint.compendium.cm;
> +
> +import java.util.Dictionary;
> +import java.util.Map;
> +import java.util.concurrent.ConcurrentHashMap;
> +import java.util.concurrent.ExecutorService;
> +import java.util.concurrent.Executors;
> +import java.util.concurrent.TimeUnit;
> +import java.util.concurrent.atomic.AtomicBoolean;
> +
> +import org.apache.aries.blueprint.utils.JavaUtils;
> +import org.osgi.framework.BundleContext;
> +import org.osgi.framework.ServiceRegistration;
> +import org.osgi.service.cm.ConfigurationException;
> +import org.osgi.service.cm.ManagedServiceFactory;
> +import org.slf4j.Logger;
> +import org.slf4j.LoggerFactory;
> +
> +public abstract class BaseManagedServiceFactory<T> implements
> ManagedServiceFactory {
> +
> +    public static final long DEFAULT_TIMEOUT_BEFORE_INTERRUPT = 30000;
> +
> +    public static final int CONFIGURATION_ADMIN_OBJECT_DELETED = 1;
> +
> +    public static final int BUNDLE_STOPPING = 2;
> +
> +    public static final int INTERNAL_ERROR = 4;
> +
> +    protected final Logger LOGGER = LoggerFactory.getLogger(getClass());
> +
> +    private final BundleContext context;
> +    private final String name;
> +    private final long timeoutBeforeInterrupt;
> +    private final AtomicBoolean destroyed;
> +    private final ExecutorService executor;
> +    private final Map<String, Pair<T, ServiceRegistration>> services;
> +    private final Map<ServiceRegistration, T> registrations;
> +
> +    public BaseManagedServiceFactory(BundleContext context, String name) {
> +        this(context, name, DEFAULT_TIMEOUT_BEFORE_INTERRUPT);
> +    }
> +
> +    public BaseManagedServiceFactory(BundleContext context, String name,
> long timeoutBeforeInterrupt) {
> +        this.context = context;
> +        this.name = name;
> +        this.timeoutBeforeInterrupt = timeoutBeforeInterrupt;
> +        this.destroyed = new AtomicBoolean(false);
> +        this.executor = Executors.newSingleThreadExecutor();
> +        this.services = new ConcurrentHashMap<String, Pair<T,
> ServiceRegistration>>();
> +        this.registrations = new ConcurrentHashMap<ServiceRegistration,
> T>();
> +    }
> +
> +    public String getName() {
> +        return name;
> +    }
> +
> +    public Map<ServiceRegistration, T> getServices() {
> +        return registrations;
> +    }
> +
> +    public void updated(final String pid, final Dictionary properties)
> throws ConfigurationException {
> +        if (destroyed.get()) {
> +            return;
> +        }
> +        checkConfiguration(pid, properties);
> +        executor.submit(new Runnable() {
> +            public void run() {
> +                try {
> +                    internalUpdate(pid, properties);
> +                } catch (Throwable t) {
> +                    LOGGER.warn("Error destroying service for
> ManagedServiceFactory " + getName(), t);
> +                }
> +            }
> +        });
> +    }
> +
> +    public void deleted(final String pid) {
> +        if (destroyed.get()) {
> +            return;
> +        }
> +        executor.submit(new Runnable() {
> +            public void run() {
> +                try {
> +                    internalDelete(pid,
> CONFIGURATION_ADMIN_OBJECT_DELETED);
> +                } catch (Throwable throwable) {
> +                    LOGGER.warn("Error destroying service for
> ManagedServiceFactory " + getName(), throwable);
> +                }
> +            }
> +        });
> +    }
> +
> +    protected void checkConfiguration(String pid, Dictionary properties)
> throws ConfigurationException {
> +        // Do nothing
> +    }
> +
> +    protected abstract T doCreate(Dictionary properties) throws Exception;
> +
> +    protected abstract T doUpdate(T t, Dictionary properties) throws
> Exception;
> +
> +    protected abstract void doDestroy(T t, Dictionary properties, int
> code) throws Exception;
> +
> +    protected abstract String[] getExposedClasses(T t);
> +
> +    private void internalUpdate(String pid, Dictionary properties) {
> +        Pair<T, ServiceRegistration> pair = services.get(pid);
> +        if (pair != null) {
> +            try {
> +                T t = doUpdate(pair.getFirst(), properties);
> +                pair.setFirst(t);
> +                pair.getSecond().setProperties(properties);
> +            } catch (Throwable throwable) {
> +                internalDelete(pid, INTERNAL_ERROR);
> +                LOGGER.warn("Error updating service for
> ManagedServiceFactory " + getName(), throwable);
> +            }
> +        } else {
> +            if (destroyed.get()) {
> +                return;
> +            }
> +            try {
> +                T t = doCreate(properties);
> +                try {
> +                    if (destroyed.get()) {
> +                        throw new
> IllegalStateException("ManagedServiceFactory has been destroyed");
> +                    }
> +                    ServiceRegistration registration =
> context.registerService(getExposedClasses(t), t, properties);
> +                    services.put(pid, new Pair<T, ServiceRegistration>(t,
> registration));
> +                    registrations.put(registration,  t);
> +                    postRegister(t, properties, registration);
> +                } catch (Throwable throwable1) {
> +                    try {
> +                        doDestroy(t, properties, INTERNAL_ERROR);
> +                    } catch (Throwable throwable2) {
> +                        // Ignore
> +                    }
> +                    throw throwable1;
> +                }
> +            } catch (Throwable throwable) {
> +                LOGGER.warn("Error creating service for
> ManagedServiceFactory " + getName(), throwable);
> +            }
> +        }
> +    }
> +
> +    protected void postRegister(T t, Dictionary properties,
> ServiceRegistration registration) {
> +        // Place holder
> +    }
> +
> +    protected void preUnregister(T t, Dictionary properties,
> ServiceRegistration registration) {
> +        // Place holder
> +    }
> +
> +    private void internalDelete(String pid, int code) {
> +        Pair<T, ServiceRegistration> pair = services.remove(pid);
> +        if (pair != null) {
> +            registrations.remove(pair.getSecond());
> +            Dictionary properties =
> JavaUtils.getProperties(pair.getSecond().getReference());
> +            try {
> +                preUnregister(pair.getFirst(), properties,
> pair.getSecond());
> +                pair.getSecond().unregister();
> +            } catch (Throwable t) {
> +                LOGGER.info("Error unregistering service", t);
> +            }
> +            try {
> +                doDestroy(pair.getFirst(), properties, code);
> +            } catch (Throwable t) {
> +                LOGGER.info("Error destroying service", t);
> +            }
> +        }
> +    }
> +
> +    public void destroy() {
> +        if (destroyed.compareAndSet(false, true)) {
> +            executor.shutdown();
> +            try {
> +                executor.awaitTermination(timeoutBeforeInterrupt,
> TimeUnit.MILLISECONDS);
> +            } catch (InterruptedException e) {
> +                throw new RuntimeException("Shutdown interrupted");
> +            }
> +            if (!executor.isTerminated()) {
> +                executor.shutdownNow();
> +                try {
> +                    executor.awaitTermination(Long.MAX_VALUE,
> TimeUnit.NANOSECONDS);
> +                } catch (InterruptedException e) {
> +                    throw new RuntimeException("Shutdown interrupted");
> +                }
> +            }
> +
> +            while (!services.isEmpty()) {
> +                String pid = services.keySet().iterator().next();
> +                internalDelete(pid, BUNDLE_STOPPING);
> +            }
> +        }
> +    }
> +
> +    static class Pair<U,V> {
> +        private U first;
> +        private V second;
> +        public Pair(U first, V second) {
> +            this.first = first;
> +            this.second = second;
> +        }
> +        public U getFirst() {
> +            return first;
> +        }
> +        public V getSecond() {
> +            return second;
> +        }
> +        public void setFirst(U first) {
> +            this.first = first;
> +        }
> +        public void setSecond(V second) {
> +            this.second = second;
> +        }
> +    }
> +
> +}
>
> Modified:
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java
> URL:
> http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java?rev=1364914&r1=1364913&r2=1364914&view=diff
>
> ==============================================================================
> ---
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java
> (original)
> +++
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java
> Tue Jul 24 06:35:34 2012
> @@ -27,23 +27,18 @@ import java.util.List;
>  import java.util.Map;
>  import java.util.Properties;
>  import java.util.Set;
> -import java.util.concurrent.ConcurrentHashMap;
>
>  import org.apache.aries.blueprint.BeanProcessor;
> -import org.apache.aries.blueprint.services.ExtendedBlueprintContainer;
>  import org.apache.aries.blueprint.ServiceProcessor;
> +import org.apache.aries.blueprint.services.ExtendedBlueprintContainer;
>  import org.apache.aries.blueprint.utils.JavaUtils;
>  import org.apache.aries.blueprint.utils.ReflectionUtils;
>  import org.apache.aries.blueprint.utils.ServiceListener;
>  import org.apache.aries.util.AriesFrameworkUtil;
>  import org.osgi.framework.Bundle;
>  import org.osgi.framework.Constants;
> -import org.osgi.framework.ServiceReference;
>  import org.osgi.framework.ServiceRegistration;
>  import org.osgi.service.blueprint.reflect.ServiceMetadata;
> -import org.osgi.service.cm.Configuration;
> -import org.osgi.service.cm.ConfigurationAdmin;
> -import org.osgi.service.cm.ConfigurationException;
>  import org.osgi.service.cm.ManagedServiceFactory;
>  import org.slf4j.Logger;
>  import org.slf4j.LoggerFactory;
> @@ -54,30 +49,27 @@ import org.slf4j.LoggerFactory;
>   *
>   * @version $Rev$, $Date$
>   */
> -public class CmManagedServiceFactory {
> -
> -    static final int CONFIGURATION_ADMIN_OBJECT_DELETED = 1;
> -
> -    static final int BUNDLE_STOPPING = 2;
> +public class CmManagedServiceFactory extends
> BaseManagedServiceFactory<Object> {
>
>      private static final Logger LOGGER =
> LoggerFactory.getLogger(CmManagedServiceFactory.class);
> -
> +
>      private ExtendedBlueprintContainer blueprintContainer;
> -    private ConfigurationAdmin configAdmin;
>      private String id;
>      private String factoryPid;
>      private List<String> interfaces;
>      private int autoExport;
>      private int ranking;
> -    private Map serviceProperties;
> +    private Map<Object,Object> serviceProperties;
>      private String managedComponentName;
>      private String componentDestroyMethod;
>      private List<ServiceListener> listeners;
> -    private final Object lock = new Object();
>
>      private ServiceRegistration registration;
> -    private final Map<String, ServiceRegistration> pids = new
> ConcurrentHashMap<String, ServiceRegistration>();
> -    private final Map<ServiceRegistration, Object> services = new
> ConcurrentHashMap<ServiceRegistration, Object>();
> +
> +    public CmManagedServiceFactory(ExtendedBlueprintContainer
> blueprintContainer) {
> +        super(blueprintContainer.getBundleContext(), null);
> +        this.blueprintContainer = blueprintContainer;
> +    }
>
>      public void init() throws Exception {
>          LOGGER.debug("Initializing CmManagedServiceFactory for
> factoryPid={}", factoryPid);
> @@ -86,61 +78,27 @@ public class CmManagedServiceFactory {
>          Bundle bundle = blueprintContainer.getBundleContext().getBundle();
>          props.put(Constants.BUNDLE_SYMBOLICNAME,
> bundle.getSymbolicName());
>          props.put(Constants.BUNDLE_VERSION,
> bundle.getHeaders().get(Constants.BUNDLE_VERSION));
> -
> -        synchronized(lock) {
> -            registration =
> blueprintContainer.getBundleContext().registerService(ManagedServiceFactory.class.getName(),
> new ConfigurationWatcher(), props);
> -
> -            String filter = '(' + ConfigurationAdmin.SERVICE_FACTORYPID +
> '=' + this.factoryPid + ')';
> -            Configuration[] configs =
> configAdmin.listConfigurations(filter);
> -            if (configs != null) {
> -                for (Configuration config : configs) {
> -                    updated(config.getPid(), config.getProperties());
> -                }
> -            }
> -        }
> +
> +        registration =
> blueprintContainer.getBundleContext().registerService(ManagedServiceFactory.class.getName(),
> this, props);
>      }
>
>      public void destroy() {
>          AriesFrameworkUtil.safeUnregisterService(registration);
> -        for (Map.Entry<ServiceRegistration, Object> entry :
> services.entrySet()) {
> -            destroy(entry.getValue(), entry.getKey(), BUNDLE_STOPPING);
> -        }
> -        services.clear();
> -        pids.clear();
> +        super.destroy();
>      }
>
> -    private void destroy(Object component, ServiceRegistration
> registration, int code) {
> -        if (listeners != null) {
> -            ServiceReference ref = registration.getReference();
> -            for (ServiceListener listener : listeners) {
> -                Hashtable props = JavaUtils.getProperties(ref);
> -                listener.unregister(component, props);
> -            }
> -        }
> -        destroyComponent(component, code);
> -        AriesFrameworkUtil.safeUnregisterService(registration);
> -    }
> -
>      public Map<ServiceRegistration, Object> getServiceMap() {
> -        return Collections.unmodifiableMap(services);
> -    }
> -
> -    public void setBlueprintContainer(ExtendedBlueprintContainer
> blueprintContainer) {
> -        this.blueprintContainer = blueprintContainer;
> -    }
> -
> -    public void setConfigAdmin(ConfigurationAdmin configAdmin) {
> -        this.configAdmin = configAdmin;
> +        return Collections.unmodifiableMap(getServices());
>      }
>
>      public void setListeners(List<ServiceListener> listeners) {
>          this.listeners = listeners;
>      }
> -
> +
>      public void setId(String id) {
>          this.id = id;
>      }
> -
> +
>      public void setFactoryPid(String factoryPid) {
>          this.factoryPid = factoryPid;
>      }
> @@ -160,7 +118,7 @@ public class CmManagedServiceFactory {
>      public void setServiceProperties(Map serviceProperties) {
>          this.serviceProperties = serviceProperties;
>      }
> -
> +
>      public void setManagedComponentName(String managedComponentName) {
>          this.managedComponentName = managedComponentName;
>      }
> @@ -168,82 +126,42 @@ public class CmManagedServiceFactory {
>      public void setComponentDestroyMethod(String componentDestroyMethod) {
>          this.componentDestroyMethod = componentDestroyMethod;
>      }
> -
> -    protected void updated(String pid, Dictionary props) {
> -      LOGGER.debug("Updated configuration {} with props {}", pid, props);
> -
> -      Hashtable regProps = null;
> -      Object component = null;
> -
> -      // This method might be multithreaded, so synchronize checking and
> -      // creating the service
> -      final ServiceRegistration existingReg;
> -      synchronized (pids) {
> -         existingReg = pids.get(pid);
> -         if (existingReg == null) {
> -            updateComponentProperties(props);
> -
> -            component =
> blueprintContainer.getComponentInstance(managedComponentName);
> -
> -            // TODO: call listeners, etc...
> -
> -            regProps = getRegistrationProperties(pid);
> -            CmProperties cm = findServiceProcessor();
> -            if (cm != null) {
> -               if ("".equals(cm.getPersistentId())) {
> -                  JavaUtils.copy(regProps, props);
> -               }
> -               cm.updateProperties(new PropertiesUpdater(pid), regProps);
> -            }
>
> -            Set<String> classes = getClasses(component);
> -            String[] classArray = classes.toArray(new
> String[classes.size()]);
> -            ServiceRegistration reg =
> blueprintContainer.getBundleContext().registerService(classArray,
> component, regProps);
> -
> -            LOGGER.debug("Service {} registered with interfaces {} and
> properties {}", new Object[] { component, classes, regProps });
> -
> -            services.put(reg, component);
> -            pids.put(pid, reg);
> -         }
> -        } // end of synchronization
> -
> -        // If we just registered a service, do the slower stuff outside
> the synchronized block
> -        if (existingReg == null)
> -        {
> -            if (listeners != null) {
> -                for (ServiceListener listener : listeners) {
> -                    listener.register(component, regProps);
> +    private void getRegistrationProperties(Dictionary properties, boolean
> update) {
> +        CmProperties cm = findServiceProcessor();
> +        if (cm == null) {
> +            while (!properties.isEmpty()) {
> +                properties.remove(properties.keys().nextElement());
> +            }
> +        } else  {
> +            if (!cm.getUpdate()) {
> +                if (update) {
> +                    while (!properties.isEmpty()) {
> +
>  properties.remove(properties.keys().nextElement());
> +                    }
> +                    for (Map.Entry entry : cm.getProperties().entrySet())
> {
> +                        properties.put(entry.getKey(), entry.getValue());
> +                    }
> +                } else {
> +                    cm.updated(properties);
>                  }
>              }
> -        } else {
> -            updateComponentProperties(props);
> -
> -            CmProperties cm = findServiceProcessor();
> -            if (cm != null && "".equals(cm.getPersistentId())) {
> -                regProps = getRegistrationProperties(pid);
> -                JavaUtils.copy(regProps, props);
> -                cm.updated(regProps);
> -            }
>          }
> -    }
> -
> -    private Hashtable getRegistrationProperties(String pid) {
> -        Hashtable regProps = new Hashtable();
>          if (serviceProperties != null) {
> -            regProps.putAll(serviceProperties);
> +            for (Map.Entry entry : serviceProperties.entrySet()) {
> +                properties.put(entry.getKey(), entry.getValue());
> +            }
>          }
> -        regProps.put(Constants.SERVICE_PID, pid);
> -        regProps.put(Constants.SERVICE_RANKING, ranking);
> -        return regProps;
> +        properties.put(Constants.SERVICE_RANKING, ranking);
>      }
> -
> +
>      private void updateComponentProperties(Dictionary props) {
>          CmManagedProperties cm = findBeanProcessor();
>          if (cm != null) {
>              cm.updated(props);
>          }
>      }
> -
> +
>      private CmManagedProperties findBeanProcessor() {
>          for (BeanProcessor beanProcessor :
> blueprintContainer.getProcessors(BeanProcessor.class)) {
>              if (beanProcessor instanceof CmManagedProperties) {
> @@ -255,7 +173,7 @@ public class CmManagedServiceFactory {
>          }
>          return null;
>      }
> -
> +
>      private CmProperties findServiceProcessor() {
>          for (ServiceProcessor processor :
> blueprintContainer.getProcessors(ServiceProcessor.class)) {
>              if (processor instanceof CmProperties) {
> @@ -267,20 +185,9 @@ public class CmManagedServiceFactory {
>          }
>          return null;
>      }
> -
> -    private void destroyComponent(Object instance, int reason) {
> -        Method method = findDestroyMethod(instance.getClass());
> -        if (method != null) {
> -            try {
> -                method.invoke(instance, new Object [] { reason });
> -            } catch (Exception e) {
> -                e.printStackTrace();
> -            }
> -        }
> -    }
> -
> +
>      private Method findDestroyMethod(Class clazz) {
> -        Method method = null;
> +        Method method = null;
>          if (componentDestroyMethod != null &&
> componentDestroyMethod.length() > 0) {
>              List<Method> methods =
> ReflectionUtils.findCompatibleMethods(clazz, componentDestroyMethod, new
> Class [] { int.class });
>              if (methods != null && !methods.isEmpty()) {
> @@ -289,17 +196,52 @@ public class CmManagedServiceFactory {
>          }
>          return method;
>      }
> -
> -    protected void deleted(String pid) {
> -        LOGGER.debug("Deleted configuration {}", pid);
> -        ServiceRegistration reg = pids.remove(pid);
> -        if (reg != null) {
> -            Object component = services.remove(reg);
> -            destroy(component, reg, CONFIGURATION_ADMIN_OBJECT_DELETED);
> +
> +    protected Object doCreate(Dictionary properties) throws Exception {
> +        updateComponentProperties(copy(properties));
> +        Object component =
> blueprintContainer.getComponentInstance(managedComponentName);
> +        getRegistrationProperties(properties, false);
> +        return component;
> +    }
> +
> +    protected Object doUpdate(Object service, Dictionary properties)
> throws Exception {
> +        updateComponentProperties(copy(properties));
> +        getRegistrationProperties(properties, true);
> +        return service;
> +    }
> +
> +    protected void doDestroy(Object service, Dictionary properties, int
> code) throws Exception {
> +        Method method = findDestroyMethod(service.getClass());
> +        if (method != null) {
> +            try {
> +                method.invoke(service, new Object [] { code });
> +            } catch (Exception e) {
> +                LOGGER.info("Error destroying component", e);
> +            }
> +        }
> +    }
> +
> +    protected void postRegister(Object service, Dictionary properties,
> ServiceRegistration registration) {
> +        if (listeners != null && !listeners.isEmpty()) {
> +            Hashtable props = new Hashtable();
> +            JavaUtils.copy(properties, props);
> +            for (ServiceListener listener : listeners) {
> +                listener.register(service, props);
> +            }
> +        }
> +    }
> +
> +    protected void preUnregister(Object service, Dictionary properties,
> ServiceRegistration registration) {
> +        if (listeners != null && !listeners.isEmpty()) {
> +            Hashtable props = new Hashtable();
> +            JavaUtils.copy(properties, props);
> +            for (ServiceListener listener : listeners) {
> +                listener.unregister(service, props);
> +            }
>          }
>      }
>
> -    private Set<String> getClasses(Object service) {
> +    protected String[] getExposedClasses(Object service) {
>          Class serviceClass = service.getClass();
>          Set<String> classes;
>          switch (autoExport) {
> @@ -317,47 +259,13 @@ public class CmManagedServiceFactory {
>                  classes = new HashSet<String>(interfaces);
>                  break;
>          }
> -        return classes;
> +        return classes.toArray(new String[classes.size()]);
>      }
> -
> -    private class ConfigurationWatcher implements ManagedServiceFactory {
> -
> -        public String getName() {
> -            return null;
> -        }
> -
> -        public void updated(String pid, Dictionary props) throws
> ConfigurationException {
> -            CmManagedServiceFactory.this.updated(pid, props);
> -        }
>
> -        public void deleted(String pid) {
> -            CmManagedServiceFactory.this.deleted(pid);
> -        }
> +    private Hashtable copy(Dictionary source) {
> +        Hashtable ht = new Hashtable();
> +        JavaUtils.copy(ht, source);
> +        return ht;
>      }
>
> -    private class PropertiesUpdater implements
> ServiceProcessor.ServicePropertiesUpdater {
> -
> -        private String pid;
> -
> -        public PropertiesUpdater(String pid) {
> -            this.pid = pid;
> -        }
> -
> -        public String getId() {
> -            return id;
> -        }
> -
> -        public void updateProperties(Dictionary properties) {
> -            ServiceRegistration reg = pids.get(pid);
> -            if (reg != null) {
> -                ServiceReference ref = reg.getReference();
> -                if (ref != null) {
> -                    Hashtable table = JavaUtils.getProperties(ref);
> -                    JavaUtils.copy(table, properties);
> -                    reg.setProperties(table);
> -                }
> -            }
> -        }
> -    }
> -
>  }
>
> Modified:
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java
> URL:
> http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java?rev=1364914&r1=1364913&r2=1364914&view=diff
>
> ==============================================================================
> ---
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java
> (original)
> +++
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java
> Tue Jul 24 06:35:34 2012
> @@ -350,8 +350,7 @@ public class CmNamespaceHandler implemen
>          factoryMetadata.setRuntimeClass(CmManagedServiceFactory.class);
>          factoryMetadata.setInitMethod("init");
>          factoryMetadata.setDestroyMethod("destroy");
> -        factoryMetadata.addProperty("configAdmin",
> createConfigurationAdminRef(context));
> -        factoryMetadata.addProperty("blueprintContainer",
> createRef(context, "blueprintContainer"));
> +        factoryMetadata.addArgument(createRef(context,
> "blueprintContainer"), null, 0);
>          factoryMetadata.addProperty("factoryPid", createValue(context,
> element.getAttribute(FACTORY_PID_ATTRIBUTE)));
>          String autoExport = element.hasAttribute(AUTO_EXPORT_ATTRIBUTE) ?
> element.getAttribute(AUTO_EXPORT_ATTRIBUTE) : AUTO_EXPORT_DEFAULT;
>          if (AUTO_EXPORT_DISABLED.equals(autoExport)) {
>
> Modified:
> aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java
> URL:
> http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java?rev=1364914&r1=1364913&r2=1364914&view=diff
>
> ==============================================================================
> ---
> aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java
> (original)
> +++
> aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java
> Tue Jul 24 06:35:34 2012
> @@ -56,6 +56,7 @@ public class ManagedServiceFactoryTest e
>          assertNull(sr.getProperty("b"));
>
>          props = new Hashtable<String,String>();
> +        props.put("a", "5");
>          props.put("b", "foo");
>          cf.update(props);
>          Thread.sleep(500);
> @@ -89,6 +90,7 @@ public class ManagedServiceFactoryTest e
>          assertNull(sr.getProperty("b"));
>
>          props = new Hashtable<String,String>();
> +        props.put("a", "5");
>          props.put("b", "foo");
>          cf.update(props);
>
> @@ -122,6 +124,7 @@ public class ManagedServiceFactoryTest e
>          assertNull(sr.getProperty("b"));
>
>          props = new Hashtable<String,String>();
> +        props.put("a", "5");
>          props.put("b", "foo");
>          cf.update(props);
>
>
>
>

Re: svn commit: r1364914 - in /aries/trunk/blueprint/blueprint-cm/src: main/java/org/apache/aries/blueprint/compendium/cm/ test/java/org/apache/aries/blueprint/compendium/cm/

Posted by Mark Nuttall <mn...@apache.org>.
Fixed in
https://builds.apache.org/job/Aries/org.apache.aries.blueprint$org.apache.aries.blueprint.itests/1597/
.
Thank you!

-- Mark

On 24 July 2012 10:53, Guillaume Nodet <gn...@gmail.com> wrote:

> Sure, I'll investigate.  They seem to have passed correctly on my machine,
> so it may be a timing issue.
>
> On Tue, Jul 24, 2012 at 11:33 AM, Mark Nuttall <mn...@apache.org>
> wrote:
>
> > Hi Guillaume,
> > Perhaps you've noticed all ready, but this change plus the three before
> it
> > is causing
> >
> org.apache.aries.blueprint.itests.TestConfigAdmin.testManagedServiceFactory
> > to fail, and so breaking the Hudson build. We saw this first in
> > https://builds.apache.org/job/Aries/1592/changes ; it can be seen again
> in
> > https://builds.apache.org/job/Aries/1593/ .
> >
> > Please would you revert these changes, or fix the resulting test failure?
> > Thank you.
> >
> > Regards,
> > Mark
> >
> > On 24 July 2012 07:35, <gn...@apache.org> wrote:
> >
> > > Author: gnodet
> > > Date: Tue Jul 24 06:35:34 2012
> > > New Revision: 1364914
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1364914&view=rev
> > > Log:
> > > [ARIES-584] Refactor ManagedServiceFactory to avoid holding lock while
> > > registering/unregistering services and have a safer destruction
> mechanism
> > >
> > > Added:
> > >
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java
> > > Modified:
> > >
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java
> > >
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java
> > >
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java
> > >
> > > Added:
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java?rev=1364914&view=auto
> > >
> > >
> >
> ==============================================================================
> > > ---
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java
> > > (added)
> > > +++
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java
> > > Tue Jul 24 06:35:34 2012
> > > @@ -0,0 +1,233 @@
> > > +/**
> > > + * 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.aries.blueprint.compendium.cm;
> > > +
> > > +import java.util.Dictionary;
> > > +import java.util.Map;
> > > +import java.util.concurrent.ConcurrentHashMap;
> > > +import java.util.concurrent.ExecutorService;
> > > +import java.util.concurrent.Executors;
> > > +import java.util.concurrent.TimeUnit;
> > > +import java.util.concurrent.atomic.AtomicBoolean;
> > > +
> > > +import org.apache.aries.blueprint.utils.JavaUtils;
> > > +import org.osgi.framework.BundleContext;
> > > +import org.osgi.framework.ServiceRegistration;
> > > +import org.osgi.service.cm.ConfigurationException;
> > > +import org.osgi.service.cm.ManagedServiceFactory;
> > > +import org.slf4j.Logger;
> > > +import org.slf4j.LoggerFactory;
> > > +
> > > +public abstract class BaseManagedServiceFactory<T> implements
> > > ManagedServiceFactory {
> > > +
> > > +    public static final long DEFAULT_TIMEOUT_BEFORE_INTERRUPT = 30000;
> > > +
> > > +    public static final int CONFIGURATION_ADMIN_OBJECT_DELETED = 1;
> > > +
> > > +    public static final int BUNDLE_STOPPING = 2;
> > > +
> > > +    public static final int INTERNAL_ERROR = 4;
> > > +
> > > +    protected final Logger LOGGER =
> LoggerFactory.getLogger(getClass());
> > > +
> > > +    private final BundleContext context;
> > > +    private final String name;
> > > +    private final long timeoutBeforeInterrupt;
> > > +    private final AtomicBoolean destroyed;
> > > +    private final ExecutorService executor;
> > > +    private final Map<String, Pair<T, ServiceRegistration>> services;
> > > +    private final Map<ServiceRegistration, T> registrations;
> > > +
> > > +    public BaseManagedServiceFactory(BundleContext context, String
> > name) {
> > > +        this(context, name, DEFAULT_TIMEOUT_BEFORE_INTERRUPT);
> > > +    }
> > > +
> > > +    public BaseManagedServiceFactory(BundleContext context, String
> name,
> > > long timeoutBeforeInterrupt) {
> > > +        this.context = context;
> > > +        this.name = name;
> > > +        this.timeoutBeforeInterrupt = timeoutBeforeInterrupt;
> > > +        this.destroyed = new AtomicBoolean(false);
> > > +        this.executor = Executors.newSingleThreadExecutor();
> > > +        this.services = new ConcurrentHashMap<String, Pair<T,
> > > ServiceRegistration>>();
> > > +        this.registrations = new
> ConcurrentHashMap<ServiceRegistration,
> > > T>();
> > > +    }
> > > +
> > > +    public String getName() {
> > > +        return name;
> > > +    }
> > > +
> > > +    public Map<ServiceRegistration, T> getServices() {
> > > +        return registrations;
> > > +    }
> > > +
> > > +    public void updated(final String pid, final Dictionary properties)
> > > throws ConfigurationException {
> > > +        if (destroyed.get()) {
> > > +            return;
> > > +        }
> > > +        checkConfiguration(pid, properties);
> > > +        executor.submit(new Runnable() {
> > > +            public void run() {
> > > +                try {
> > > +                    internalUpdate(pid, properties);
> > > +                } catch (Throwable t) {
> > > +                    LOGGER.warn("Error destroying service for
> > > ManagedServiceFactory " + getName(), t);
> > > +                }
> > > +            }
> > > +        });
> > > +    }
> > > +
> > > +    public void deleted(final String pid) {
> > > +        if (destroyed.get()) {
> > > +            return;
> > > +        }
> > > +        executor.submit(new Runnable() {
> > > +            public void run() {
> > > +                try {
> > > +                    internalDelete(pid,
> > > CONFIGURATION_ADMIN_OBJECT_DELETED);
> > > +                } catch (Throwable throwable) {
> > > +                    LOGGER.warn("Error destroying service for
> > > ManagedServiceFactory " + getName(), throwable);
> > > +                }
> > > +            }
> > > +        });
> > > +    }
> > > +
> > > +    protected void checkConfiguration(String pid, Dictionary
> properties)
> > > throws ConfigurationException {
> > > +        // Do nothing
> > > +    }
> > > +
> > > +    protected abstract T doCreate(Dictionary properties) throws
> > Exception;
> > > +
> > > +    protected abstract T doUpdate(T t, Dictionary properties) throws
> > > Exception;
> > > +
> > > +    protected abstract void doDestroy(T t, Dictionary properties, int
> > > code) throws Exception;
> > > +
> > > +    protected abstract String[] getExposedClasses(T t);
> > > +
> > > +    private void internalUpdate(String pid, Dictionary properties) {
> > > +        Pair<T, ServiceRegistration> pair = services.get(pid);
> > > +        if (pair != null) {
> > > +            try {
> > > +                T t = doUpdate(pair.getFirst(), properties);
> > > +                pair.setFirst(t);
> > > +                pair.getSecond().setProperties(properties);
> > > +            } catch (Throwable throwable) {
> > > +                internalDelete(pid, INTERNAL_ERROR);
> > > +                LOGGER.warn("Error updating service for
> > > ManagedServiceFactory " + getName(), throwable);
> > > +            }
> > > +        } else {
> > > +            if (destroyed.get()) {
> > > +                return;
> > > +            }
> > > +            try {
> > > +                T t = doCreate(properties);
> > > +                try {
> > > +                    if (destroyed.get()) {
> > > +                        throw new
> > > IllegalStateException("ManagedServiceFactory has been destroyed");
> > > +                    }
> > > +                    ServiceRegistration registration =
> > > context.registerService(getExposedClasses(t), t, properties);
> > > +                    services.put(pid, new Pair<T,
> > ServiceRegistration>(t,
> > > registration));
> > > +                    registrations.put(registration,  t);
> > > +                    postRegister(t, properties, registration);
> > > +                } catch (Throwable throwable1) {
> > > +                    try {
> > > +                        doDestroy(t, properties, INTERNAL_ERROR);
> > > +                    } catch (Throwable throwable2) {
> > > +                        // Ignore
> > > +                    }
> > > +                    throw throwable1;
> > > +                }
> > > +            } catch (Throwable throwable) {
> > > +                LOGGER.warn("Error creating service for
> > > ManagedServiceFactory " + getName(), throwable);
> > > +            }
> > > +        }
> > > +    }
> > > +
> > > +    protected void postRegister(T t, Dictionary properties,
> > > ServiceRegistration registration) {
> > > +        // Place holder
> > > +    }
> > > +
> > > +    protected void preUnregister(T t, Dictionary properties,
> > > ServiceRegistration registration) {
> > > +        // Place holder
> > > +    }
> > > +
> > > +    private void internalDelete(String pid, int code) {
> > > +        Pair<T, ServiceRegistration> pair = services.remove(pid);
> > > +        if (pair != null) {
> > > +            registrations.remove(pair.getSecond());
> > > +            Dictionary properties =
> > > JavaUtils.getProperties(pair.getSecond().getReference());
> > > +            try {
> > > +                preUnregister(pair.getFirst(), properties,
> > > pair.getSecond());
> > > +                pair.getSecond().unregister();
> > > +            } catch (Throwable t) {
> > > +                LOGGER.info("Error unregistering service", t);
> > > +            }
> > > +            try {
> > > +                doDestroy(pair.getFirst(), properties, code);
> > > +            } catch (Throwable t) {
> > > +                LOGGER.info("Error destroying service", t);
> > > +            }
> > > +        }
> > > +    }
> > > +
> > > +    public void destroy() {
> > > +        if (destroyed.compareAndSet(false, true)) {
> > > +            executor.shutdown();
> > > +            try {
> > > +                executor.awaitTermination(timeoutBeforeInterrupt,
> > > TimeUnit.MILLISECONDS);
> > > +            } catch (InterruptedException e) {
> > > +                throw new RuntimeException("Shutdown interrupted");
> > > +            }
> > > +            if (!executor.isTerminated()) {
> > > +                executor.shutdownNow();
> > > +                try {
> > > +                    executor.awaitTermination(Long.MAX_VALUE,
> > > TimeUnit.NANOSECONDS);
> > > +                } catch (InterruptedException e) {
> > > +                    throw new RuntimeException("Shutdown
> interrupted");
> > > +                }
> > > +            }
> > > +
> > > +            while (!services.isEmpty()) {
> > > +                String pid = services.keySet().iterator().next();
> > > +                internalDelete(pid, BUNDLE_STOPPING);
> > > +            }
> > > +        }
> > > +    }
> > > +
> > > +    static class Pair<U,V> {
> > > +        private U first;
> > > +        private V second;
> > > +        public Pair(U first, V second) {
> > > +            this.first = first;
> > > +            this.second = second;
> > > +        }
> > > +        public U getFirst() {
> > > +            return first;
> > > +        }
> > > +        public V getSecond() {
> > > +            return second;
> > > +        }
> > > +        public void setFirst(U first) {
> > > +            this.first = first;
> > > +        }
> > > +        public void setSecond(V second) {
> > > +            this.second = second;
> > > +        }
> > > +    }
> > > +
> > > +}
> > >
> > > Modified:
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java?rev=1364914&r1=1364913&r2=1364914&view=diff
> > >
> > >
> >
> ==============================================================================
> > > ---
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java
> > > (original)
> > > +++
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java
> > > Tue Jul 24 06:35:34 2012
> > > @@ -27,23 +27,18 @@ import java.util.List;
> > >  import java.util.Map;
> > >  import java.util.Properties;
> > >  import java.util.Set;
> > > -import java.util.concurrent.ConcurrentHashMap;
> > >
> > >  import org.apache.aries.blueprint.BeanProcessor;
> > > -import org.apache.aries.blueprint.services.ExtendedBlueprintContainer;
> > >  import org.apache.aries.blueprint.ServiceProcessor;
> > > +import org.apache.aries.blueprint.services.ExtendedBlueprintContainer;
> > >  import org.apache.aries.blueprint.utils.JavaUtils;
> > >  import org.apache.aries.blueprint.utils.ReflectionUtils;
> > >  import org.apache.aries.blueprint.utils.ServiceListener;
> > >  import org.apache.aries.util.AriesFrameworkUtil;
> > >  import org.osgi.framework.Bundle;
> > >  import org.osgi.framework.Constants;
> > > -import org.osgi.framework.ServiceReference;
> > >  import org.osgi.framework.ServiceRegistration;
> > >  import org.osgi.service.blueprint.reflect.ServiceMetadata;
> > > -import org.osgi.service.cm.Configuration;
> > > -import org.osgi.service.cm.ConfigurationAdmin;
> > > -import org.osgi.service.cm.ConfigurationException;
> > >  import org.osgi.service.cm.ManagedServiceFactory;
> > >  import org.slf4j.Logger;
> > >  import org.slf4j.LoggerFactory;
> > > @@ -54,30 +49,27 @@ import org.slf4j.LoggerFactory;
> > >   *
> > >   * @version $Rev$, $Date$
> > >   */
> > > -public class CmManagedServiceFactory {
> > > -
> > > -    static final int CONFIGURATION_ADMIN_OBJECT_DELETED = 1;
> > > -
> > > -    static final int BUNDLE_STOPPING = 2;
> > > +public class CmManagedServiceFactory extends
> > > BaseManagedServiceFactory<Object> {
> > >
> > >      private static final Logger LOGGER =
> > > LoggerFactory.getLogger(CmManagedServiceFactory.class);
> > > -
> > > +
> > >      private ExtendedBlueprintContainer blueprintContainer;
> > > -    private ConfigurationAdmin configAdmin;
> > >      private String id;
> > >      private String factoryPid;
> > >      private List<String> interfaces;
> > >      private int autoExport;
> > >      private int ranking;
> > > -    private Map serviceProperties;
> > > +    private Map<Object,Object> serviceProperties;
> > >      private String managedComponentName;
> > >      private String componentDestroyMethod;
> > >      private List<ServiceListener> listeners;
> > > -    private final Object lock = new Object();
> > >
> > >      private ServiceRegistration registration;
> > > -    private final Map<String, ServiceRegistration> pids = new
> > > ConcurrentHashMap<String, ServiceRegistration>();
> > > -    private final Map<ServiceRegistration, Object> services = new
> > > ConcurrentHashMap<ServiceRegistration, Object>();
> > > +
> > > +    public CmManagedServiceFactory(ExtendedBlueprintContainer
> > > blueprintContainer) {
> > > +        super(blueprintContainer.getBundleContext(), null);
> > > +        this.blueprintContainer = blueprintContainer;
> > > +    }
> > >
> > >      public void init() throws Exception {
> > >          LOGGER.debug("Initializing CmManagedServiceFactory for
> > > factoryPid={}", factoryPid);
> > > @@ -86,61 +78,27 @@ public class CmManagedServiceFactory {
> > >          Bundle bundle =
> > blueprintContainer.getBundleContext().getBundle();
> > >          props.put(Constants.BUNDLE_SYMBOLICNAME,
> > > bundle.getSymbolicName());
> > >          props.put(Constants.BUNDLE_VERSION,
> > > bundle.getHeaders().get(Constants.BUNDLE_VERSION));
> > > -
> > > -        synchronized(lock) {
> > > -            registration =
> > >
> >
> blueprintContainer.getBundleContext().registerService(ManagedServiceFactory.class.getName(),
> > > new ConfigurationWatcher(), props);
> > > -
> > > -            String filter = '(' +
> ConfigurationAdmin.SERVICE_FACTORYPID
> > +
> > > '=' + this.factoryPid + ')';
> > > -            Configuration[] configs =
> > > configAdmin.listConfigurations(filter);
> > > -            if (configs != null) {
> > > -                for (Configuration config : configs) {
> > > -                    updated(config.getPid(), config.getProperties());
> > > -                }
> > > -            }
> > > -        }
> > > +
> > > +        registration =
> > >
> >
> blueprintContainer.getBundleContext().registerService(ManagedServiceFactory.class.getName(),
> > > this, props);
> > >      }
> > >
> > >      public void destroy() {
> > >          AriesFrameworkUtil.safeUnregisterService(registration);
> > > -        for (Map.Entry<ServiceRegistration, Object> entry :
> > > services.entrySet()) {
> > > -            destroy(entry.getValue(), entry.getKey(),
> BUNDLE_STOPPING);
> > > -        }
> > > -        services.clear();
> > > -        pids.clear();
> > > +        super.destroy();
> > >      }
> > >
> > > -    private void destroy(Object component, ServiceRegistration
> > > registration, int code) {
> > > -        if (listeners != null) {
> > > -            ServiceReference ref = registration.getReference();
> > > -            for (ServiceListener listener : listeners) {
> > > -                Hashtable props = JavaUtils.getProperties(ref);
> > > -                listener.unregister(component, props);
> > > -            }
> > > -        }
> > > -        destroyComponent(component, code);
> > > -        AriesFrameworkUtil.safeUnregisterService(registration);
> > > -    }
> > > -
> > >      public Map<ServiceRegistration, Object> getServiceMap() {
> > > -        return Collections.unmodifiableMap(services);
> > > -    }
> > > -
> > > -    public void setBlueprintContainer(ExtendedBlueprintContainer
> > > blueprintContainer) {
> > > -        this.blueprintContainer = blueprintContainer;
> > > -    }
> > > -
> > > -    public void setConfigAdmin(ConfigurationAdmin configAdmin) {
> > > -        this.configAdmin = configAdmin;
> > > +        return Collections.unmodifiableMap(getServices());
> > >      }
> > >
> > >      public void setListeners(List<ServiceListener> listeners) {
> > >          this.listeners = listeners;
> > >      }
> > > -
> > > +
> > >      public void setId(String id) {
> > >          this.id = id;
> > >      }
> > > -
> > > +
> > >      public void setFactoryPid(String factoryPid) {
> > >          this.factoryPid = factoryPid;
> > >      }
> > > @@ -160,7 +118,7 @@ public class CmManagedServiceFactory {
> > >      public void setServiceProperties(Map serviceProperties) {
> > >          this.serviceProperties = serviceProperties;
> > >      }
> > > -
> > > +
> > >      public void setManagedComponentName(String managedComponentName) {
> > >          this.managedComponentName = managedComponentName;
> > >      }
> > > @@ -168,82 +126,42 @@ public class CmManagedServiceFactory {
> > >      public void setComponentDestroyMethod(String
> > componentDestroyMethod) {
> > >          this.componentDestroyMethod = componentDestroyMethod;
> > >      }
> > > -
> > > -    protected void updated(String pid, Dictionary props) {
> > > -      LOGGER.debug("Updated configuration {} with props {}", pid,
> > props);
> > > -
> > > -      Hashtable regProps = null;
> > > -      Object component = null;
> > > -
> > > -      // This method might be multithreaded, so synchronize checking
> and
> > > -      // creating the service
> > > -      final ServiceRegistration existingReg;
> > > -      synchronized (pids) {
> > > -         existingReg = pids.get(pid);
> > > -         if (existingReg == null) {
> > > -            updateComponentProperties(props);
> > > -
> > > -            component =
> > > blueprintContainer.getComponentInstance(managedComponentName);
> > > -
> > > -            // TODO: call listeners, etc...
> > > -
> > > -            regProps = getRegistrationProperties(pid);
> > > -            CmProperties cm = findServiceProcessor();
> > > -            if (cm != null) {
> > > -               if ("".equals(cm.getPersistentId())) {
> > > -                  JavaUtils.copy(regProps, props);
> > > -               }
> > > -               cm.updateProperties(new PropertiesUpdater(pid),
> > regProps);
> > > -            }
> > >
> > > -            Set<String> classes = getClasses(component);
> > > -            String[] classArray = classes.toArray(new
> > > String[classes.size()]);
> > > -            ServiceRegistration reg =
> > > blueprintContainer.getBundleContext().registerService(classArray,
> > > component, regProps);
> > > -
> > > -            LOGGER.debug("Service {} registered with interfaces {} and
> > > properties {}", new Object[] { component, classes, regProps });
> > > -
> > > -            services.put(reg, component);
> > > -            pids.put(pid, reg);
> > > -         }
> > > -        } // end of synchronization
> > > -
> > > -        // If we just registered a service, do the slower stuff
> outside
> > > the synchronized block
> > > -        if (existingReg == null)
> > > -        {
> > > -            if (listeners != null) {
> > > -                for (ServiceListener listener : listeners) {
> > > -                    listener.register(component, regProps);
> > > +    private void getRegistrationProperties(Dictionary properties,
> > boolean
> > > update) {
> > > +        CmProperties cm = findServiceProcessor();
> > > +        if (cm == null) {
> > > +            while (!properties.isEmpty()) {
> > > +                properties.remove(properties.keys().nextElement());
> > > +            }
> > > +        } else  {
> > > +            if (!cm.getUpdate()) {
> > > +                if (update) {
> > > +                    while (!properties.isEmpty()) {
> > > +
> > >  properties.remove(properties.keys().nextElement());
> > > +                    }
> > > +                    for (Map.Entry entry :
> > cm.getProperties().entrySet())
> > > {
> > > +                        properties.put(entry.getKey(),
> > entry.getValue());
> > > +                    }
> > > +                } else {
> > > +                    cm.updated(properties);
> > >                  }
> > >              }
> > > -        } else {
> > > -            updateComponentProperties(props);
> > > -
> > > -            CmProperties cm = findServiceProcessor();
> > > -            if (cm != null && "".equals(cm.getPersistentId())) {
> > > -                regProps = getRegistrationProperties(pid);
> > > -                JavaUtils.copy(regProps, props);
> > > -                cm.updated(regProps);
> > > -            }
> > >          }
> > > -    }
> > > -
> > > -    private Hashtable getRegistrationProperties(String pid) {
> > > -        Hashtable regProps = new Hashtable();
> > >          if (serviceProperties != null) {
> > > -            regProps.putAll(serviceProperties);
> > > +            for (Map.Entry entry : serviceProperties.entrySet()) {
> > > +                properties.put(entry.getKey(), entry.getValue());
> > > +            }
> > >          }
> > > -        regProps.put(Constants.SERVICE_PID, pid);
> > > -        regProps.put(Constants.SERVICE_RANKING, ranking);
> > > -        return regProps;
> > > +        properties.put(Constants.SERVICE_RANKING, ranking);
> > >      }
> > > -
> > > +
> > >      private void updateComponentProperties(Dictionary props) {
> > >          CmManagedProperties cm = findBeanProcessor();
> > >          if (cm != null) {
> > >              cm.updated(props);
> > >          }
> > >      }
> > > -
> > > +
> > >      private CmManagedProperties findBeanProcessor() {
> > >          for (BeanProcessor beanProcessor :
> > > blueprintContainer.getProcessors(BeanProcessor.class)) {
> > >              if (beanProcessor instanceof CmManagedProperties) {
> > > @@ -255,7 +173,7 @@ public class CmManagedServiceFactory {
> > >          }
> > >          return null;
> > >      }
> > > -
> > > +
> > >      private CmProperties findServiceProcessor() {
> > >          for (ServiceProcessor processor :
> > > blueprintContainer.getProcessors(ServiceProcessor.class)) {
> > >              if (processor instanceof CmProperties) {
> > > @@ -267,20 +185,9 @@ public class CmManagedServiceFactory {
> > >          }
> > >          return null;
> > >      }
> > > -
> > > -    private void destroyComponent(Object instance, int reason) {
> > > -        Method method = findDestroyMethod(instance.getClass());
> > > -        if (method != null) {
> > > -            try {
> > > -                method.invoke(instance, new Object [] { reason });
> > > -            } catch (Exception e) {
> > > -                e.printStackTrace();
> > > -            }
> > > -        }
> > > -    }
> > > -
> > > +
> > >      private Method findDestroyMethod(Class clazz) {
> > > -        Method method = null;
> > > +        Method method = null;
> > >          if (componentDestroyMethod != null &&
> > > componentDestroyMethod.length() > 0) {
> > >              List<Method> methods =
> > > ReflectionUtils.findCompatibleMethods(clazz, componentDestroyMethod,
> new
> > > Class [] { int.class });
> > >              if (methods != null && !methods.isEmpty()) {
> > > @@ -289,17 +196,52 @@ public class CmManagedServiceFactory {
> > >          }
> > >          return method;
> > >      }
> > > -
> > > -    protected void deleted(String pid) {
> > > -        LOGGER.debug("Deleted configuration {}", pid);
> > > -        ServiceRegistration reg = pids.remove(pid);
> > > -        if (reg != null) {
> > > -            Object component = services.remove(reg);
> > > -            destroy(component, reg,
> CONFIGURATION_ADMIN_OBJECT_DELETED);
> > > +
> > > +    protected Object doCreate(Dictionary properties) throws Exception
> {
> > > +        updateComponentProperties(copy(properties));
> > > +        Object component =
> > > blueprintContainer.getComponentInstance(managedComponentName);
> > > +        getRegistrationProperties(properties, false);
> > > +        return component;
> > > +    }
> > > +
> > > +    protected Object doUpdate(Object service, Dictionary properties)
> > > throws Exception {
> > > +        updateComponentProperties(copy(properties));
> > > +        getRegistrationProperties(properties, true);
> > > +        return service;
> > > +    }
> > > +
> > > +    protected void doDestroy(Object service, Dictionary properties,
> int
> > > code) throws Exception {
> > > +        Method method = findDestroyMethod(service.getClass());
> > > +        if (method != null) {
> > > +            try {
> > > +                method.invoke(service, new Object [] { code });
> > > +            } catch (Exception e) {
> > > +                LOGGER.info("Error destroying component", e);
> > > +            }
> > > +        }
> > > +    }
> > > +
> > > +    protected void postRegister(Object service, Dictionary properties,
> > > ServiceRegistration registration) {
> > > +        if (listeners != null && !listeners.isEmpty()) {
> > > +            Hashtable props = new Hashtable();
> > > +            JavaUtils.copy(properties, props);
> > > +            for (ServiceListener listener : listeners) {
> > > +                listener.register(service, props);
> > > +            }
> > > +        }
> > > +    }
> > > +
> > > +    protected void preUnregister(Object service, Dictionary
> properties,
> > > ServiceRegistration registration) {
> > > +        if (listeners != null && !listeners.isEmpty()) {
> > > +            Hashtable props = new Hashtable();
> > > +            JavaUtils.copy(properties, props);
> > > +            for (ServiceListener listener : listeners) {
> > > +                listener.unregister(service, props);
> > > +            }
> > >          }
> > >      }
> > >
> > > -    private Set<String> getClasses(Object service) {
> > > +    protected String[] getExposedClasses(Object service) {
> > >          Class serviceClass = service.getClass();
> > >          Set<String> classes;
> > >          switch (autoExport) {
> > > @@ -317,47 +259,13 @@ public class CmManagedServiceFactory {
> > >                  classes = new HashSet<String>(interfaces);
> > >                  break;
> > >          }
> > > -        return classes;
> > > +        return classes.toArray(new String[classes.size()]);
> > >      }
> > > -
> > > -    private class ConfigurationWatcher implements
> ManagedServiceFactory
> > {
> > > -
> > > -        public String getName() {
> > > -            return null;
> > > -        }
> > > -
> > > -        public void updated(String pid, Dictionary props) throws
> > > ConfigurationException {
> > > -            CmManagedServiceFactory.this.updated(pid, props);
> > > -        }
> > >
> > > -        public void deleted(String pid) {
> > > -            CmManagedServiceFactory.this.deleted(pid);
> > > -        }
> > > +    private Hashtable copy(Dictionary source) {
> > > +        Hashtable ht = new Hashtable();
> > > +        JavaUtils.copy(ht, source);
> > > +        return ht;
> > >      }
> > >
> > > -    private class PropertiesUpdater implements
> > > ServiceProcessor.ServicePropertiesUpdater {
> > > -
> > > -        private String pid;
> > > -
> > > -        public PropertiesUpdater(String pid) {
> > > -            this.pid = pid;
> > > -        }
> > > -
> > > -        public String getId() {
> > > -            return id;
> > > -        }
> > > -
> > > -        public void updateProperties(Dictionary properties) {
> > > -            ServiceRegistration reg = pids.get(pid);
> > > -            if (reg != null) {
> > > -                ServiceReference ref = reg.getReference();
> > > -                if (ref != null) {
> > > -                    Hashtable table = JavaUtils.getProperties(ref);
> > > -                    JavaUtils.copy(table, properties);
> > > -                    reg.setProperties(table);
> > > -                }
> > > -            }
> > > -        }
> > > -    }
> > > -
> > >  }
> > >
> > > Modified:
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java?rev=1364914&r1=1364913&r2=1364914&view=diff
> > >
> > >
> >
> ==============================================================================
> > > ---
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java
> > > (original)
> > > +++
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java
> > > Tue Jul 24 06:35:34 2012
> > > @@ -350,8 +350,7 @@ public class CmNamespaceHandler implemen
> > >
>  factoryMetadata.setRuntimeClass(CmManagedServiceFactory.class);
> > >          factoryMetadata.setInitMethod("init");
> > >          factoryMetadata.setDestroyMethod("destroy");
> > > -        factoryMetadata.addProperty("configAdmin",
> > > createConfigurationAdminRef(context));
> > > -        factoryMetadata.addProperty("blueprintContainer",
> > > createRef(context, "blueprintContainer"));
> > > +        factoryMetadata.addArgument(createRef(context,
> > > "blueprintContainer"), null, 0);
> > >          factoryMetadata.addProperty("factoryPid", createValue(context,
> > > element.getAttribute(FACTORY_PID_ATTRIBUTE)));
> > >          String autoExport =
> element.hasAttribute(AUTO_EXPORT_ATTRIBUTE)
> > ?
> > > element.getAttribute(AUTO_EXPORT_ATTRIBUTE) : AUTO_EXPORT_DEFAULT;
> > >          if (AUTO_EXPORT_DISABLED.equals(autoExport)) {
> > >
> > > Modified:
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java?rev=1364914&r1=1364913&r2=1364914&view=diff
> > >
> > >
> >
> ==============================================================================
> > > ---
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java
> > > (original)
> > > +++
> > >
> >
> aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java
> > > Tue Jul 24 06:35:34 2012
> > > @@ -56,6 +56,7 @@ public class ManagedServiceFactoryTest e
> > >          assertNull(sr.getProperty("b"));
> > >
> > >          props = new Hashtable<String,String>();
> > > +        props.put("a", "5");
> > >          props.put("b", "foo");
> > >          cf.update(props);
> > >          Thread.sleep(500);
> > > @@ -89,6 +90,7 @@ public class ManagedServiceFactoryTest e
> > >          assertNull(sr.getProperty("b"));
> > >
> > >          props = new Hashtable<String,String>();
> > > +        props.put("a", "5");
> > >          props.put("b", "foo");
> > >          cf.update(props);
> > >
> > > @@ -122,6 +124,7 @@ public class ManagedServiceFactoryTest e
> > >          assertNull(sr.getProperty("b"));
> > >
> > >          props = new Hashtable<String,String>();
> > > +        props.put("a", "5");
> > >          props.put("b", "foo");
> > >          cf.update(props);
> > >
> > >
> > >
> > >
> >
>
>
>
> --
> ------------------------
> Guillaume Nodet
> ------------------------
> Blog: http://gnodet.blogspot.com/
> ------------------------
> FuseSource, Integration everywhere
> http://fusesource.com
>

Re: svn commit: r1364914 - in /aries/trunk/blueprint/blueprint-cm/src: main/java/org/apache/aries/blueprint/compendium/cm/ test/java/org/apache/aries/blueprint/compendium/cm/

Posted by Guillaume Nodet <gn...@gmail.com>.
Sure, I'll investigate.  They seem to have passed correctly on my machine,
so it may be a timing issue.

On Tue, Jul 24, 2012 at 11:33 AM, Mark Nuttall <mn...@apache.org> wrote:

> Hi Guillaume,
> Perhaps you've noticed all ready, but this change plus the three before it
> is causing
> org.apache.aries.blueprint.itests.TestConfigAdmin.testManagedServiceFactory
> to fail, and so breaking the Hudson build. We saw this first in
> https://builds.apache.org/job/Aries/1592/changes ; it can be seen again in
> https://builds.apache.org/job/Aries/1593/ .
>
> Please would you revert these changes, or fix the resulting test failure?
> Thank you.
>
> Regards,
> Mark
>
> On 24 July 2012 07:35, <gn...@apache.org> wrote:
>
> > Author: gnodet
> > Date: Tue Jul 24 06:35:34 2012
> > New Revision: 1364914
> >
> > URL: http://svn.apache.org/viewvc?rev=1364914&view=rev
> > Log:
> > [ARIES-584] Refactor ManagedServiceFactory to avoid holding lock while
> > registering/unregistering services and have a safer destruction mechanism
> >
> > Added:
> >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java
> > Modified:
> >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java
> >
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java
> >
> >
> aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java
> >
> > Added:
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java
> > URL:
> >
> http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java?rev=1364914&view=auto
> >
> >
> ==============================================================================
> > ---
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java
> > (added)
> > +++
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/BaseManagedServiceFactory.java
> > Tue Jul 24 06:35:34 2012
> > @@ -0,0 +1,233 @@
> > +/**
> > + * 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.aries.blueprint.compendium.cm;
> > +
> > +import java.util.Dictionary;
> > +import java.util.Map;
> > +import java.util.concurrent.ConcurrentHashMap;
> > +import java.util.concurrent.ExecutorService;
> > +import java.util.concurrent.Executors;
> > +import java.util.concurrent.TimeUnit;
> > +import java.util.concurrent.atomic.AtomicBoolean;
> > +
> > +import org.apache.aries.blueprint.utils.JavaUtils;
> > +import org.osgi.framework.BundleContext;
> > +import org.osgi.framework.ServiceRegistration;
> > +import org.osgi.service.cm.ConfigurationException;
> > +import org.osgi.service.cm.ManagedServiceFactory;
> > +import org.slf4j.Logger;
> > +import org.slf4j.LoggerFactory;
> > +
> > +public abstract class BaseManagedServiceFactory<T> implements
> > ManagedServiceFactory {
> > +
> > +    public static final long DEFAULT_TIMEOUT_BEFORE_INTERRUPT = 30000;
> > +
> > +    public static final int CONFIGURATION_ADMIN_OBJECT_DELETED = 1;
> > +
> > +    public static final int BUNDLE_STOPPING = 2;
> > +
> > +    public static final int INTERNAL_ERROR = 4;
> > +
> > +    protected final Logger LOGGER = LoggerFactory.getLogger(getClass());
> > +
> > +    private final BundleContext context;
> > +    private final String name;
> > +    private final long timeoutBeforeInterrupt;
> > +    private final AtomicBoolean destroyed;
> > +    private final ExecutorService executor;
> > +    private final Map<String, Pair<T, ServiceRegistration>> services;
> > +    private final Map<ServiceRegistration, T> registrations;
> > +
> > +    public BaseManagedServiceFactory(BundleContext context, String
> name) {
> > +        this(context, name, DEFAULT_TIMEOUT_BEFORE_INTERRUPT);
> > +    }
> > +
> > +    public BaseManagedServiceFactory(BundleContext context, String name,
> > long timeoutBeforeInterrupt) {
> > +        this.context = context;
> > +        this.name = name;
> > +        this.timeoutBeforeInterrupt = timeoutBeforeInterrupt;
> > +        this.destroyed = new AtomicBoolean(false);
> > +        this.executor = Executors.newSingleThreadExecutor();
> > +        this.services = new ConcurrentHashMap<String, Pair<T,
> > ServiceRegistration>>();
> > +        this.registrations = new ConcurrentHashMap<ServiceRegistration,
> > T>();
> > +    }
> > +
> > +    public String getName() {
> > +        return name;
> > +    }
> > +
> > +    public Map<ServiceRegistration, T> getServices() {
> > +        return registrations;
> > +    }
> > +
> > +    public void updated(final String pid, final Dictionary properties)
> > throws ConfigurationException {
> > +        if (destroyed.get()) {
> > +            return;
> > +        }
> > +        checkConfiguration(pid, properties);
> > +        executor.submit(new Runnable() {
> > +            public void run() {
> > +                try {
> > +                    internalUpdate(pid, properties);
> > +                } catch (Throwable t) {
> > +                    LOGGER.warn("Error destroying service for
> > ManagedServiceFactory " + getName(), t);
> > +                }
> > +            }
> > +        });
> > +    }
> > +
> > +    public void deleted(final String pid) {
> > +        if (destroyed.get()) {
> > +            return;
> > +        }
> > +        executor.submit(new Runnable() {
> > +            public void run() {
> > +                try {
> > +                    internalDelete(pid,
> > CONFIGURATION_ADMIN_OBJECT_DELETED);
> > +                } catch (Throwable throwable) {
> > +                    LOGGER.warn("Error destroying service for
> > ManagedServiceFactory " + getName(), throwable);
> > +                }
> > +            }
> > +        });
> > +    }
> > +
> > +    protected void checkConfiguration(String pid, Dictionary properties)
> > throws ConfigurationException {
> > +        // Do nothing
> > +    }
> > +
> > +    protected abstract T doCreate(Dictionary properties) throws
> Exception;
> > +
> > +    protected abstract T doUpdate(T t, Dictionary properties) throws
> > Exception;
> > +
> > +    protected abstract void doDestroy(T t, Dictionary properties, int
> > code) throws Exception;
> > +
> > +    protected abstract String[] getExposedClasses(T t);
> > +
> > +    private void internalUpdate(String pid, Dictionary properties) {
> > +        Pair<T, ServiceRegistration> pair = services.get(pid);
> > +        if (pair != null) {
> > +            try {
> > +                T t = doUpdate(pair.getFirst(), properties);
> > +                pair.setFirst(t);
> > +                pair.getSecond().setProperties(properties);
> > +            } catch (Throwable throwable) {
> > +                internalDelete(pid, INTERNAL_ERROR);
> > +                LOGGER.warn("Error updating service for
> > ManagedServiceFactory " + getName(), throwable);
> > +            }
> > +        } else {
> > +            if (destroyed.get()) {
> > +                return;
> > +            }
> > +            try {
> > +                T t = doCreate(properties);
> > +                try {
> > +                    if (destroyed.get()) {
> > +                        throw new
> > IllegalStateException("ManagedServiceFactory has been destroyed");
> > +                    }
> > +                    ServiceRegistration registration =
> > context.registerService(getExposedClasses(t), t, properties);
> > +                    services.put(pid, new Pair<T,
> ServiceRegistration>(t,
> > registration));
> > +                    registrations.put(registration,  t);
> > +                    postRegister(t, properties, registration);
> > +                } catch (Throwable throwable1) {
> > +                    try {
> > +                        doDestroy(t, properties, INTERNAL_ERROR);
> > +                    } catch (Throwable throwable2) {
> > +                        // Ignore
> > +                    }
> > +                    throw throwable1;
> > +                }
> > +            } catch (Throwable throwable) {
> > +                LOGGER.warn("Error creating service for
> > ManagedServiceFactory " + getName(), throwable);
> > +            }
> > +        }
> > +    }
> > +
> > +    protected void postRegister(T t, Dictionary properties,
> > ServiceRegistration registration) {
> > +        // Place holder
> > +    }
> > +
> > +    protected void preUnregister(T t, Dictionary properties,
> > ServiceRegistration registration) {
> > +        // Place holder
> > +    }
> > +
> > +    private void internalDelete(String pid, int code) {
> > +        Pair<T, ServiceRegistration> pair = services.remove(pid);
> > +        if (pair != null) {
> > +            registrations.remove(pair.getSecond());
> > +            Dictionary properties =
> > JavaUtils.getProperties(pair.getSecond().getReference());
> > +            try {
> > +                preUnregister(pair.getFirst(), properties,
> > pair.getSecond());
> > +                pair.getSecond().unregister();
> > +            } catch (Throwable t) {
> > +                LOGGER.info("Error unregistering service", t);
> > +            }
> > +            try {
> > +                doDestroy(pair.getFirst(), properties, code);
> > +            } catch (Throwable t) {
> > +                LOGGER.info("Error destroying service", t);
> > +            }
> > +        }
> > +    }
> > +
> > +    public void destroy() {
> > +        if (destroyed.compareAndSet(false, true)) {
> > +            executor.shutdown();
> > +            try {
> > +                executor.awaitTermination(timeoutBeforeInterrupt,
> > TimeUnit.MILLISECONDS);
> > +            } catch (InterruptedException e) {
> > +                throw new RuntimeException("Shutdown interrupted");
> > +            }
> > +            if (!executor.isTerminated()) {
> > +                executor.shutdownNow();
> > +                try {
> > +                    executor.awaitTermination(Long.MAX_VALUE,
> > TimeUnit.NANOSECONDS);
> > +                } catch (InterruptedException e) {
> > +                    throw new RuntimeException("Shutdown interrupted");
> > +                }
> > +            }
> > +
> > +            while (!services.isEmpty()) {
> > +                String pid = services.keySet().iterator().next();
> > +                internalDelete(pid, BUNDLE_STOPPING);
> > +            }
> > +        }
> > +    }
> > +
> > +    static class Pair<U,V> {
> > +        private U first;
> > +        private V second;
> > +        public Pair(U first, V second) {
> > +            this.first = first;
> > +            this.second = second;
> > +        }
> > +        public U getFirst() {
> > +            return first;
> > +        }
> > +        public V getSecond() {
> > +            return second;
> > +        }
> > +        public void setFirst(U first) {
> > +            this.first = first;
> > +        }
> > +        public void setSecond(V second) {
> > +            this.second = second;
> > +        }
> > +    }
> > +
> > +}
> >
> > Modified:
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java
> > URL:
> >
> http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java?rev=1364914&r1=1364913&r2=1364914&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java
> > (original)
> > +++
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmManagedServiceFactory.java
> > Tue Jul 24 06:35:34 2012
> > @@ -27,23 +27,18 @@ import java.util.List;
> >  import java.util.Map;
> >  import java.util.Properties;
> >  import java.util.Set;
> > -import java.util.concurrent.ConcurrentHashMap;
> >
> >  import org.apache.aries.blueprint.BeanProcessor;
> > -import org.apache.aries.blueprint.services.ExtendedBlueprintContainer;
> >  import org.apache.aries.blueprint.ServiceProcessor;
> > +import org.apache.aries.blueprint.services.ExtendedBlueprintContainer;
> >  import org.apache.aries.blueprint.utils.JavaUtils;
> >  import org.apache.aries.blueprint.utils.ReflectionUtils;
> >  import org.apache.aries.blueprint.utils.ServiceListener;
> >  import org.apache.aries.util.AriesFrameworkUtil;
> >  import org.osgi.framework.Bundle;
> >  import org.osgi.framework.Constants;
> > -import org.osgi.framework.ServiceReference;
> >  import org.osgi.framework.ServiceRegistration;
> >  import org.osgi.service.blueprint.reflect.ServiceMetadata;
> > -import org.osgi.service.cm.Configuration;
> > -import org.osgi.service.cm.ConfigurationAdmin;
> > -import org.osgi.service.cm.ConfigurationException;
> >  import org.osgi.service.cm.ManagedServiceFactory;
> >  import org.slf4j.Logger;
> >  import org.slf4j.LoggerFactory;
> > @@ -54,30 +49,27 @@ import org.slf4j.LoggerFactory;
> >   *
> >   * @version $Rev$, $Date$
> >   */
> > -public class CmManagedServiceFactory {
> > -
> > -    static final int CONFIGURATION_ADMIN_OBJECT_DELETED = 1;
> > -
> > -    static final int BUNDLE_STOPPING = 2;
> > +public class CmManagedServiceFactory extends
> > BaseManagedServiceFactory<Object> {
> >
> >      private static final Logger LOGGER =
> > LoggerFactory.getLogger(CmManagedServiceFactory.class);
> > -
> > +
> >      private ExtendedBlueprintContainer blueprintContainer;
> > -    private ConfigurationAdmin configAdmin;
> >      private String id;
> >      private String factoryPid;
> >      private List<String> interfaces;
> >      private int autoExport;
> >      private int ranking;
> > -    private Map serviceProperties;
> > +    private Map<Object,Object> serviceProperties;
> >      private String managedComponentName;
> >      private String componentDestroyMethod;
> >      private List<ServiceListener> listeners;
> > -    private final Object lock = new Object();
> >
> >      private ServiceRegistration registration;
> > -    private final Map<String, ServiceRegistration> pids = new
> > ConcurrentHashMap<String, ServiceRegistration>();
> > -    private final Map<ServiceRegistration, Object> services = new
> > ConcurrentHashMap<ServiceRegistration, Object>();
> > +
> > +    public CmManagedServiceFactory(ExtendedBlueprintContainer
> > blueprintContainer) {
> > +        super(blueprintContainer.getBundleContext(), null);
> > +        this.blueprintContainer = blueprintContainer;
> > +    }
> >
> >      public void init() throws Exception {
> >          LOGGER.debug("Initializing CmManagedServiceFactory for
> > factoryPid={}", factoryPid);
> > @@ -86,61 +78,27 @@ public class CmManagedServiceFactory {
> >          Bundle bundle =
> blueprintContainer.getBundleContext().getBundle();
> >          props.put(Constants.BUNDLE_SYMBOLICNAME,
> > bundle.getSymbolicName());
> >          props.put(Constants.BUNDLE_VERSION,
> > bundle.getHeaders().get(Constants.BUNDLE_VERSION));
> > -
> > -        synchronized(lock) {
> > -            registration =
> >
> blueprintContainer.getBundleContext().registerService(ManagedServiceFactory.class.getName(),
> > new ConfigurationWatcher(), props);
> > -
> > -            String filter = '(' + ConfigurationAdmin.SERVICE_FACTORYPID
> +
> > '=' + this.factoryPid + ')';
> > -            Configuration[] configs =
> > configAdmin.listConfigurations(filter);
> > -            if (configs != null) {
> > -                for (Configuration config : configs) {
> > -                    updated(config.getPid(), config.getProperties());
> > -                }
> > -            }
> > -        }
> > +
> > +        registration =
> >
> blueprintContainer.getBundleContext().registerService(ManagedServiceFactory.class.getName(),
> > this, props);
> >      }
> >
> >      public void destroy() {
> >          AriesFrameworkUtil.safeUnregisterService(registration);
> > -        for (Map.Entry<ServiceRegistration, Object> entry :
> > services.entrySet()) {
> > -            destroy(entry.getValue(), entry.getKey(), BUNDLE_STOPPING);
> > -        }
> > -        services.clear();
> > -        pids.clear();
> > +        super.destroy();
> >      }
> >
> > -    private void destroy(Object component, ServiceRegistration
> > registration, int code) {
> > -        if (listeners != null) {
> > -            ServiceReference ref = registration.getReference();
> > -            for (ServiceListener listener : listeners) {
> > -                Hashtable props = JavaUtils.getProperties(ref);
> > -                listener.unregister(component, props);
> > -            }
> > -        }
> > -        destroyComponent(component, code);
> > -        AriesFrameworkUtil.safeUnregisterService(registration);
> > -    }
> > -
> >      public Map<ServiceRegistration, Object> getServiceMap() {
> > -        return Collections.unmodifiableMap(services);
> > -    }
> > -
> > -    public void setBlueprintContainer(ExtendedBlueprintContainer
> > blueprintContainer) {
> > -        this.blueprintContainer = blueprintContainer;
> > -    }
> > -
> > -    public void setConfigAdmin(ConfigurationAdmin configAdmin) {
> > -        this.configAdmin = configAdmin;
> > +        return Collections.unmodifiableMap(getServices());
> >      }
> >
> >      public void setListeners(List<ServiceListener> listeners) {
> >          this.listeners = listeners;
> >      }
> > -
> > +
> >      public void setId(String id) {
> >          this.id = id;
> >      }
> > -
> > +
> >      public void setFactoryPid(String factoryPid) {
> >          this.factoryPid = factoryPid;
> >      }
> > @@ -160,7 +118,7 @@ public class CmManagedServiceFactory {
> >      public void setServiceProperties(Map serviceProperties) {
> >          this.serviceProperties = serviceProperties;
> >      }
> > -
> > +
> >      public void setManagedComponentName(String managedComponentName) {
> >          this.managedComponentName = managedComponentName;
> >      }
> > @@ -168,82 +126,42 @@ public class CmManagedServiceFactory {
> >      public void setComponentDestroyMethod(String
> componentDestroyMethod) {
> >          this.componentDestroyMethod = componentDestroyMethod;
> >      }
> > -
> > -    protected void updated(String pid, Dictionary props) {
> > -      LOGGER.debug("Updated configuration {} with props {}", pid,
> props);
> > -
> > -      Hashtable regProps = null;
> > -      Object component = null;
> > -
> > -      // This method might be multithreaded, so synchronize checking and
> > -      // creating the service
> > -      final ServiceRegistration existingReg;
> > -      synchronized (pids) {
> > -         existingReg = pids.get(pid);
> > -         if (existingReg == null) {
> > -            updateComponentProperties(props);
> > -
> > -            component =
> > blueprintContainer.getComponentInstance(managedComponentName);
> > -
> > -            // TODO: call listeners, etc...
> > -
> > -            regProps = getRegistrationProperties(pid);
> > -            CmProperties cm = findServiceProcessor();
> > -            if (cm != null) {
> > -               if ("".equals(cm.getPersistentId())) {
> > -                  JavaUtils.copy(regProps, props);
> > -               }
> > -               cm.updateProperties(new PropertiesUpdater(pid),
> regProps);
> > -            }
> >
> > -            Set<String> classes = getClasses(component);
> > -            String[] classArray = classes.toArray(new
> > String[classes.size()]);
> > -            ServiceRegistration reg =
> > blueprintContainer.getBundleContext().registerService(classArray,
> > component, regProps);
> > -
> > -            LOGGER.debug("Service {} registered with interfaces {} and
> > properties {}", new Object[] { component, classes, regProps });
> > -
> > -            services.put(reg, component);
> > -            pids.put(pid, reg);
> > -         }
> > -        } // end of synchronization
> > -
> > -        // If we just registered a service, do the slower stuff outside
> > the synchronized block
> > -        if (existingReg == null)
> > -        {
> > -            if (listeners != null) {
> > -                for (ServiceListener listener : listeners) {
> > -                    listener.register(component, regProps);
> > +    private void getRegistrationProperties(Dictionary properties,
> boolean
> > update) {
> > +        CmProperties cm = findServiceProcessor();
> > +        if (cm == null) {
> > +            while (!properties.isEmpty()) {
> > +                properties.remove(properties.keys().nextElement());
> > +            }
> > +        } else  {
> > +            if (!cm.getUpdate()) {
> > +                if (update) {
> > +                    while (!properties.isEmpty()) {
> > +
> >  properties.remove(properties.keys().nextElement());
> > +                    }
> > +                    for (Map.Entry entry :
> cm.getProperties().entrySet())
> > {
> > +                        properties.put(entry.getKey(),
> entry.getValue());
> > +                    }
> > +                } else {
> > +                    cm.updated(properties);
> >                  }
> >              }
> > -        } else {
> > -            updateComponentProperties(props);
> > -
> > -            CmProperties cm = findServiceProcessor();
> > -            if (cm != null && "".equals(cm.getPersistentId())) {
> > -                regProps = getRegistrationProperties(pid);
> > -                JavaUtils.copy(regProps, props);
> > -                cm.updated(regProps);
> > -            }
> >          }
> > -    }
> > -
> > -    private Hashtable getRegistrationProperties(String pid) {
> > -        Hashtable regProps = new Hashtable();
> >          if (serviceProperties != null) {
> > -            regProps.putAll(serviceProperties);
> > +            for (Map.Entry entry : serviceProperties.entrySet()) {
> > +                properties.put(entry.getKey(), entry.getValue());
> > +            }
> >          }
> > -        regProps.put(Constants.SERVICE_PID, pid);
> > -        regProps.put(Constants.SERVICE_RANKING, ranking);
> > -        return regProps;
> > +        properties.put(Constants.SERVICE_RANKING, ranking);
> >      }
> > -
> > +
> >      private void updateComponentProperties(Dictionary props) {
> >          CmManagedProperties cm = findBeanProcessor();
> >          if (cm != null) {
> >              cm.updated(props);
> >          }
> >      }
> > -
> > +
> >      private CmManagedProperties findBeanProcessor() {
> >          for (BeanProcessor beanProcessor :
> > blueprintContainer.getProcessors(BeanProcessor.class)) {
> >              if (beanProcessor instanceof CmManagedProperties) {
> > @@ -255,7 +173,7 @@ public class CmManagedServiceFactory {
> >          }
> >          return null;
> >      }
> > -
> > +
> >      private CmProperties findServiceProcessor() {
> >          for (ServiceProcessor processor :
> > blueprintContainer.getProcessors(ServiceProcessor.class)) {
> >              if (processor instanceof CmProperties) {
> > @@ -267,20 +185,9 @@ public class CmManagedServiceFactory {
> >          }
> >          return null;
> >      }
> > -
> > -    private void destroyComponent(Object instance, int reason) {
> > -        Method method = findDestroyMethod(instance.getClass());
> > -        if (method != null) {
> > -            try {
> > -                method.invoke(instance, new Object [] { reason });
> > -            } catch (Exception e) {
> > -                e.printStackTrace();
> > -            }
> > -        }
> > -    }
> > -
> > +
> >      private Method findDestroyMethod(Class clazz) {
> > -        Method method = null;
> > +        Method method = null;
> >          if (componentDestroyMethod != null &&
> > componentDestroyMethod.length() > 0) {
> >              List<Method> methods =
> > ReflectionUtils.findCompatibleMethods(clazz, componentDestroyMethod, new
> > Class [] { int.class });
> >              if (methods != null && !methods.isEmpty()) {
> > @@ -289,17 +196,52 @@ public class CmManagedServiceFactory {
> >          }
> >          return method;
> >      }
> > -
> > -    protected void deleted(String pid) {
> > -        LOGGER.debug("Deleted configuration {}", pid);
> > -        ServiceRegistration reg = pids.remove(pid);
> > -        if (reg != null) {
> > -            Object component = services.remove(reg);
> > -            destroy(component, reg, CONFIGURATION_ADMIN_OBJECT_DELETED);
> > +
> > +    protected Object doCreate(Dictionary properties) throws Exception {
> > +        updateComponentProperties(copy(properties));
> > +        Object component =
> > blueprintContainer.getComponentInstance(managedComponentName);
> > +        getRegistrationProperties(properties, false);
> > +        return component;
> > +    }
> > +
> > +    protected Object doUpdate(Object service, Dictionary properties)
> > throws Exception {
> > +        updateComponentProperties(copy(properties));
> > +        getRegistrationProperties(properties, true);
> > +        return service;
> > +    }
> > +
> > +    protected void doDestroy(Object service, Dictionary properties, int
> > code) throws Exception {
> > +        Method method = findDestroyMethod(service.getClass());
> > +        if (method != null) {
> > +            try {
> > +                method.invoke(service, new Object [] { code });
> > +            } catch (Exception e) {
> > +                LOGGER.info("Error destroying component", e);
> > +            }
> > +        }
> > +    }
> > +
> > +    protected void postRegister(Object service, Dictionary properties,
> > ServiceRegistration registration) {
> > +        if (listeners != null && !listeners.isEmpty()) {
> > +            Hashtable props = new Hashtable();
> > +            JavaUtils.copy(properties, props);
> > +            for (ServiceListener listener : listeners) {
> > +                listener.register(service, props);
> > +            }
> > +        }
> > +    }
> > +
> > +    protected void preUnregister(Object service, Dictionary properties,
> > ServiceRegistration registration) {
> > +        if (listeners != null && !listeners.isEmpty()) {
> > +            Hashtable props = new Hashtable();
> > +            JavaUtils.copy(properties, props);
> > +            for (ServiceListener listener : listeners) {
> > +                listener.unregister(service, props);
> > +            }
> >          }
> >      }
> >
> > -    private Set<String> getClasses(Object service) {
> > +    protected String[] getExposedClasses(Object service) {
> >          Class serviceClass = service.getClass();
> >          Set<String> classes;
> >          switch (autoExport) {
> > @@ -317,47 +259,13 @@ public class CmManagedServiceFactory {
> >                  classes = new HashSet<String>(interfaces);
> >                  break;
> >          }
> > -        return classes;
> > +        return classes.toArray(new String[classes.size()]);
> >      }
> > -
> > -    private class ConfigurationWatcher implements ManagedServiceFactory
> {
> > -
> > -        public String getName() {
> > -            return null;
> > -        }
> > -
> > -        public void updated(String pid, Dictionary props) throws
> > ConfigurationException {
> > -            CmManagedServiceFactory.this.updated(pid, props);
> > -        }
> >
> > -        public void deleted(String pid) {
> > -            CmManagedServiceFactory.this.deleted(pid);
> > -        }
> > +    private Hashtable copy(Dictionary source) {
> > +        Hashtable ht = new Hashtable();
> > +        JavaUtils.copy(ht, source);
> > +        return ht;
> >      }
> >
> > -    private class PropertiesUpdater implements
> > ServiceProcessor.ServicePropertiesUpdater {
> > -
> > -        private String pid;
> > -
> > -        public PropertiesUpdater(String pid) {
> > -            this.pid = pid;
> > -        }
> > -
> > -        public String getId() {
> > -            return id;
> > -        }
> > -
> > -        public void updateProperties(Dictionary properties) {
> > -            ServiceRegistration reg = pids.get(pid);
> > -            if (reg != null) {
> > -                ServiceReference ref = reg.getReference();
> > -                if (ref != null) {
> > -                    Hashtable table = JavaUtils.getProperties(ref);
> > -                    JavaUtils.copy(table, properties);
> > -                    reg.setProperties(table);
> > -                }
> > -            }
> > -        }
> > -    }
> > -
> >  }
> >
> > Modified:
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java
> > URL:
> >
> http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java?rev=1364914&r1=1364913&r2=1364914&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java
> > (original)
> > +++
> >
> aries/trunk/blueprint/blueprint-cm/src/main/java/org/apache/aries/blueprint/compendium/cm/CmNamespaceHandler.java
> > Tue Jul 24 06:35:34 2012
> > @@ -350,8 +350,7 @@ public class CmNamespaceHandler implemen
> >          factoryMetadata.setRuntimeClass(CmManagedServiceFactory.class);
> >          factoryMetadata.setInitMethod("init");
> >          factoryMetadata.setDestroyMethod("destroy");
> > -        factoryMetadata.addProperty("configAdmin",
> > createConfigurationAdminRef(context));
> > -        factoryMetadata.addProperty("blueprintContainer",
> > createRef(context, "blueprintContainer"));
> > +        factoryMetadata.addArgument(createRef(context,
> > "blueprintContainer"), null, 0);
> >          factoryMetadata.addProperty("factoryPid", createValue(context,
> > element.getAttribute(FACTORY_PID_ATTRIBUTE)));
> >          String autoExport = element.hasAttribute(AUTO_EXPORT_ATTRIBUTE)
> ?
> > element.getAttribute(AUTO_EXPORT_ATTRIBUTE) : AUTO_EXPORT_DEFAULT;
> >          if (AUTO_EXPORT_DISABLED.equals(autoExport)) {
> >
> > Modified:
> >
> aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java
> > URL:
> >
> http://svn.apache.org/viewvc/aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java?rev=1364914&r1=1364913&r2=1364914&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java
> > (original)
> > +++
> >
> aries/trunk/blueprint/blueprint-cm/src/test/java/org/apache/aries/blueprint/compendium/cm/ManagedServiceFactoryTest.java
> > Tue Jul 24 06:35:34 2012
> > @@ -56,6 +56,7 @@ public class ManagedServiceFactoryTest e
> >          assertNull(sr.getProperty("b"));
> >
> >          props = new Hashtable<String,String>();
> > +        props.put("a", "5");
> >          props.put("b", "foo");
> >          cf.update(props);
> >          Thread.sleep(500);
> > @@ -89,6 +90,7 @@ public class ManagedServiceFactoryTest e
> >          assertNull(sr.getProperty("b"));
> >
> >          props = new Hashtable<String,String>();
> > +        props.put("a", "5");
> >          props.put("b", "foo");
> >          cf.update(props);
> >
> > @@ -122,6 +124,7 @@ public class ManagedServiceFactoryTest e
> >          assertNull(sr.getProperty("b"));
> >
> >          props = new Hashtable<String,String>();
> > +        props.put("a", "5");
> >          props.put("b", "foo");
> >          cf.update(props);
> >
> >
> >
> >
>



-- 
------------------------
Guillaume Nodet
------------------------
Blog: http://gnodet.blogspot.com/
------------------------
FuseSource, Integration everywhere
http://fusesource.com