[concurrency-interest] FW: Cant detect deadlock when thread reenters synchronization block in Object.wait()

Dmytro Sheyko dmytro_sheyko at hotmail.com
Thu Jun 4 04:44:37 EDT 2009


Hi all,

There is a bug related to deadlock detection:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6841139

Fixing this issue requires changes in hotspot (it regards pure monitors only). However I believe that for consistency java.util.concurrent classes need to be modified as well. So I created separate bug report for java part as David Holmes suggested. The general idea is to adjust AbstractOwnableSynchronizer so that it would be able to detect deadlocks even if threads wait on condition.
Details are here:
https://bugs.openjdk.java.net/show_bug.cgi?id=100060

proposed patch (java part only):
diff -r 045743e0eb2d src/share/classes/java/util/concurrent/locks/AbstractOwnableSynchronizer.java
--- a/src/share/classes/java/util/concurrent/locks/AbstractOwnableSynchronizer.java    Thu Jun 04 11:28:03 2009 +0800
+++ b/src/share/classes/java/util/concurrent/locks/AbstractOwnableSynchronizer.java    Thu Jun 04 10:54:58 2009 +0300
@@ -54,9 +54,23 @@
     private static final long serialVersionUID = 3737899427754241961L;
 
     /**
-     * Empty constructor for use by subclasses.
+     * Empty constructor for use by subclasses,
+     * creates <tt>main</tt> ownable synchronizer.
      */
-    protected AbstractOwnableSynchronizer() { }
+    protected AbstractOwnableSynchronizer() {
+        this(null);
+    }
+
+    /**
+     * Facility to create <tt>side</tt> ownable synchronizer, which shares
+     * information about exclusive owner thread with specified parent.
+     *
+     * @param parent - ownable synchronizer, with which exclusive owner thread will be shared,
+     * or <tt>null</tt> to create <tt>main</tt> ownable synchronizer.
+     */
+    protected AbstractOwnableSynchronizer(AbstractOwnableSynchronizer parent) {
+        mainOwnableSynchronizer = (parent == null) ? this : parent.mainOwnableSynchronizer;
+    }
 
     /**
      * The current owner of exclusive mode synchronization.
@@ -64,13 +78,18 @@
     private transient Thread exclusiveOwnerThread;
 
     /**
+     * The <tt>main</tt> ownable synchronizer, which actually maintains owner thread.
+     */
+    private final AbstractOwnableSynchronizer mainOwnableSynchronizer;
+
+    /**
      * Sets the thread that currently owns exclusive access. A
      * <tt>null</tt> argument indicates that no thread owns access.
      * This method does not otherwise impose any synchronization or
      * <tt>volatile</tt> field accesses.
      */
     protected final void setExclusiveOwnerThread(Thread t) {
-        exclusiveOwnerThread = t;
+        mainOwnableSynchronizer.exclusiveOwnerThread = t;
     }
 
     /**
@@ -81,6 +100,6 @@
      * @return the owner thread
      */
     protected final Thread getExclusiveOwnerThread() {
-        return exclusiveOwnerThread;
+        return mainOwnableSynchronizer.exclusiveOwnerThread;
     }
 }
diff -r 045743e0eb2d src/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java
--- a/src/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java    Thu Jun 04 11:28:03 2009 +0800
+++ b/src/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java    Thu Jun 04 10:54:58 2009 +0300
@@ -1452,6 +1452,7 @@
          * attempt to set waitStatus fails, wake up to resync (in which
          * case the waitStatus can be transiently and harmlessly wrong).
          */
+        LockSupport.setBlocker(node.thread, this);
         Node p = enq(node);
         int ws = p.waitStatus;
         if (ws > 0 || !compareAndSetWaitStatus(p, ws, Node.SIGNAL))
@@ -1604,7 +1605,7 @@
      *
      * @since 1.6
      */
