[jira] Created: (FELIX-1841) SCR invokes bind method twice when dependency service properties are modified

SCR invokes bind method twice when dependency service properties are modified
-----------------------------------------------------------------------------

                 Key: FELIX-1841
                 URL: https://issues.apache.org/jira/browse/FELIX-1841
             Project: Felix
          Issue Type: Bug
          Components: Declarative Services (SCR)
            Reporter: Pierre De Rop
            Priority: Minor


It seems that when a service S is depending on another service D, and when D service properties are modified (using ServiceRegistration.setProperties method), then D is
re-bound twice into the using service S.

For example, I have a service "Client" has a "1..1" dependency over "Service":

<?xml version="1.0" encoding="UTF-8"?>
<component name="Client">
  <implementation class="client.Client"/>
  <reference name="SERVICE"
    interface="service.Service"
    policy="dynamic"
    cardinality="1..1"
    target="(foo=bar)"
    bind="bind"/>
</component>

public class Client {
  protected void bind(Service s) {
    System.out.println("Client:: bound Service : " + s);
    Thread.dumpStack();
  }
}



I have another bundle which provide the "Service" dependency, and sometimes, the "Service" properties are modified like this:

        ServiceRegistration reg ...
        reg.setProperties(...)
 
So, when the setProperties takes place, "Client" is re-bound twice with the service "Service".
Indeed, in DependencyManager, when a ServiceEvent.MODIFIED event. is caught, the following code is invoked (line 170):


            case ServiceEvent.MODIFIED:
                m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
                    { serviceString }, null );

                // remove the service first
                // only continue with further event handling if the service
                // removal did not cause the component to be deactivated
                if ( serviceRemoved( ref ) )
                {
                    // recalculate the number of services matching the filter
                    // because we don't know whether this service previously matched
                    // or not
                    ServiceReference refs[] = getFrameworkServiceReferences();
                    m_size = ( refs == null ) ? 0 : refs.length;

                    // now try to bind the service - if it matches the target filter
                    // without recalculating the size (already done).
                    if ( targetFilterMatch( ref ) )
                    {
                        serviceAdded( ref );
                    }
                }

                break;

So, the service is first re-bound to the Client, when the serviceRemoved() method is invoked (it's a bound service replacement, I guess).
But the problem here is that the modified service is also re-bound, when serviceAdded is invoked (line 189).
Don't you think that this is a bug and the service should be re-bound only once, not twice ?

Here is the first stacktrace of the first bind:

java.lang.Exception: Stack trace
        at java.lang.Thread.dumpStack(Thread.java:1158)
        at client.Client.bind(Client.java:13)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:592)
        at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
        at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
        at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
        at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
        at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
        at org.apache.felix.scr.impl.manager.DependencyManager.bind(DependencyManager.java:884)
        at org.apache.felix.scr.impl.manager.DependencyManager.serviceRemoved(DependencyManager.java:367)
        at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:177)
        at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
        at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
        at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
        at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
        at org.apache.felix.framework.Felix.access$000(Felix.java:40)
        at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
        at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
        at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
        at service.impl.ServiceFactory.run(ServiceFactory.java:48)
        at java.lang.Thread.run(Thread.java:595)

and here is the second stacktrace, when Service is re-bound:

Client: bound Service : service.impl.ServiceImpl@142a80d
java.lang.Exception: Stack trace
        at java.lang.Thread.dumpStack(Thread.java:1158)
        at client.Client.bind(Client.java:13)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:592)
        at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
        at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
        at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
        at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
        at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
        at org.apache.felix.scr.impl.manager.DependencyManager.serviceAdded(DependencyManager.java:271)
        at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:189)
        at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
        at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
        at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
        at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
        at org.apache.felix.framework.Felix.access$000(Felix.java:40)
        at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
        at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
        at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
        at service.impl.ServiceFactory.run(ServiceFactory.java:48)
        at java.lang.Thread.run(Thread.java:595)


--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/FELIX-1841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12773264#action_12773264 ]

Pierre De Rop commented on FELIX-1841:
--------------------------------------

Felix,

I'm not sure, but it seems that the issue comes from the DependencyManager.ungetService(), which is wrongly invoked
from the DependencyManager.serviceRemoved() method:

Let me try to explain:

- The service "Service" is injected to "Client" service, which do have a (1..1) dependency over "Service".
- Then the "Service" service properties are updated.
- So, in DependencyManager.java, the serviceChanged is called with a MODIFIED event:

            case ServiceEvent.MODIFIED:
                m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
                    { serviceString }, null );

                // remove the service first
                // only continue with further event handling if the service
                // removal did not cause the component to be deactivated
                if ( serviceRemoved( ref ) )
                {
                    // recalculate the number of services matching the filter
                    // because we don't know whether this service previously matched
                    // or not
                    ServiceReference refs[] = getFrameworkServiceReferences();
                    m_size = ( refs == null ) ? 0 : refs.length;

                    // now try to bind the service - if it matches the target filter
                    // without recalculating the size (already done).
                    if ( targetFilterMatch( ref ) )
                    {
                        serviceAdded( ref );
                    }
                }

