Aries
  1. Aries
  2. ARIES-584

Blueprint Managed Service Factory Instantiates Duplicate Service

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.2, 0.3
    • Fix Version/s: 1.0
    • Component/s: Blueprint
    • Labels:
      None
    • Environment:

      Mac OSX 10.6.6 / Equinox 3.6.1 / Felix Config Admin 1.2.8 / Aries Blueprint 0.3 (and 0.2)

      Description

      Creating a simple managed service factory, two services are instantiated for a single factory configuration.

      1. logs.txt
        23 kB
        Greg Rapp
      2. org.apache.aries.blueprint.cm.patch-withtrunk.patch
        3 kB
        Marcel Hanser
      3. org.apache.aries.blueprint.cm-0.3.1.patch
        3 kB
        Marcel Hanser

        Activity

        Hide
        Greg Rapp added a comment -

        Blueprint XML definition -

        <blueprint xmlns="http://www.osgi.org/xmlns/blueprint/v1.0.0"
        xmlns:cm="http://aries.apache.org/blueprint/xmlns/blueprint-cm/v1.0.0">

        <cm:managed-service-factory id="testfactory"
        factory-pid="testfactory" interface="testplugin.ServerImpl"
        auto-export="all-classes">
        <cm:managed-component class="testplugin.ServerImpl" />
        </cm:managed-service-factory>
        </blueprint>

        Show
        Greg Rapp added a comment - Blueprint XML definition - <blueprint xmlns="http://www.osgi.org/xmlns/blueprint/v1.0.0" xmlns:cm="http://aries.apache.org/blueprint/xmlns/blueprint-cm/v1.0.0"> <cm:managed-service-factory id="testfactory" factory-pid="testfactory" interface="testplugin.ServerImpl" auto-export="all-classes"> <cm:managed-component class="testplugin.ServerImpl" /> </cm:managed-service-factory> </blueprint>
        Hide
        Greg Rapp added a comment -

        Service code -

        package testplugin;

        import java.util.Map;

        import org.slf4j.Logger;
        import org.slf4j.LoggerFactory;

        public class ServerImpl {

        Logger log = LoggerFactory.getLogger(ServerImpl.class);

        public ServerImpl()

        { log.error("************ Server Created!"); }

        public void update (Map<String,?> props) {
        log.error("************ Configuration updated - {}.",props.toString());
        }
        }

        Show
        Greg Rapp added a comment - Service code - package testplugin; import java.util.Map; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class ServerImpl { Logger log = LoggerFactory.getLogger(ServerImpl.class); public ServerImpl() { log.error("************ Server Created!"); } public void update (Map<String,?> props) { log.error("************ Configuration updated - {}.",props.toString()); } }
        Hide
        Greg Rapp added a comment -

        Configuration List -

        cm list
        Configuration list:
        testfactory.daf5d806-19c9-4e64-b727-9c48789eaf99 initial@reference:file:../../../workspace-sts/TestPlugin/

        osgi> cm getv testfactory.daf5d806-19c9-4e64-b727-9c48789eaf99

        Configuration for service (pid) "testfactory.daf5d806-19c9-4e64-b727-9c48789eaf99"
        (bundle location = initial@reference:file:../../../workspace-sts/TestPlugin/)

        key value type
        ------ ------ ------
        service.pid testfactory.daf5d806-19c9-4e64-b727-9c48789eaf99 java.lang.String
        service.factoryPid testfactory java.lang.String

        Show
        Greg Rapp added a comment - Configuration List - cm list Configuration list: testfactory.daf5d806-19c9-4e64-b727-9c48789eaf99 initial@reference: file:../../../workspace-sts/TestPlugin/ osgi> cm getv testfactory.daf5d806-19c9-4e64-b727-9c48789eaf99 Configuration for service (pid) "testfactory.daf5d806-19c9-4e64-b727-9c48789eaf99" (bundle location = initial@reference: file:../../../workspace-sts/TestPlugin/ ) key value type ------ ------ ------ service.pid testfactory.daf5d806-19c9-4e64-b727-9c48789eaf99 java.lang.String service.factoryPid testfactory java.lang.String
        Hide
        Greg Rapp added a comment -

        Bundle runtime details showing two services with the same factory PID -

        osgi> bundle 20
        TestPlugin_1.0.0.qualifier [20]
        Id=20, Status=ACTIVE Data Root=/Users/grapp/Documents/workspace-sts/.metadata/.plugins/org.eclipse.pde.core/Test/org.eclipse.osgi/bundles/20/data
        Registered Services

        {org.osgi.service.cm.ManagedServiceFactory}

        =

        {Bundle-SymbolicName=TestPlugin, Bundle-Version=1.0.0.qualifier, service.pid=testfactory, service.id=44}

        {testplugin.ServerImpl}={service.ranking=0, service.pid=testfactory.daf5d806-19c9-4e64-b727-9c48789eaf99, service.id=45}
        {org.osgi.service.blueprint.container.BlueprintContainer}={osgi.blueprint.container.version=1.0.0.qualifier, osgi.blueprint.container.symbolicname=TestPlugin, service.id=46}
        {testplugin.ServerImpl}

        =

        {service.ranking=0, service.pid=testfactory.daf5d806-19c9-4e64-b727-9c48789eaf99, service.id=47}

        No services in use.
        Exported packages
        testplugin; version="0.0.0"[exported]
        testplugin.api; version="0.0.0"[exported]
        Imported packages
        org.slf4j; version="1.6.1"<slf4j.api_1.6.1 [8]>
        No fragment bundles
        Named class space
        TestPlugin; bundle-version="1.0.0.qualifier"[provided]
        No required bundles

        osgi>

        Show
        Greg Rapp added a comment - Bundle runtime details showing two services with the same factory PID - osgi> bundle 20 TestPlugin_1.0.0.qualifier [20] Id=20, Status=ACTIVE Data Root=/Users/grapp/Documents/workspace-sts/.metadata/.plugins/org.eclipse.pde.core/Test/org.eclipse.osgi/bundles/20/data Registered Services {org.osgi.service.cm.ManagedServiceFactory} = {Bundle-SymbolicName=TestPlugin, Bundle-Version=1.0.0.qualifier, service.pid=testfactory, service.id=44} {testplugin.ServerImpl}={service.ranking=0, service.pid=testfactory.daf5d806-19c9-4e64-b727-9c48789eaf99, service.id=45} {org.osgi.service.blueprint.container.BlueprintContainer}={osgi.blueprint.container.version=1.0.0.qualifier, osgi.blueprint.container.symbolicname=TestPlugin, service.id=46} {testplugin.ServerImpl} = {service.ranking=0, service.pid=testfactory.daf5d806-19c9-4e64-b727-9c48789eaf99, service.id=47} No services in use. Exported packages testplugin; version="0.0.0" [exported] testplugin.api; version="0.0.0" [exported] Imported packages org.slf4j; version="1.6.1"<slf4j.api_1.6.1 [8] > No fragment bundles Named class space TestPlugin; bundle-version="1.0.0.qualifier" [provided] No required bundles osgi>
        Hide
        Greg Rapp added a comment - - edited

        Blueprint logs during container startup showing creation of the duplicate service instance

        Show
        Greg Rapp added a comment - - edited Blueprint logs during container startup showing creation of the duplicate service instance
        Hide
        Marcel Hanser added a comment -

        Currently the org.apache.aries.blueprint.compendium.cm.CmManagedServiceFactory lists the CM configurations and calls the updated method manually in the init method. But the method is called a second time by the configuration admin after service registration as specified. See attached patch to avoid the first updated call in the init method. I would also remove the mutex object which is locked during service registration but I am a little bit unsure to remove locks from code i didn't write The patch works fine on our container (Felix 4.0.2, Blueprint 0.3.1, ConfigurationAdmin 1.2.8). I added two patches one for blueprint-cm 0.3.1 and for the current trunk.

        Show
        Marcel Hanser added a comment - Currently the org.apache.aries.blueprint.compendium.cm.CmManagedServiceFactory lists the CM configurations and calls the updated method manually in the init method. But the method is called a second time by the configuration admin after service registration as specified. See attached patch to avoid the first updated call in the init method. I would also remove the mutex object which is locked during service registration but I am a little bit unsure to remove locks from code i didn't write The patch works fine on our container (Felix 4.0.2, Blueprint 0.3.1, ConfigurationAdmin 1.2.8). I added two patches one for blueprint-cm 0.3.1 and for the current trunk.
        Hide
        Hendy Irawan added a comment -

        +1!

        I really hope this gets resolved. Still happens on Aries Blueprint 0.3.2 / Karaf 2.2.8.

        Show
        Hendy Irawan added a comment - +1! I really hope this gets resolved. Still happens on Aries Blueprint 0.3.2 / Karaf 2.2.8.
        Hide
        Holly Cummins added a comment -

        Fixed in the nick of time, before the blueprint.cm release. Thanks for the patch.

        I had some difficulty reproducing this problem in our ConfigAdminTest. However, I could sometimes reproduce it, which makes me conclude there's a concurrency element to the problem. I think this means the patch isn't quite the right fix, although it definitely points to the right area.

        The updated() method on the CmManagedServiceFactory does checks if the pid exists, and only registers a service if the pid doesn't, so calling it twice shouldn't be a problem. Problems do happen when the updated() method is driven by both the init() method and configuration admin concurrently. My suspicion is that removing the updated() call in the init method exposes us to problems - if the ConfigurationWatcher is registered too late, it won't be notified of config changes, and the service won't be registered at all. I've synchronized the updated() method so that we don't check whether the pid has been registered until we're sure any previous registration has completed.

        I've also updated our TestConfigAdmin test to make sure only one service is registered, although this test was passing even for the original code, unless I changed the concurrency by removing other tests.

        I haven't applied a fix to the 0.3.1 branch, since I'm not sure if there will be more releases on that.

        Show
        Holly Cummins added a comment - Fixed in the nick of time, before the blueprint.cm release. Thanks for the patch. I had some difficulty reproducing this problem in our ConfigAdminTest. However, I could sometimes reproduce it, which makes me conclude there's a concurrency element to the problem. I think this means the patch isn't quite the right fix, although it definitely points to the right area. The updated() method on the CmManagedServiceFactory does checks if the pid exists, and only registers a service if the pid doesn't, so calling it twice shouldn't be a problem. Problems do happen when the updated() method is driven by both the init() method and configuration admin concurrently. My suspicion is that removing the updated() call in the init method exposes us to problems - if the ConfigurationWatcher is registered too late, it won't be notified of config changes, and the service won't be registered at all. I've synchronized the updated() method so that we don't check whether the pid has been registered until we're sure any previous registration has completed. I've also updated our TestConfigAdmin test to make sure only one service is registered, although this test was passing even for the original code, unless I changed the concurrency by removing other tests. I haven't applied a fix to the 0.3.1 branch, since I'm not sure if there will be more releases on that.
        Hide
        Hendy Irawan added a comment -

        Thank you Holly !

        BTW... is there any plans to support managed service factory properties without implementing 'updated()' ? i.e. using the same behavior as managed service (bean initially created using managed properties, if the managed properties change then the bean gets reloaded).

        Show
        Hendy Irawan added a comment - Thank you Holly ! BTW... is there any plans to support managed service factory properties without implementing 'updated()' ? i.e. using the same behavior as managed service (bean initially created using managed properties, if the managed properties change then the bean gets reloaded).
        Hide
        Guillaume Nodet added a comment -

        I'd like to get that fixed in 0.3.x in case we need a new release (which might happen).
        Also, I've learned that calling OSGi API while holding java locks is bound to lead to deadlocks, so I think we should find a better solution for that.

        Show
        Guillaume Nodet added a comment - I'd like to get that fixed in 0.3.x in case we need a new release (which might happen). Also, I've learned that calling OSGi API while holding java locks is bound to lead to deadlocks, so I think we should find a better solution for that.
        Hide
        Marcel Hanser added a comment -

        As I understand the OSGi Configuration Admin spec garantees that we are notified if there is already an existing configuration for a ManagedServiceFactory as the ConfigurationWatcher is one, so there is no need to additionally check it in the init() method? (See 104.6.2 OSGi compendium specification V 4.3 or 4.2).

        "When the Configuration Admin service detects the registration of a Managed Service Factory, it must find all configuration dictionaries for this factory and must then sequentially call ManagedServiceFactory.updated(String,Dictionary) for each configuration dictionary. The first argument is the PID of the Configuration object (the one created by the Configuration Admin service) and the second argument contains the configuration properties."

        This would also reduce the critical area for a deadlock. If you call the updated method on your own during the init() method the chance to get caught into the deadlock described here https://issues.apache.org/jira/browse/ARIES-867 increases. Clear: the deadlock situation is not totally mitigated due to the service registration in the init() method, while holding the BlueprintContainer lock...

        Show
        Marcel Hanser added a comment - As I understand the OSGi Configuration Admin spec garantees that we are notified if there is already an existing configuration for a ManagedServiceFactory as the ConfigurationWatcher is one, so there is no need to additionally check it in the init() method? (See 104.6.2 OSGi compendium specification V 4.3 or 4.2). "When the Configuration Admin service detects the registration of a Managed Service Factory, it must find all configuration dictionaries for this factory and must then sequentially call ManagedServiceFactory.updated( String ,Dictionary) for each configuration dictionary. The first argument is the PID of the Configuration object (the one created by the Configuration Admin service) and the second argument contains the configuration properties." This would also reduce the critical area for a deadlock. If you call the updated method on your own during the init() method the chance to get caught into the deadlock described here https://issues.apache.org/jira/browse/ARIES-867 increases. Clear: the deadlock situation is not totally mitigated due to the service registration in the init() method, while holding the BlueprintContainer lock...
        Show
        Guillaume Nodet added a comment - See rev. http://svn.apache.org/viewvc?view=revision&revision=1364914
        Hide
        Hendy Irawan added a comment -

        Thank you Guillaume !

        BTW forgive me to ask again... is there any plans to support managed service factory properties without implementing 'updated()' ? i.e. using the same behavior as managed service (bean initially created using managed properties, if the managed properties change then the bean gets reloaded).

        Show
        Hendy Irawan added a comment - Thank you Guillaume ! BTW forgive me to ask again... is there any plans to support managed service factory properties without implementing 'updated()' ? i.e. using the same behavior as managed service (bean initially created using managed properties, if the managed properties change then the bean gets reloaded).

          People

          • Assignee:
            Guillaume Nodet
            Reporter:
            Greg Rapp
          • Votes:
            7 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development