-    public class ConditionObject implements Condition, java.io.Serializable {
+    public class ConditionObject extends AbstractOwnableSynchronizer implements Condition, java.io.Serializable {
         private static final long serialVersionUID = 1173984872572414699L;
         /** First node of condition queue. */
         private transient Node firstWaiter;
@@ -1614,7 +1615,9 @@
         /**
          * Creates a new <tt>ConditionObject</tt> instance.
          */
-        public ConditionObject() { }
+        public ConditionObject() {
+            super(AbstractQueuedLongSynchronizer.this);
+        }
 
         // Internal methods
 
diff -r 045743e0eb2d src/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java
--- a/src/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java    Thu Jun 04 11:28:03 2009 +0800
+++ b/src/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java    Thu Jun 04 10:54:58 2009 +0300
@@ -1675,6 +1675,7 @@
          * attempt to set waitStatus fails, wake up to resync (in which
          * case the waitStatus can be transiently and harmlessly wrong).
          */
+        LockSupport.setBlocker(node.thread, this);
         Node p = enq(node);
         int ws = p.waitStatus;
         if (ws > 0 || !compareAndSetWaitStatus(p, ws, Node.SIGNAL))
@@ -1825,7 +1826,7 @@
      * <p>This class is Serializable, but all fields are transient,
      * so deserialized conditions have no waiters.
      */
-    public class ConditionObject implements Condition, java.io.Serializable {
+    public class ConditionObject extends AbstractOwnableSynchronizer implements Condition, java.io.Serializable {
         private static final long serialVersionUID = 1173984872572414699L;
         /** First node of condition queue. */
         private transient Node firstWaiter;
@@ -1835,7 +1836,9 @@
         /**
          * Creates a new <tt>ConditionObject</tt> instance.
          */
-        public ConditionObject() { }
+        public ConditionObject() {
+            super(AbstractQueuedSynchronizer.this);
+        }
 
         // Internal methods
 
diff -r 045743e0eb2d src/share/classes/java/util/concurrent/locks/LockSupport.java
--- a/src/share/classes/java/util/concurrent/locks/LockSupport.java    Thu Jun 04 11:28:03 2009 +0800
+++ b/src/share/classes/java/util/concurrent/locks/LockSupport.java    Thu Jun 04 10:54:58 2009 +0300
@@ -131,7 +131,7 @@
         } catch (Exception ex) { throw new Error(ex); }
     }
 
-    private static void setBlocker(Thread t, Object arg) {
+    static void setBlocker(Thread t, Object arg) {
         // Even though volatile, hotspot doesn't need a write barrier here.
         unsafe.putObject(t, parkBlockerOffset, arg);
     }


> Date: Thu, 4 Jun 2009 01:04:23 +1000
> From: David.Holmes at Sun.COM
> Subject: Re: Cant detect deadlock when thread reenters synchronization block in	Object.wait()
> To: dmytro_sheyko at hotmail.com
> CC: serviceability-dev at openjdk.java.net; hotspot-dev at openjdk.java.net; core-libs-dev at openjdk.java.net
> 
> Hi Dmytro,
> 
> Can you split this into two separate bug reports please. The changes to the 
> VM are quite separate from the changes to the java.util.concurrent classes. 
> The j.u.c changes should then be discussed on the concurrent-interest 
> mailing list:
> 
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
> 
> This will let all the JSR-166 EG members, and the genercal c-i community 
> evaluate what you have done.
> 
> At this stage I have not evaluated the actual proposals.
> 
> Thanks,
> David Holmes
> (j.u.c bug evaluator)
> 
> Dmytro Sheyko wrote:
> > Hi,
> > 
> > Could you have a look at following patch regarding deadlock detection:
> > 
> > Cant detect deadlock when thread reenters synchronization block in 
> > Object.wait()
> > https://bugs.openjdk.java.net/show_bug.cgi?id=100058
> > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6841139
> > 
> > Thank you,
> > Dmytro Sheyko
> > 
> > ------------------------------------------------------------------------
> > See all the ways you can stay connected to friends and family 
> > <http://www.microsoft.com/windows/windowslive/default.aspx>

_________________________________________________________________
Invite your mail contacts to join your friends list with Windows Live Spaces. It's easy!
http://spaces.live.com/spacesapi.aspx?wx_action=create&wx_url=/friends.aspx&mkt=en-us
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20090604/fc0f1a8e/attachment.html>


More information about the Concurrency-interest mailing list