cxf-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Amichai Rothman (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (DOSGI-163) Deadlock when shutting down
Date Wed, 03 Apr 2013 15:23:16 GMT

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

Amichai Rothman updated DOSGI-163:
----------------------------------

    Attachment: fix_importregistrationimpl_2.diff
                fix_importregistrationimpl_1.diff

Ok, here is an updated patch which should address both deadlocks. For the reviewer's convenience,
it is split into two patch files - the first is a bunch of cleanup, refactoring and documenting
which should make the class more clear, concise and straightforward (and hopefully less error-prone),
and the second is the actual synchronization fix on top of the first.

The first patch doesn't include any functional changes (except for log texts), but only makes
future changes easier and clearer (hopefully). It consists of the following changes:

- deleted ImportReferenceImpl, which was a simple wrapper delegating everything back to ImportRegistrationImpl,
and made the latter implement ImportReference directly instead (which it basically already
did)
- added clarifying comments and javadocs
- renamed init() to initParent(), childs to children, and detatched to detached for clarity
and correctness
- reordered methods to be better grouped by their relationships
- unified checks for isFailure and closed into a single isInvalid() check
- removed redundant check of closed before calling close() (which already checks that)
- replaced cumbersome methods with simple returned conditionals
- removed unused methods
- removed accessors which were used only in one place (internally)
- made children use parent's importedService reference, instead of always setting the same
reference on all children

The second patch is the one that addresses the deadlock issue at hand, by reworking the synchronized
methods into finer synchronized blocks, such that they guard only the internal state of the
class from inconsistencies, but make sure that no locks are held when invoking external methods
(most of which have their own locks - the previous double-locking is how the deadlocks came
to be in the first place).

Comments, suggestions and corrections are most welcome (and committing the patches to trunk
too, if all is well! ;-) )

                
> Deadlock when shutting down
> ---------------------------
>
>                 Key: DOSGI-163
>                 URL: https://issues.apache.org/jira/browse/DOSGI-163
>             Project: CXF Distributed OSGi
>          Issue Type: Bug
>          Components: DSW
>    Affects Versions: 1.4.0
>         Environment: Oracle JDK 1.7.0_17, Karaf 2.3.1, DOSGi 1.4.0
>            Reporter: Amichai Rothman
>         Attachments: fix_dosgi_unregister_deadlock.diff, fix_importregistrationimpl_1.diff,
fix_importregistrationimpl_2.diff
>
>
> Karaf sometimes hangs while shutting down, and a thread dump shows a deadlock in DOSGi.
In general, the OSGi specs recommend not to hold any locks when calling the APIs since it
can result in a deadlock, as is the case here.
> A proposed fix calls ServiceRegistration.unregister outside of the synchronized block
in ImportRegistrationImpl.close (note that it still is synchronized when called from closeAll,
which may pose a related but separate problem - but the fix does fix this specific deadlock
as it was encountered here).
> Found one Java-level deadlock:
> =============================
> "pool-18-thread-3":
>   waiting to lock monitor 0x00007f0658003f40 (object 0x00000000e5b485b0, a org.eclipse.osgi.internal.serviceregistry.ServiceUse),
>   which is held by "Blueprint Extender: 2"
> "Blueprint Extender: 2":
>   waiting to lock monitor 0x00007f0654002a50 (object 0x00000000f1b70240, a org.apache.cxf.dosgi.dsw.service.ImportRegistrationImpl),
>   which is held by "pool-18-thread-3"
> Java stack information for the threads listed above:
> ===================================================
> "pool-18-thread-3":
>         at org.eclipse.osgi.internal.serviceregistry.ServiceRegistrationImpl.releaseService(ServiceRegistrationImpl.java:562)
>         - waiting to lock <0x00000000e5b485b0> (a org.eclipse.osgi.internal.serviceregistry.ServiceUse)
>         at org.eclipse.osgi.internal.serviceregistry.ServiceRegistrationImpl.unregister(ServiceRegistrationImpl.java:245)
>         at org.apache.cxf.dosgi.dsw.service.ImportRegistrationImpl.instanceClosed(ImportRegistrationImpl.java:118)
>         - locked <0x00000000f1b70240> (a org.apache.cxf.dosgi.dsw.service.ImportRegistrationImpl)
>         at org.apache.cxf.dosgi.dsw.service.ImportRegistrationImpl.close(ImportRegistrationImpl.java:100)
>         - locked <0x00000000f1b70240> (a org.apache.cxf.dosgi.dsw.service.ImportRegistrationImpl)
>         at org.apache.cxf.dosgi.topologymanager.importer.TopologyManagerImport.unexportNotAvailableServices(TopologyManagerImport.java:227)
>         at org.apache.cxf.dosgi.topologymanager.importer.TopologyManagerImport.access$200(TopologyManagerImport.java:53)
>         at org.apache.cxf.dosgi.topologymanager.importer.TopologyManagerImport$2.run(TopologyManagerImport.java:201)
>         - locked <0x00000000ea71b6c0> (a java.util.HashMap)
>         - locked <0x00000000ea71b6f8> (a java.util.HashMap)
>         at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
>         at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
>         at java.lang.Thread.run(Thread.java:722)
> "Blueprint Extender: 2":
>         at org.apache.cxf.dosgi.dsw.service.ImportRegistrationImpl.closeAll(ImportRegistrationImpl.java:127)
>         - waiting to lock <0x00000000f1b70240> (a org.apache.cxf.dosgi.dsw.service.ImportRegistrationImpl)
>         at org.apache.cxf.dosgi.dsw.service.ClientServiceFactory.remove(ClientServiceFactory.java:110)
>         at org.apache.cxf.dosgi.dsw.service.ClientServiceFactory.ungetService(ClientServiceFactory.java:104)
>         - locked <0x00000000f1b6f7e0> (a org.apache.cxf.dosgi.dsw.service.ClientServiceFactory)
>         at org.eclipse.osgi.internal.serviceregistry.ServiceUse$2.run(ServiceUse.java:238)
>         at java.security.AccessController.doPrivileged(Native Method)
>         at org.eclipse.osgi.internal.serviceregistry.ServiceUse.ungetService(ServiceUse.java:236)
>         at org.eclipse.osgi.internal.serviceregistry.ServiceRegistrationImpl.ungetService(ServiceRegistrationImpl.java:518)
>         - locked <0x00000000e5b485b0> (a org.eclipse.osgi.internal.serviceregistry.ServiceUse)
>         at org.eclipse.osgi.internal.serviceregistry.ServiceRegistry.ungetService(ServiceRegistry.java:510)
>         at org.eclipse.osgi.framework.internal.core.BundleContextImpl.ungetService(BundleContextImpl.java:636)
>         at org.apache.aries.blueprint.container.ReferenceListRecipe$ServiceDispatcher.destroy(ReferenceListRecipe.java:197)
>         - locked <0x00000000e5bab728> (a org.apache.aries.blueprint.container.ReferenceListRecipe$ServiceDispatcher)
>         at org.apache.aries.blueprint.container.ReferenceListRecipe.untrack(ReferenceListRecipe.java:147)
>         - locked <0x00000000eaef82d0> (a java.lang.Object)
>         at org.apache.aries.blueprint.container.AbstractServiceReferenceRecipe.serviceRemoved(AbstractServiceReferenceRecipe.java:370)
>         at org.apache.aries.blueprint.container.AbstractServiceReferenceRecipe.access$200(AbstractServiceReferenceRecipe.java:72)
>         at org.apache.aries.blueprint.container.AbstractServiceReferenceRecipe$3.run(AbstractServiceReferenceRecipe.java:324)
>         at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
>         at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
>         at java.util.concurrent.FutureTask.run(FutureTask.java:166)
>         at org.apache.aries.blueprint.container.ExecutorServiceWrapper.run(ExecutorServiceWrapper.java:106)
>         at org.apache.aries.blueprint.utils.threading.impl.DiscardableRunnable.run(DiscardableRunnable.java:48)
>         at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
>         at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
>         at java.util.concurrent.FutureTask.run(FutureTask.java:166)
>         at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:178)
>         at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:292)
>         at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
>         at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
>         at java.lang.Thread.run(Thread.java:722)
> Found 1 deadlock.

--
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

Mime
View raw message