United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-6236036 : Timeouts can cause garbage retention in lock classes

Details
Type:
Bug
Submit Date:
2005-03-04
Status:
Resolved
Updated Date:
2010-04-02
Project Name:
JDK
Resolved Date:
2005-09-04
Component:
core-libs
OS:
linux,generic
Sub-Component:
java.util.concurrent
CPU:
x86,generic
Priority:
P3
Resolution:
Fixed
Affected Versions:
5.0,6
Fixed Versions:

Related Reports
Backport:
Duplicate:
Duplicate:
Relates:
Relates:
Relates:
Relates:

Sub Tasks

Description
Doug Lea writes:

Classes built using AbstractQueuedSynchronizer and/or SynchronousQueue
can retain garbage nodes when consumers repeatedly time out and there
are never any producers/signals.

Test code: get from jsr166 CVS:
   src/test/jtreg/util/concurrent/BlockingQueue/PollMemoryLeak.java

(I don't know test harness syntax to ensure that it runs with
little enough memory to trip error in finite time. You probably
need to adjust this.)

Fix: Check for lack of signals on cancellation and clear queues.
See changes to AbstractQueuedSynchronizer and SynchronousQueue
(the new AbstractQueuedLongSynchronizer also conforms.)


###@###.### 2005-03-04 04:15:39 GMT

                                    

Comments
EVALUATION

Will be fixed as part of jsr166-2005 import
###@###.### 2005-03-04 04:15:39 GMT
                                     
2005-03-04
EVALUATION

This bug was actually fixed (inadvertently) 
in mustang b49 as part of the changes for

6237968: Add AbstractQueuedLongSynchronizer, providing support for 64 bits of synchronization state


+        /**
+         * Returns true if given node is on this condition queue.
+         * Call only when holding lock.
+         */
+        private boolean isOnConditionQueue(Node node) {
+            return node.next != null || node == lastWaiter;
+        }
+
+        /**
+         * Unlinks a cancelled waiter node from condition queue.  This
+         * is called when cancellation occurred during condition wait,
+         * not lock wait, and is called only after lock has been
+         * re-acquired by a cancelled waiter and the node is not known
+         * to already have been dequeued.  It is needed to avoid
+         * garbage retention in the absence of signals. So even though
+         * it may require a full traversal, it comes into play only
+         * when timeouts or cancellations occur in the absence of
+         * signals.
+         */
+        private void unlinkCancelledWaiter(Node node) {
+            Node t = firstWaiter;
+            Node trail = null;
+            while (t != null) {
+                if (t == node) {
+                    Node next = t.nextWaiter;
+                    if (trail == null)
+                        firstWaiter = next;
+                    else
+                        trail.nextWaiter = next;
+                    if (lastWaiter == node)
+                        lastWaiter = trail;
+                    break;
+                }
+                trail = t;
+                t = t.nextWaiter;
+            }
+        }
+
         // public methods
 
         /**
          * Moves the longest-waiting thread, if one exists, from the
          * wait queue for this condition to the wait queue for the
          * owning lock.
          * @throws IllegalMonitorStateException if {@link #isHeldExclusively}
@@ -1780,14 +1823,16 @@
             while (!isOnSyncQueue(node)) {
                 LockSupport.park(this);
                 if ((interruptMode = checkInterruptWhileWaiting(node)) != 0)
                     break;
             }
             if (acquireQueued(node, savedState) && interruptMode != THROW_IE)
                 interruptMode = REINTERRUPT;
+            if (isOnConditionQueue(node))
+                unlinkCancelledWaiter(node);
             if (interruptMode != 0)
                 reportInterruptAfterWait(interruptMode);
         }
 
         /**
          * Implements timed condition wait.
          * <ol>
@@ -1820,14 +1865,16 @@
 
                 long now = System.nanoTime();
                 nanosTimeout -= now - lastTime;
                 lastTime = now;
             }
             if (acquireQueued(node, savedState) && interruptMode != THROW_IE)
                 interruptMode = REINTERRUPT;
+            if (isOnConditionQueue(node))
+                unlinkCancelledWaiter(node);
             if (interruptMode != 0)
                 reportInterruptAfterWait(interruptMode);
             return nanosTimeout - (System.nanoTime() - lastTime);
         }
 
         /**
          * Implements absolute timed condition wait.
@@ -1861,14 +1908,16 @@
                 }
                 LockSupport.parkUntil(this, abstime);
                 if ((interruptMode = checkInterruptWhileWaiting(node)) != 0)
                     break;
             }
             if (acquireQueued(node, savedState) && interruptMode != THROW_IE)
                 interruptMode = REINTERRUPT;
+            if (isOnConditionQueue(node))
+                unlinkCancelledWaiter(node);
             if (interruptMode != 0)
                 reportInterruptAfterWait(interruptMode);
             return !timedout;
         }
 
         /**
          * Implements timed condition wait.
@@ -1906,14 +1955,16 @@
                     break;
                 long now = System.nanoTime();
                 nanosTimeout -= now - lastTime;
                 lastTime = now;
             }
             if (acquireQueued(node, savedState) && interruptMode != THROW_IE)
                 interruptMode = REINTERRUPT;
+            if (isOnConditionQueue(node))
+                unlinkCancelledWaiter(node);
             if (interruptMode != 0)
                 reportInterruptAfterWait(interruptMode);
             return !timedout;
         }
 
         //  support for instrumentation
                                     
2006-08-04
EVALUATION

This fix was correct, but incomplete, and needs to be backported to jdk 5.
That work is being done under

6460501: Synchronizer timed acquire still leaks memory
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6460501
                                     
2006-12-05



Hardware and Software, Engineered to Work Together