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

Richard S. Hall commented on FELIX-3687:
----------------------------------------

Technically, I believe this patch leaves a window open where a client could 
re-obtain the service before it was invalidated. Notice the comment in the 
block above this block, where it says we don't hold the registration lock 
because we want existing service references to still be able to look it up 
while unregistering.

This means we don't have to worry about new clients getting the service 
references to the unregistering service, but clients that already have a 
service reference can still look it up. (You could argue that this isn't 
sufficient and we really should allow new people to look it up too, but that's 
a separate issue.)

Given that, since we no longer hold the lock, even though we force existing 
clients to release, they could grab it again once we move onto forcing the next 
client to release. Perhaps we need need to lock the service registration (by 
putting it into the m_lockedRegsMap) when we take the client snapshot too.

This would allow existing ref holders to get the service while getting the 
UNREGISTERING event, but not once we start to force releasing.
                
> Deadlock potential from ServiceRegistry.unregisterService
> ---------------------------------------------------------
>
>                 Key: FELIX-3687
>                 URL: https://issues.apache.org/jira/browse/FELIX-3687
>             Project: Felix
>          Issue Type: Bug
>          Components: Framework
>    Affects Versions: framework-4.0.3
>            Reporter: David Jencks
>         Attachments: FELIX-3687-1.diff
>
>
> At the end of unregisterService there's this block:
> {code}
>         // Now forcibly unget the service object for all stubborn clients.
>         synchronized (this)
>         {
>             Bundle[] clients = getUsingBundles(reg.getReference());
>             for (int i = 0; (clients != null) && (i < clients.length); i++)
>             {
>                 while (ungetService(clients[i], reg.getReference()))
>                     ; // Keep removing until it is no longer possible
>             }
>             ((ServiceRegistrationImpl) reg).invalidate();
>         }
> {code}
> Note the call to ungetService from within a synchronized block.  ungetService 
> itself is very careful to release the lock before calling out to the service 
> factory, however this call from unregisterService negates this care.
> This causes problems with DS with thread dumps like:
> {code}
> ThreadId: 16 : name: SCR Component Actor State: RUNNABLE
>   LockInfo: null LockOwnerId: -1 LockOwnerName: null
>   sun.management.ThreadImpl.dumpThreads0(Native Method)
>   sun.management.ThreadImpl.dumpAllThreads(ThreadImpl.java:433)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager.dumpThreads(AbstractComponentManager.java:294)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager.logLockingInfo(AbstractComponentManager.java:240)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager.releaseReadLock(AbstractComponentManager.java:222)
>   
> org.apache.felix.scr.impl.manager.ImmediateComponentManager.ungetService(ImmediateComponentManager.java:710)
>   
> org.apache.felix.framework.ServiceRegistrationImpl.ungetFactoryUnchecked(ServiceRegistrationImpl.java:349)
>   
> org.apache.felix.framework.ServiceRegistrationImpl.ungetService(ServiceRegistrationImpl.java:258)
>   
> org.apache.felix.framework.ServiceRegistry.ungetService(ServiceRegistry.java:389)
>   org.apache.felix.framework.Felix.ungetService(Felix.java:3432)
>   
> org.apache.felix.framework.BundleContextImpl.ungetService(BundleContextImpl.java:486)
>   
> org.apache.felix.scr.impl.manager.DependencyManager.ungetService(DependencyManager.java:900)
>   
> org.apache.felix.scr.impl.manager.DependencyManager.unbind(DependencyManager.java:1138)
>   
> org.apache.felix.scr.impl.manager.DependencyManager.close(DependencyManager.java:970)
>   
> org.apache.felix.scr.impl.manager.ImmediateComponentManager.disposeImplementationObject(ImmediateComponentManager.java:276)
>   
> org.apache.felix.scr.impl.manager.ImmediateComponentManager.deleteComponent(ImmediateComponentManager.java:148)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager$Active.ungetService(AbstractComponentManager.java:1718)
>   
> org.apache.felix.scr.impl.manager.ImmediateComponentManager.ungetService(ImmediateComponentManager.java:695)
>   
> org.apache.felix.framework.ServiceRegistrationImpl.ungetFactoryUnchecked(ServiceRegistrationImpl.java:349)
>   
> org.apache.felix.framework.ServiceRegistrationImpl.ungetService(ServiceRegistrationImpl.java:258)
>   
> org.apache.felix.framework.ServiceRegistry.ungetService(ServiceRegistry.java:389)
> >>>>>>>>inside the lock  
> >>>>>>>>org.apache.felix.framework.ServiceRegistry.unregisterService(ServiceRegistry.java:158)
>   
> org.apache.felix.framework.ServiceRegistrationImpl.unregister(ServiceRegistrationImpl.java:127)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager.unregisterComponentService(AbstractComponentManager.java:779)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager$State.doDeactivate(AbstractComponentManager.java:1387)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager$Satisfied.deactivate(AbstractComponentManager.java:1665)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager.deactivateInternal(AbstractComponentManager.java:635)
>   
> org.apache.felix.scr.impl.manager.DependencyManager.serviceRemoved(DependencyManager.java:375)
>   
> org.apache.felix.scr.impl.manager.DependencyManager.serviceChanged(DependencyManager.java:217)
>   
> org.apache.felix.framework.util.EventDispatcher.invokeServiceListenerCallback(EventDispatcher.java:932)
>   
> org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:793)
>   
> org.apache.felix.framework.util.EventDispatcher.fireServiceEvent(EventDispatcher.java:543)
>   org.apache.felix.framework.Felix.fireServiceEvent(Felix.java:4260)
>   org.apache.felix.framework.Felix.access$000(Felix.java:74)
>   org.apache.felix.framework.Felix$1.serviceChanged(Felix.java:390)
>   
> org.apache.felix.framework.ServiceRegistry.unregisterService(ServiceRegistry.java:148)
>   
> org.apache.felix.framework.ServiceRegistrationImpl.unregister(ServiceRegistrationImpl.java:127)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager.unregisterComponentService(AbstractComponentManager.java:779)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager$State.doDeactivate(AbstractComponentManager.java:1387)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager$Disabled.deactivate(AbstractComponentManager.java:1461)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager.deactivateInternal(AbstractComponentManager.java:635)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager$2.run(AbstractComponentManager.java:435)
>   
> org.apache.felix.scr.impl.ComponentActorThread.run(ComponentActorThread.java:98)
>   java.lang.Thread.run(Thread.java:680)
> ThreadId: 67 : name: pool-1-thread-49 State: BLOCKED
>   LockInfo: org.apache.felix.framework.ServiceRegistry@26d66426 LockOwnerId: 
> 16 LockOwnerName: SCR Component Actor
>   
> org.apache.felix.framework.ServiceRegistry.getServiceReferences(ServiceRegistry.java:204)
>   org.apache.felix.framework.Felix.getServiceReferences(Felix.java:3310)
>   
> org.apache.felix.framework.Felix.getAllowedServiceReferences(Felix.java:3383)
>   
> org.apache.felix.framework.BundleContextImpl.getServiceReferences(BundleContextImpl.java:432)
>   
> org.apache.felix.scr.impl.manager.DependencyManager.getFrameworkServiceReferences(DependencyManager.java:658)
>   
> org.apache.felix.scr.impl.manager.DependencyManager.getFrameworkServiceReferences(DependencyManager.java:634)
>   
> org.apache.felix.scr.impl.manager.DependencyManager.enable(DependencyManager.java:551)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager.enableDependencyManagers(AbstractComponentManager.java:1061)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager.access$600(AbstractComponentManager.java:63)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager$Disabled.enable(AbstractComponentManager.java:1445)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager.enableInternal(AbstractComponentManager.java:625)
>   
> org.apache.felix.scr.impl.manager.AbstractComponentManager.enable(AbstractComponentManager.java:358)
>   
> org.apache.felix.scr.impl.config.ImmediateComponentHolder.enableComponents(ImmediateComponentHolder.java:384)
>   
> org.apache.felix.scr.impl.BundleComponentActivator.enableComponent(BundleComponentActivator.java:395)
>   
> org.apache.felix.scr.impl.manager.ComponentContextImpl.enableComponent(ComponentContextImpl.java:101)
>   test.scr.Main$EnableManager$1.run(Main.java:56)
>   
> java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
>   
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
>   java.lang.Thread.run(Thread.java:680)
> {code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to