- Here, the serviceRemoved method is then invoked, which ends up in doing a bound service replacement (line 358):

            else
            {

                // try to bind a replacement service first if this is a unary
                // cardinality reference and a replacement is available.
                if ( !m_dependencyMetadata.isMultiple() )
                {
                    // if the dependency is mandatory and no replacement is
                    // available, bind returns false and we deactivate
                    if ( !bind() )
                    {
                        m_componentManager
                            .log(
                                LogService.LOG_DEBUG,
                                "Dependency Manager: Deactivating component due to mandatory dependency on {0}/{1} not satisfied",
                                new Object[]
                                    { m_dependencyMetadata.getName(), m_dependencyMetadata.getInterface() }, null );
                        m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE );

                        // required service could not be replaced, component
                        // is deactivated and we are done
                        return false;
                    }
                }

                // call the unbind method if one is defined
                if ( m_dependencyMetadata.getUnbind() != null )
                {
                    invokeUnbindMethod( reference );
                }

                // make sure the service is returned
                ungetService( reference );
            }

- the "if (!bind())" line will re-bind "Client" to the same "Service" service, as "Service" properties are just modified (and "Service" is still present)
- The invokeUnbindMethod will then unbind the same service
- And the ungetService method will remove the bound service from the m_bound map -> but I think that this is the issue, because the map should contain the same "Service" service, which has just been re-bound, in the bind() method.
- So, let's follow up with the issue: the "serviceAdded" method is then called from the serviceChanged() method.
- The serviceAdded method will then attempt to bind the same service, line 268 (but we already did it from the serviceRemoved method), and here, the isBound() returns false because the m_bound map has been wrongly emptied previously.

                // multiple bindings or not bound at all yet
                if ( m_dependencyMetadata.isMultiple() || !isBound() )
                {
                    // bind the service, getting it if required
                    invokeBindMethod( reference );
                }
            }


So, I replaced the else bloc (line 358) with the following patch (which is attached to this issue): and the problem seems to disappear:


            // dynamic dependency, multiple or single but this service is the bound one
            else
            {
                boolean doUnget = true;
               
                // try to bind a replacement service first if this is a unary
                // cardinality reference and a replacement is available.
                if ( !m_dependencyMetadata.isMultiple() )
                {
                    // if the dependency is mandatory and no replacement is
                    // available, bind returns false and we deactivate
                    if ( !bind() )
                    {
                        m_componentManager
                            .log(
                                LogService.LOG_DEBUG,
                                "Dependency Manager: Deactivating component due to mandatory dependency on {0}/{1} not satisfied",
                                new Object[]
                                    { m_dependencyMetadata.getName(), m_dependencyMetadata.getInterface() }, null );
                        m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE );

                        // required service could not be replaced, component
                        // is deactivated and we are done
                        return false;
                    }
                   
                    // Check if we have been rebound to the same (modified) service.
                    if (m_bound.size() == 1 && getBoundService( reference ) != null)  {
                        // the same (modified) service has been re-bound: don't unget.
                        doUnget = false;
                    }
                }

                // call the unbind method if one is defined
                if ( m_dependencyMetadata.getUnbind() != null )
                {
                    invokeUnbindMethod( reference );
                }

                // make sure the service is returned.
                if (doUnget) {
                    ungetService( reference );
                }
            }


Does this make sense to you ? Am I correct or did I miss something else ?


Thanks;
/pierre

> SCR invokes bind method twice when dependency service properties are modified
> -----------------------------------------------------------------------------
>
>                 Key: FELIX-1841
>                 URL: https://issues.apache.org/jira/browse/FELIX-1841
>             Project: Felix
>          Issue Type: Bug
>          Components: Declarative Services (SCR)
>            Reporter: Pierre De Rop
>            Priority: Minor
>
> It seems that when a service S is depending on another service D, and when D service properties are modified (using ServiceRegistration.setProperties method), then D is
> re-bound twice into the using service S.
> For example, I have a service "Client" has a "1..1" dependency over "Service":
> <?xml version="1.0" encoding="UTF-8"?>
> <component name="Client">
>   <implementation class="client.Client"/>
>   <reference name="SERVICE"
>     interface="service.Service"
>     policy="dynamic"
>     cardinality="1..1"
>     target="(foo=bar)"
>     bind="bind"/>
> </component>
> public class Client {
>   protected void bind(Service s) {
>     System.out.println("Client:: bound Service : " + s);
>     Thread.dumpStack();
>   }
> }
> I have another bundle which provide the "Service" dependency, and sometimes, the "Service" properties are modified like this:
>         ServiceRegistration reg ...
>         reg.setProperties(...)
>  
> So, when the setProperties takes place, "Client" is re-bound twice with the service "Service".
> Indeed, in DependencyManager, when a ServiceEvent.MODIFIED event. is caught, the following code is invoked (line 170):
>             case ServiceEvent.MODIFIED:
>                 m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
>                     { serviceString }, null );
>                 // remove the service first
>                 // only continue with further event handling if the service
>                 // removal did not cause the component to be deactivated
>                 if ( serviceRemoved( ref ) )
>                 {
>                     // recalculate the number of services matching the filter
>                     // because we don't know whether this service previously matched
>                     // or not
>                     ServiceReference refs[] = getFrameworkServiceReferences();
>                     m_size = ( refs == null ) ? 0 : refs.length;
>                     // now try to bind the service - if it matches the target filter
>                     // without recalculating the size (already done).
>                     if ( targetFilterMatch( ref ) )
>                     {
>                         serviceAdded( ref );
>                     }
>                 }
>                 break;
> So, the service is first re-bound to the Client, when the serviceRemoved() method is invoked (it's a bound service replacement, I guess).
> But the problem here is that the modified service is also re-bound, when serviceAdded is invoked (line 189).
> Don't you think that this is a bug and the service should be re-bound only once, not twice ?
> Here is the first stacktrace of the first bind:
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.bind(DependencyManager.java:884)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceRemoved(DependencyManager.java:367)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:177)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)
> and here is the second stacktrace, when Service is re-bound:
> Client: bound Service : service.impl.ServiceImpl@142a80d
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceAdded(DependencyManager.java:271)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:189)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/FELIX-1841?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Pierre De Rop updated FELIX-1841:
---------------------------------

    Attachment: DependencyManager.java

