[ 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