[
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.servicei...@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.