> SCR invokes bind method twice when dependency service properties are modified
> -----------------------------------------------------------------------------
>
>                 Key: FELIX-1841
>                 URL: https://issues.apache.org/jira/browse/FELIX-1841
>             Project: Felix
>          Issue Type: Bug
>          Components: Declarative Services (SCR)
>            Reporter: Pierre De Rop
>            Priority: Minor
>         Attachments: DependencyManager.java
>
>
> It seems that when a service S is depending on another service D, and when D service properties are modified (using ServiceRegistration.setProperties method), then D is
> re-bound twice into the using service S.
> For example, I have a service "Client" has a "1..1" dependency over "Service":
> <?xml version="1.0" encoding="UTF-8"?>
> <component name="Client">
>   <implementation class="client.Client"/>
>   <reference name="SERVICE"
>     interface="service.Service"
>     policy="dynamic"
>     cardinality="1..1"
>     target="(foo=bar)"
>     bind="bind"/>
> </component>
> public class Client {
>   protected void bind(Service s) {
>     System.out.println("Client:: bound Service : " + s);
>     Thread.dumpStack();
>   }
> }
> I have another bundle which provide the "Service" dependency, and sometimes, the "Service" properties are modified like this:
>         ServiceRegistration reg ...
>         reg.setProperties(...)
>  
> So, when the setProperties takes place, "Client" is re-bound twice with the service "Service".
> Indeed, in DependencyManager, when a ServiceEvent.MODIFIED event. is caught, the following code is invoked (line 170):
>             case ServiceEvent.MODIFIED:
>                 m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
>                     { serviceString }, null );
>                 // remove the service first
>                 // only continue with further event handling if the service
>                 // removal did not cause the component to be deactivated
>                 if ( serviceRemoved( ref ) )
>                 {
>                     // recalculate the number of services matching the filter
>                     // because we don't know whether this service previously matched
>                     // or not
>                     ServiceReference refs[] = getFrameworkServiceReferences();
>                     m_size = ( refs == null ) ? 0 : refs.length;
>                     // now try to bind the service - if it matches the target filter
>                     // without recalculating the size (already done).
>                     if ( targetFilterMatch( ref ) )
>                     {
>                         serviceAdded( ref );
>                     }
>                 }
>                 break;
> So, the service is first re-bound to the Client, when the serviceRemoved() method is invoked (it's a bound service replacement, I guess).
> But the problem here is that the modified service is also re-bound, when serviceAdded is invoked (line 189).
> Don't you think that this is a bug and the service should be re-bound only once, not twice ?
> Here is the first stacktrace of the first bind:
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.bind(DependencyManager.java:884)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceRemoved(DependencyManager.java:367)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:177)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)
> and here is the second stacktrace, when Service is re-bound:
> Client: bound Service : service.impl.ServiceImpl@142a80d
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceAdded(DependencyManager.java:271)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:189)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/FELIX-1841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12773334#action_12773334 ]

Felix Meschberger commented on FELIX-1841:
------------------------------------------

Your are correct: The problem is that the serviceRemoved method correctly tries to bind another service to the component for the service that was "removed". Now the problem is, that this is just the same service because it has not really been removed.

This can probably be solved with your patch (I just glanced over it) -- the point is that we should not try to bind to the same service, we intend to unbind.

The other problem in this concrete case is, that actually, we cannot unbind, check and rebind because the dependency is required and thus unbinding the service would actually (theoretically) require to take down the component  and reactivate component. And this is not what I would like to do in this case...

Maybe I should really be doing more in the serviceChanged method handling the MODIFIED event: Check whether the service in question is actually bind and only if it is bound check whether the target filter still matches and ensure the service is removed if not matching anymore and doing nothing if not.

But then: we would probably want to inform the client that the server has modified its properties, so rebinding is definitely the option to follow up on.

Not sure right now ....

