You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@felix.apache.org by Guillaume Nodet <gn...@gmail.com> on 2012/03/14 10:48:42 UTC

Possible deadlock fix

I'd like to discuss a possible deadlock fix.
This happen when a thread which has a bundle lock has some reentrant call
while the global lock is held by another thread (see stack trace below)

In the code Felix#acquireBundleLock, the comment on the loop says "Wait if
the desired bundle is already locked by someone else or if any thread has
the global lock, unless the current thread holds the global lock or the
bundle lock already." which makes total sense, but I don't think the code
handle the case where the lock is already held by the current thread *and*
the global lock is held by a different thread.

Possible patch below.


diff --git a/framework/src/main/java/org/apache/felix/framework/Felix.java
b/framework/src/main/java/org/apache/felix/framework/Felix.java
index 6504f13..0965d8b 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -5002,7 +5002,8 @@ public class Felix extends BundleImpl implements
Framework
             // holds the global lock or the bundle lock already.
             while (!bundle.isLockable() ||
                 ((m_globalLockThread != null)
-                    && (m_globalLockThread != Thread.currentThread())))
+                    && (m_globalLockThread != Thread.currentThread())
+                    && (bundle.getLockingThread() !=
Thread.currentThread())))
             {
                 // Check to make sure the bundle is in a desired state.
                 // If so, keep waiting. If not, throw an exception.



"FelixStartLevel" daemon prio=5 tid=7fb1b2ac7800 nid=0x111144000 in
Object.wait() [111142000]
   java.lang.Thread.State: WAITING (on object monitor)
at java.lang.Object.wait(Native Method)
- waiting on <7e039bc90> (a [Ljava.lang.Object;)
at java.lang.Object.wait(Object.java:485)
at org.apache.felix.framework.Felix.acquireBundleLock(Felix.java:5025)
- locked <7e039bc90> (a [Ljava.lang.Object;)
at org.apache.felix.framework.Felix.registerService(Felix.java:3282)
at
org.apache.felix.framework.BundleContextImpl.registerService(BundleContextImpl.java:346)
at
org.apache.aries.blueprint.container.BlueprintContainerImpl.registerService(BlueprintContainerImpl.java:408)
at
org.apache.aries.blueprint.container.ServiceRecipe.register(ServiceRecipe.java:184)
at
org.apache.aries.blueprint.container.BlueprintContainerImpl.registerServices(BlueprintContainerImpl.java:666)
at
org.apache.aries.blueprint.container.BlueprintContainerImpl.doRun(BlueprintContainerImpl.java:334)
at
org.apache.aries.blueprint.container.BlueprintContainerImpl.run(BlueprintContainerImpl.java:230)
- locked <7f5898750> (a java.util.concurrent.atomic.AtomicBoolean)
- locked <7f5898740> (a java.util.concurrent.atomic.AtomicBoolean)
at
org.apache.aries.blueprint.container.BlueprintExtender.checkBundle(BlueprintExtender.java:325)
at
org.apache.aries.blueprint.container.BlueprintExtender.bundleChanged(BlueprintExtender.java:244)
at
org.apache.aries.blueprint.container.BlueprintExtender$BlueprintBundleTrackerCustomizer.modifiedBundle(BlueprintExtender.java:471)
at
org.osgi.util.tracker.BundleTracker$Tracked.customizerModified(BundleTracker.java:495)
at
org.osgi.util.tracker.BundleTracker$Tracked.customizerModified(BundleTracker.java:1)
at org.osgi.util.tracker.AbstractTracked.track(AbstractTracked.java:238)
at
org.osgi.util.tracker.BundleTracker$Tracked.bundleChanged(BundleTracker.java:457)
at
org.apache.felix.framework.util.EventDispatcher.invokeBundleListenerCallback(EventDispatcher.java:870)
at
org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:791)
at
org.apache.felix.framework.util.EventDispatcher.fireBundleEvent(EventDispatcher.java:515)
at org.apache.felix.framework.Felix.fireBundleEvent(Felix.java:4321)
at org.apache.felix.framework.Felix.startBundle(Felix.java:1945)
at org.apache.felix.framework.Felix.setActiveStartLevel(Felix.java:1213)
at
org.apache.felix.framework.FrameworkStartLevelImpl.run(FrameworkStartLevelImpl.java:295)
at java.lang.Thread.run(Thread.java:680)

-- 
------------------------
Guillaume Nodet
------------------------
Blog: http://gnodet.blogspot.com/
------------------------
FuseSource, Integration everywhere
http://fusesource.com

Re: Possible deadlock fix

Posted by "Richard S. Hall" <he...@ungoverned.org>.
Yes, I think you are correct.

I'm guessing that the logical mistake was thinking that 
!bundle.isLockable() was avoiding the loop since it checks for the 
current thread (and who knows it may have at one time), but in the 
current loop condition below since if the current thread holds the 
bundle lock the "!" makes it false, then it must evaluate the other side 
of the OR, which as you point out doesn't correctly check for the 
current thread holding the bundle lock already.

Go ahead and apply your patch, but please open an issue for it first. 
Thanks.

-> richard

On 3/14/12 05:48 , Guillaume Nodet wrote:
> I'd like to discuss a possible deadlock fix.
> This happen when a thread which has a bundle lock has some reentrant call
> while the global lock is held by another thread (see stack trace below)
>
> In the code Felix#acquireBundleLock, the comment on the loop says "Wait if
> the desired bundle is already locked by someone else or if any thread has
> the global lock, unless the current thread holds the global lock or the
> bundle lock already." which makes total sense, but I don't think the code
> handle the case where the lock is already held by the current thread *and*
> the global lock is held by a different thread.
>
> Possible patch below.
>
>
> diff --git a/framework/src/main/java/org/apache/felix/framework/Felix.java
> b/framework/src/main/java/org/apache/felix/framework/Felix.java
> index 6504f13..0965d8b 100644
> --- a/framework/src/main/java/org/apache/felix/framework/Felix.java
> +++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
> @@ -5002,7 +5002,8 @@ public class Felix extends BundleImpl implements
> Framework
>               // holds the global lock or the bundle lock already.
>               while (!bundle.isLockable() ||
>                   ((m_globalLockThread != null)
> -&&  (m_globalLockThread != Thread.currentThread())))
> +&&  (m_globalLockThread != Thread.currentThread())
> +&&  (bundle.getLockingThread() !=
> Thread.currentThread())))
>               {
>                   // Check to make sure the bundle is in a desired state.
>                   // If so, keep waiting. If not, throw an exception.
>
>
>
> "FelixStartLevel" daemon prio=5 tid=7fb1b2ac7800 nid=0x111144000 in
> Object.wait() [111142000]
>     java.lang.Thread.State: WAITING (on object monitor)
> at java.lang.Object.wait(Native Method)
> - waiting on<7e039bc90>  (a [Ljava.lang.Object;)
> at java.lang.Object.wait(Object.java:485)
> at org.apache.felix.framework.Felix.acquireBundleLock(Felix.java:5025)
> - locked<7e039bc90>  (a [Ljava.lang.Object;)
> at org.apache.felix.framework.Felix.registerService(Felix.java:3282)
> at
> org.apache.felix.framework.BundleContextImpl.registerService(BundleContextImpl.java:346)
> at
> org.apache.aries.blueprint.container.BlueprintContainerImpl.registerService(BlueprintContainerImpl.java:408)
> at
> org.apache.aries.blueprint.container.ServiceRecipe.register(ServiceRecipe.java:184)
> at
> org.apache.aries.blueprint.container.BlueprintContainerImpl.registerServices(BlueprintContainerImpl.java:666)
> at
> org.apache.aries.blueprint.container.BlueprintContainerImpl.doRun(BlueprintContainerImpl.java:334)
> at
> org.apache.aries.blueprint.container.BlueprintContainerImpl.run(BlueprintContainerImpl.java:230)
> - locked<7f5898750>  (a java.util.concurrent.atomic.AtomicBoolean)
> - locked<7f5898740>  (a java.util.concurrent.atomic.AtomicBoolean)
> at
> org.apache.aries.blueprint.container.BlueprintExtender.checkBundle(BlueprintExtender.java:325)
> at
> org.apache.aries.blueprint.container.BlueprintExtender.bundleChanged(BlueprintExtender.java:244)
> at
> org.apache.aries.blueprint.container.BlueprintExtender$BlueprintBundleTrackerCustomizer.modifiedBundle(BlueprintExtender.java:471)
> at
> org.osgi.util.tracker.BundleTracker$Tracked.customizerModified(BundleTracker.java:495)
> at
> org.osgi.util.tracker.BundleTracker$Tracked.customizerModified(BundleTracker.java:1)
> at org.osgi.util.tracker.AbstractTracked.track(AbstractTracked.java:238)
> at
> org.osgi.util.tracker.BundleTracker$Tracked.bundleChanged(BundleTracker.java:457)
> at
> org.apache.felix.framework.util.EventDispatcher.invokeBundleListenerCallback(EventDispatcher.java:870)
> at
> org.apache.felix.framework.util.EventDispatcher.fireEventImmediately(EventDispatcher.java:791)
> at
> org.apache.felix.framework.util.EventDispatcher.fireBundleEvent(EventDispatcher.java:515)
> at org.apache.felix.framework.Felix.fireBundleEvent(Felix.java:4321)
> at org.apache.felix.framework.Felix.startBundle(Felix.java:1945)
> at org.apache.felix.framework.Felix.setActiveStartLevel(Felix.java:1213)
> at
> org.apache.felix.framework.FrameworkStartLevelImpl.run(FrameworkStartLevelImpl.java:295)
> at java.lang.Thread.run(Thread.java:680)
>