> SCR invokes bind method twice when dependency service properties are modified
> -----------------------------------------------------------------------------
>
>                 Key: FELIX-1841
>                 URL: https://issues.apache.org/jira/browse/FELIX-1841
>             Project: Felix
>          Issue Type: Bug
>          Components: Declarative Services (SCR)
>            Reporter: Pierre De Rop
>            Priority: Minor
>         Attachments: DependencyManager.java
>
>
> It seems that when a service S is depending on another service D, and when D service properties are modified (using ServiceRegistration.setProperties method), then D is
> re-bound twice into the using service S.
> For example, I have a service "Client" has a "1..1" dependency over "Service":
> <?xml version="1.0" encoding="UTF-8"?>
> <component name="Client">
>   <implementation class="client.Client"/>
>   <reference name="SERVICE"
>     interface="service.Service"
>     policy="dynamic"
>     cardinality="1..1"
>     target="(foo=bar)"
>     bind="bind"/>
> </component>
> public class Client {
>   protected void bind(Service s) {
>     System.out.println("Client:: bound Service : " + s);
>     Thread.dumpStack();
>   }
> }
> I have another bundle which provide the "Service" dependency, and sometimes, the "Service" properties are modified like this:
>         ServiceRegistration reg ...
>         reg.setProperties(...)
>  
> So, when the setProperties takes place, "Client" is re-bound twice with the service "Service".
> Indeed, in DependencyManager, when a ServiceEvent.MODIFIED event. is caught, the following code is invoked (line 170):
>             case ServiceEvent.MODIFIED:
>                 m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
>                     { serviceString }, null );
>                 // remove the service first
>                 // only continue with further event handling if the service
>                 // removal did not cause the component to be deactivated
>                 if ( serviceRemoved( ref ) )
>                 {
>                     // recalculate the number of services matching the filter
>                     // because we don't know whether this service previously matched
>                     // or not
>                     ServiceReference refs[] = getFrameworkServiceReferences();
>                     m_size = ( refs == null ) ? 0 : refs.length;
>                     // now try to bind the service - if it matches the target filter
>                     // without recalculating the size (already done).
>                     if ( targetFilterMatch( ref ) )
>                     {
>                         serviceAdded( ref );
>                     }
>                 }
>                 break;
> So, the service is first re-bound to the Client, when the serviceRemoved() method is invoked (it's a bound service replacement, I guess).
> But the problem here is that the modified service is also re-bound, when serviceAdded is invoked (line 189).
> Don't you think that this is a bug and the service should be re-bound only once, not twice ?
> Here is the first stacktrace of the first bind:
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.bind(DependencyManager.java:884)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceRemoved(DependencyManager.java:367)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:177)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)
> and here is the second stacktrace, when Service is re-bound:
> Client: bound Service : service.impl.ServiceImpl@142a80d
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceAdded(DependencyManager.java:271)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:189)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/FELIX-1841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12773406#action_12773406 ]

Pierre De Rop commented on FELIX-1841:
--------------------------------------

Ok. I do agree;

Just calling back the "Client" in its bind method, when the "Service" properties are modified sounds like to be enough, and as you said, we don't really want to reactivate the "Client ...

I checked in the specification, but I did not find anything regarding this issue.
I think that this use case is a sub-set of the "bound service replacement" feature: the "bind" is called with the same service, but not the "unbind" ...

For now, we'll use my patch in our application server, because, well, we really need it.

thanks;
/pierre

> SCR invokes bind method twice when dependency service properties are modified
> -----------------------------------------------------------------------------
>
>                 Key: FELIX-1841
>                 URL: https://issues.apache.org/jira/browse/FELIX-1841
>             Project: Felix
>          Issue Type: Bug
>          Components: Declarative Services (SCR)
>            Reporter: Pierre De Rop
>            Priority: Minor
>         Attachments: DependencyManager.java
>
>
> It seems that when a service S is depending on another service D, and when D service properties are modified (using ServiceRegistration.setProperties method), then D is
> re-bound twice into the using service S.
> For example, I have a service "Client" has a "1..1" dependency over "Service":
> <?xml version="1.0" encoding="UTF-8"?>
> <component name="Client">
>   <implementation class="client.Client"/>
>   <reference name="SERVICE"
>     interface="service.Service"
>     policy="dynamic"
>     cardinality="1..1"
>     target="(foo=bar)"
>     bind="bind"/>
> </component>
> public class Client {
>   protected void bind(Service s) {
>     System.out.println("Client:: bound Service : " + s);
>     Thread.dumpStack();
>   }
> }
> I have another bundle which provide the "Service" dependency, and sometimes, the "Service" properties are modified like this:
>         ServiceRegistration reg ...
>         reg.setProperties(...)
>  
> So, when the setProperties takes place, "Client" is re-bound twice with the service "Service".
> Indeed, in DependencyManager, when a ServiceEvent.MODIFIED event. is caught, the following code is invoked (line 170):
>             case ServiceEvent.MODIFIED:
>                 m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
>                     { serviceString }, null );
>                 // remove the service first
>                 // only continue with further event handling if the service
>                 // removal did not cause the component to be deactivated
>                 if ( serviceRemoved( ref ) )
>                 {
>                     // recalculate the number of services matching the filter
>                     // because we don't know whether this service previously matched
>                     // or not
>                     ServiceReference refs[] = getFrameworkServiceReferences();
>                     m_size = ( refs == null ) ? 0 : refs.length;
>                     // now try to bind the service - if it matches the target filter
>                     // without recalculating the size (already done).
>                     if ( targetFilterMatch( ref ) )
>                     {
>                         serviceAdded( ref );
>                     }
>                 }
>                 break;
> So, the service is first re-bound to the Client, when the serviceRemoved() method is invoked (it's a bound service replacement, I guess).
> But the problem here is that the modified service is also re-bound, when serviceAdded is invoked (line 189).
> Don't you think that this is a bug and the service should be re-bound only once, not twice ?
> Here is the first stacktrace of the first bind:
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.bind(DependencyManager.java:884)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceRemoved(DependencyManager.java:367)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:177)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)
> and here is the second stacktrace, when Service is re-bound:
> Client: bound Service : service.impl.ServiceImpl@142a80d
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceAdded(DependencyManager.java:271)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:189)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/FELIX-1841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12774002#action_12774002 ]

Felix Meschberger commented on FELIX-1841:
------------------------------------------

Good. If this works out for you. In the meantime I try to find out how we could solve this problem...

> SCR invokes bind method twice when dependency service properties are modified
> -----------------------------------------------------------------------------
>
>                 Key: FELIX-1841
>                 URL: https://issues.apache.org/jira/browse/FELIX-1841
>             Project: Felix
>          Issue Type: Bug
>          Components: Declarative Services (SCR)
>            Reporter: Pierre De Rop
>            Priority: Minor
>         Attachments: DependencyManager.java
>
>
> It seems that when a service S is depending on another service D, and when D service properties are modified (using ServiceRegistration.setProperties method), then D is
> re-bound twice into the using service S.
> For example, I have a service "Client" has a "1..1" dependency over "Service":
> <?xml version="1.0" encoding="UTF-8"?>
> <component name="Client">
>   <implementation class="client.Client"/>
>   <reference name="SERVICE"
>     interface="service.Service"
>     policy="dynamic"
>     cardinality="1..1"
>     target="(foo=bar)"
>     bind="bind"/>
> </component>
> public class Client {
>   protected void bind(Service s) {
>     System.out.println("Client:: bound Service : " + s);
>     Thread.dumpStack();
>   }
> }
> I have another bundle which provide the "Service" dependency, and sometimes, the "Service" properties are modified like this:
>         ServiceRegistration reg ...
>         reg.setProperties(...)
>  
> So, when the setProperties takes place, "Client" is re-bound twice with the service "Service".
> Indeed, in DependencyManager, when a ServiceEvent.MODIFIED event. is caught, the following code is invoked (line 170):
>             case ServiceEvent.MODIFIED:
>                 m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
>                     { serviceString }, null );
>                 // remove the service first
>                 // only continue with further event handling if the service
>                 // removal did not cause the component to be deactivated
>                 if ( serviceRemoved( ref ) )
>                 {
>                     // recalculate the number of services matching the filter
>                     // because we don't know whether this service previously matched
>                     // or not
>                     ServiceReference refs[] = getFrameworkServiceReferences();
>                     m_size = ( refs == null ) ? 0 : refs.length;
>                     // now try to bind the service - if it matches the target filter
>                     // without recalculating the size (already done).
>                     if ( targetFilterMatch( ref ) )
>                     {
>                         serviceAdded( ref );
>                     }
>                 }
>                 break;
> So, the service is first re-bound to the Client, when the serviceRemoved() method is invoked (it's a bound service replacement, I guess).
> But the problem here is that the modified service is also re-bound, when serviceAdded is invoked (line 189).
> Don't you think that this is a bug and the service should be re-bound only once, not twice ?
> Here is the first stacktrace of the first bind:
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.bind(DependencyManager.java:884)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceRemoved(DependencyManager.java:367)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:177)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)
> and here is the second stacktrace, when Service is re-bound:
> Client: bound Service : service.impl.ServiceImpl@142a80d
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceAdded(DependencyManager.java:271)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:189)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/FELIX-1841?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Felix Meschberger updated FELIX-1841:
-------------------------------------

    Attachment: FELIX-1841.patch

Another proposed patch:

  * If the service is not currently bound, bind if the filter matches and binding makes sense
  * If the service is currently bound, check whether the filter matches and bind if it matches.

> SCR invokes bind method twice when dependency service properties are modified
> -----------------------------------------------------------------------------
>
>                 Key: FELIX-1841
>                 URL: https://issues.apache.org/jira/browse/FELIX-1841
>             Project: Felix
>          Issue Type: Bug
>          Components: Declarative Services (SCR)
>            Reporter: Pierre De Rop
>            Priority: Minor
>         Attachments: DependencyManager.java, FELIX-1841.patch
>
>
> It seems that when a service S is depending on another service D, and when D service properties are modified (using ServiceRegistration.setProperties method), then D is
> re-bound twice into the using service S.
> For example, I have a service "Client" has a "1..1" dependency over "Service":
> <?xml version="1.0" encoding="UTF-8"?>
> <component name="Client">
>   <implementation class="client.Client"/>
>   <reference name="SERVICE"
>     interface="service.Service"
>     policy="dynamic"
>     cardinality="1..1"
>     target="(foo=bar)"
>     bind="bind"/>
> </component>
> public class Client {
>   protected void bind(Service s) {
>     System.out.println("Client:: bound Service : " + s);
>     Thread.dumpStack();
>   }
> }
> I have another bundle which provide the "Service" dependency, and sometimes, the "Service" properties are modified like this:
>         ServiceRegistration reg ...
>         reg.setProperties(...)
>  
> So, when the setProperties takes place, "Client" is re-bound twice with the service "Service".
> Indeed, in DependencyManager, when a ServiceEvent.MODIFIED event. is caught, the following code is invoked (line 170):
>             case ServiceEvent.MODIFIED:
>                 m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
>                     { serviceString }, null );
>                 // remove the service first
>                 // only continue with further event handling if the service
>                 // removal did not cause the component to be deactivated
>                 if ( serviceRemoved( ref ) )
>                 {
>                     // recalculate the number of services matching the filter
>                     // because we don't know whether this service previously matched
>                     // or not
>                     ServiceReference refs[] = getFrameworkServiceReferences();
>                     m_size = ( refs == null ) ? 0 : refs.length;
>                     // now try to bind the service - if it matches the target filter
>                     // without recalculating the size (already done).
>                     if ( targetFilterMatch( ref ) )
>                     {
>                         serviceAdded( ref );
>                     }
>                 }
>                 break;
> So, the service is first re-bound to the Client, when the serviceRemoved() method is invoked (it's a bound service replacement, I guess).
> But the problem here is that the modified service is also re-bound, when serviceAdded is invoked (line 189).
> Don't you think that this is a bug and the service should be re-bound only once, not twice ?
> Here is the first stacktrace of the first bind:
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.bind(DependencyManager.java:884)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceRemoved(DependencyManager.java:367)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:177)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)
> and here is the second stacktrace, when Service is re-bound:
> Client: bound Service : service.impl.ServiceImpl@142a80d
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceAdded(DependencyManager.java:271)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:189)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/FELIX-1841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12778154#action_12778154 ]

Pierre De Rop commented on FELIX-1841:
--------------------------------------

Felix,

I have tested the patch for this issue and for the other FELIX-1866 issue.
Indeed, when the service properties are changed, no re-bind is attempted at all.

But I am confused now and I realize that I did not initially state my problems clearly.
So, let me try to go to the right point and reformulate my problems:

I have two problems:

1) Using CM, When I update some public service properties (not starting with a "."), I would like to re-bind
the modified service to the other components which depend on it (just once, not twice)

2) When I update (using CM) only some private service properties (prefixed with a "."), then I would like to get my
component called in its "modified" callback but the component should not be re-bound to other
components which depend on it, because the modified private properties are not supposed to be published
(see 112.6.1, in spec, am I correct ?)

So, what do you think about the following strategy:

1) add a "else" in your patch in order to re-bind once:

                ...
                else if ( !targetFilterMatch( ref ) )
                {
                    m_size--;
                    serviceRemoved( ref );
                } else {
->                bind(); // -> re-bind once.
                }

2) in ImmediateComponentManager.java, around line 472 -> check if the properties modified from CM are private (start with a ".") and don't invoke "sr.setProperties( regProps )" if modified properties are private.

Does this make sense ?

> SCR invokes bind method twice when dependency service properties are modified
> -----------------------------------------------------------------------------
>
>                 Key: FELIX-1841
>                 URL: https://issues.apache.org/jira/browse/FELIX-1841
>             Project: Felix
>          Issue Type: Bug
>          Components: Declarative Services (SCR)
>            Reporter: Pierre De Rop
>            Priority: Minor
>         Attachments: DependencyManager.java, FELIX-1841.patch
>
>
> It seems that when a service S is depending on another service D, and when D service properties are modified (using ServiceRegistration.setProperties method), then D is
> re-bound twice into the using service S.
> For example, I have a service "Client" has a "1..1" dependency over "Service":
> <?xml version="1.0" encoding="UTF-8"?>
> <component name="Client">
>   <implementation class="client.Client"/>
>   <reference name="SERVICE"
>     interface="service.Service"
>     policy="dynamic"
>     cardinality="1..1"
>     target="(foo=bar)"
>     bind="bind"/>
> </component>
> public class Client {
>   protected void bind(Service s) {
>     System.out.println("Client:: bound Service : " + s);
>     Thread.dumpStack();
>   }
> }
> I have another bundle which provide the "Service" dependency, and sometimes, the "Service" properties are modified like this:
>         ServiceRegistration reg ...
>         reg.setProperties(...)
>  
> So, when the setProperties takes place, "Client" is re-bound twice with the service "Service".
> Indeed, in DependencyManager, when a ServiceEvent.MODIFIED event. is caught, the following code is invoked (line 170):
>             case ServiceEvent.MODIFIED:
>                 m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
>                     { serviceString }, null );
>                 // remove the service first
>                 // only continue with further event handling if the service
>                 // removal did not cause the component to be deactivated
>                 if ( serviceRemoved( ref ) )
>                 {
>                     // recalculate the number of services matching the filter
>                     // because we don't know whether this service previously matched
>                     // or not
>                     ServiceReference refs[] = getFrameworkServiceReferences();
>                     m_size = ( refs == null ) ? 0 : refs.length;
>                     // now try to bind the service - if it matches the target filter
>                     // without recalculating the size (already done).
>                     if ( targetFilterMatch( ref ) )
>                     {
>                         serviceAdded( ref );
>                     }
>                 }
>                 break;
> So, the service is first re-bound to the Client, when the serviceRemoved() method is invoked (it's a bound service replacement, I guess).
> But the problem here is that the modified service is also re-bound, when serviceAdded is invoked (line 189).
> Don't you think that this is a bug and the service should be re-bound only once, not twice ?
> Here is the first stacktrace of the first bind:
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.bind(DependencyManager.java:884)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceRemoved(DependencyManager.java:367)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:177)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)
> and here is the second stacktrace, when Service is re-bound:
> Client: bound Service : service.impl.ServiceImpl@142a80d
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceAdded(DependencyManager.java:271)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:189)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/FELIX-1841?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Pierre De Rop updated FELIX-1841:
---------------------------------

    Attachment: FELIX-1841.patch.2

Felix,
I tried to implement about what I was talking earlier.
Please review FELIX-1841.patch.2 (I probably missed something, but it seems to work).

/pierre

> SCR invokes bind method twice when dependency service properties are modified
> -----------------------------------------------------------------------------
>
>                 Key: FELIX-1841
>                 URL: https://issues.apache.org/jira/browse/FELIX-1841
>             Project: Felix
>          Issue Type: Bug
>          Components: Declarative Services (SCR)
>            Reporter: Pierre De Rop
>            Priority: Minor
>         Attachments: DependencyManager.java, FELIX-1841.patch, FELIX-1841.patch.2
>
>
> It seems that when a service S is depending on another service D, and when D service properties are modified (using ServiceRegistration.setProperties method), then D is
> re-bound twice into the using service S.
> For example, I have a service "Client" has a "1..1" dependency over "Service":
> <?xml version="1.0" encoding="UTF-8"?>
> <component name="Client">
>   <implementation class="client.Client"/>
>   <reference name="SERVICE"
>     interface="service.Service"
>     policy="dynamic"
>     cardinality="1..1"
>     target="(foo=bar)"
>     bind="bind"/>
> </component>
> public class Client {
>   protected void bind(Service s) {
>     System.out.println("Client:: bound Service : " + s);
>     Thread.dumpStack();
>   }
> }
> I have another bundle which provide the "Service" dependency, and sometimes, the "Service" properties are modified like this:
>         ServiceRegistration reg ...
>         reg.setProperties(...)
>  
> So, when the setProperties takes place, "Client" is re-bound twice with the service "Service".
> Indeed, in DependencyManager, when a ServiceEvent.MODIFIED event. is caught, the following code is invoked (line 170):
>             case ServiceEvent.MODIFIED:
>                 m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
>                     { serviceString }, null );
>                 // remove the service first
>                 // only continue with further event handling if the service
>                 // removal did not cause the component to be deactivated
>                 if ( serviceRemoved( ref ) )
>                 {
>                     // recalculate the number of services matching the filter
>                     // because we don't know whether this service previously matched
>                     // or not
>                     ServiceReference refs[] = getFrameworkServiceReferences();
>                     m_size = ( refs == null ) ? 0 : refs.length;
>                     // now try to bind the service - if it matches the target filter
>                     // without recalculating the size (already done).
>                     if ( targetFilterMatch( ref ) )
>                     {
>                         serviceAdded( ref );
>                     }
>                 }
>                 break;
> So, the service is first re-bound to the Client, when the serviceRemoved() method is invoked (it's a bound service replacement, I guess).
> But the problem here is that the modified service is also re-bound, when serviceAdded is invoked (line 189).
> Don't you think that this is a bug and the service should be re-bound only once, not twice ?
> Here is the first stacktrace of the first bind:
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.bind(DependencyManager.java:884)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceRemoved(DependencyManager.java:367)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:177)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)
> and here is the second stacktrace, when Service is re-bound:
> Client: bound Service : service.impl.ServiceImpl@142a80d
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceAdded(DependencyManager.java:271)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:189)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/FELIX-1841?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Felix Meschberger reassigned FELIX-1841:
----------------------------------------

    Assignee: Felix Meschberger

> SCR invokes bind method twice when dependency service properties are modified
> -----------------------------------------------------------------------------
>
>                 Key: FELIX-1841
>                 URL: https://issues.apache.org/jira/browse/FELIX-1841
>             Project: Felix
>          Issue Type: Bug
>          Components: Declarative Services (SCR)
>            Reporter: Pierre De Rop
>            Assignee: Felix Meschberger
>            Priority: Minor
>         Attachments: DependencyManager.java, FELIX-1841.patch, FELIX-1841.patch.2
>
>
> It seems that when a service S is depending on another service D, and when D service properties are modified (using ServiceRegistration.setProperties method), then D is
> re-bound twice into the using service S.
> For example, I have a service "Client" has a "1..1" dependency over "Service":
> <?xml version="1.0" encoding="UTF-8"?>
> <component name="Client">
>   <implementation class="client.Client"/>
>   <reference name="SERVICE"
>     interface="service.Service"
>     policy="dynamic"
>     cardinality="1..1"
>     target="(foo=bar)"
>     bind="bind"/>
> </component>
> public class Client {
>   protected void bind(Service s) {
>     System.out.println("Client:: bound Service : " + s);
>     Thread.dumpStack();
>   }
> }
> I have another bundle which provide the "Service" dependency, and sometimes, the "Service" properties are modified like this:
>         ServiceRegistration reg ...
>         reg.setProperties(...)
>  
> So, when the setProperties takes place, "Client" is re-bound twice with the service "Service".
> Indeed, in DependencyManager, when a ServiceEvent.MODIFIED event. is caught, the following code is invoked (line 170):
>             case ServiceEvent.MODIFIED:
>                 m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
>                     { serviceString }, null );
>                 // remove the service first
>                 // only continue with further event handling if the service
>                 // removal did not cause the component to be deactivated
>                 if ( serviceRemoved( ref ) )
>                 {
>                     // recalculate the number of services matching the filter
>                     // because we don't know whether this service previously matched
>                     // or not
>                     ServiceReference refs[] = getFrameworkServiceReferences();
>                     m_size = ( refs == null ) ? 0 : refs.length;
>                     // now try to bind the service - if it matches the target filter
>                     // without recalculating the size (already done).
>                     if ( targetFilterMatch( ref ) )
>                     {
>                         serviceAdded( ref );
>                     }
>                 }
>                 break;
> So, the service is first re-bound to the Client, when the serviceRemoved() method is invoked (it's a bound service replacement, I guess).
> But the problem here is that the modified service is also re-bound, when serviceAdded is invoked (line 189).
> Don't you think that this is a bug and the service should be re-bound only once, not twice ?
> Here is the first stacktrace of the first bind:
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.bind(DependencyManager.java:884)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceRemoved(DependencyManager.java:367)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:177)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)
> and here is the second stacktrace, when Service is re-bound:
> Client: bound Service : service.impl.ServiceImpl@142a80d
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceAdded(DependencyManager.java:271)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:189)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/FELIX-1841?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12781873#action_12781873 ]

Felix Meschberger commented on FELIX-1841:
------------------------------------------

Applied the ImmediateComponentManager part of the Felix-1841.patch.2 supplied by Pierre de Rop (thanks alot) in Rev. 883652. This now prevents an update of the service registration  if the configuration update does not have an influence on the service registration (such as is the case if the configuration update only is for private properties)

> SCR invokes bind method twice when dependency service properties are modified
> -----------------------------------------------------------------------------
>
>                 Key: FELIX-1841
>                 URL: https://issues.apache.org/jira/browse/FELIX-1841
>             Project: Felix
>          Issue Type: Bug
>          Components: Declarative Services (SCR)
>            Reporter: Pierre De Rop
>            Assignee: Felix Meschberger
>            Priority: Minor
>         Attachments: DependencyManager.java, FELIX-1841.patch, FELIX-1841.patch.2
>
>
> It seems that when a service S is depending on another service D, and when D service properties are modified (using ServiceRegistration.setProperties method), then D is
> re-bound twice into the using service S.
> For example, I have a service "Client" has a "1..1" dependency over "Service":
> <?xml version="1.0" encoding="UTF-8"?>
> <component name="Client">
>   <implementation class="client.Client"/>
>   <reference name="SERVICE"
>     interface="service.Service"
>     policy="dynamic"
>     cardinality="1..1"
>     target="(foo=bar)"
>     bind="bind"/>
> </component>
> public class Client {
>   protected void bind(Service s) {
>     System.out.println("Client:: bound Service : " + s);
>     Thread.dumpStack();
>   }
> }
> I have another bundle which provide the "Service" dependency, and sometimes, the "Service" properties are modified like this:
>         ServiceRegistration reg ...
>         reg.setProperties(...)
>  
> So, when the setProperties takes place, "Client" is re-bound twice with the service "Service".
> Indeed, in DependencyManager, when a ServiceEvent.MODIFIED event. is caught, the following code is invoked (line 170):
>             case ServiceEvent.MODIFIED:
>                 m_componentManager.log( LogService.LOG_DEBUG, "Dependency Manager: Updating {0}", new Object[]
>                     { serviceString }, null );
>                 // remove the service first
>                 // only continue with further event handling if the service
>                 // removal did not cause the component to be deactivated
>                 if ( serviceRemoved( ref ) )
>                 {
>                     // recalculate the number of services matching the filter
>                     // because we don't know whether this service previously matched
>                     // or not
>                     ServiceReference refs[] = getFrameworkServiceReferences();
>                     m_size = ( refs == null ) ? 0 : refs.length;
>                     // now try to bind the service - if it matches the target filter
>                     // without recalculating the size (already done).
>                     if ( targetFilterMatch( ref ) )
>                     {
>                         serviceAdded( ref );
>                     }
>                 }
>                 break;
> So, the service is first re-bound to the Client, when the serviceRemoved() method is invoked (it's a bound service replacement, I guess).
> But the problem here is that the modified service is also re-bound, when serviceAdded is invoked (line 189).
> Don't you think that this is a bug and the service should be re-bound only once, not twice ?
> Here is the first stacktrace of the first bind:
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.bind(DependencyManager.java:884)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceRemoved(DependencyManager.java:367)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:177)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)
> and here is the second stacktrace, when Service is re-bound:
> Client: bound Service : service.impl.ServiceImpl@142a80d
> java.lang.Exception: Stack trace
>         at java.lang.Thread.dumpStack(Thread.java:1158)
>         at client.Client.bind(Client.java:13)
>         at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>         at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
>         at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
>         at java.lang.reflect.Method.invoke(Method.java:592)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invokeMethod(BaseMethod.java:213)
>         at org.apache.felix.scr.impl.helper.BaseMethod.access$500(BaseMethod.java:38)
>         at org.apache.felix.scr.impl.helper.BaseMethod$Resolved.invoke(BaseMethod.java:542)
>         at org.apache.felix.scr.impl.helper.BaseMethod.invoke(BaseMethod.java:434)
>         at org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:948)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceAdded(DependencyManager.java:271)
>         at org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:189)
>         at org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:878)
>         at org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:732)
>         at org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:662)
>         at org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:3587)
>         at org.apache.felix.framework.Felix.access$000(Felix.java:40)
>         at org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:625)
>         at org.apache.felix.framework.ServiceRegistry.servicePropertiesModified(ServiceRegistry.java:505)
>         at org.apache.felix.framework.ServiceRegistrationImpl.setProperties(ServiceRegistrationImpl.java:116)
>         at service.impl.ServiceFactory.run(ServiceFactory.java:48)
>         at java.lang.Thread.run(Thread.java:595)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.