Bug ID: JDK-6236036 Timeouts can cause garbage retention in lock classes

Type: Bug
Component: core-libs
Sub-Component: java.util.concurrent
Affected Version: 5.0,6

Priority: P3
Status: Resolved
Resolution: Fixed
OS: generic,linux
CPU: generic,x86

Submitted: 2005-03-04
Updated: 2010-04-02
Resolved: 2005-09-04

JDK 6
6 b51Fixed

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

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

05-12-2006

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

04-08-2006

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

04-03-2005

Duplicate :	JDK-6457125 - ArrayBlockingQueue.poll(long, TimeUnit) memory leak
Duplicate :	JDK-6457123 - timeouts cause garbage retention in java.util.concurrent lock classes
Relates :	JDK-6460501 - Synchronizer timed acquire still leaks memory
Relates :	JDK-6491621 - Documentation for timeouts cause garbage retention in java.util.concurrent lock
Relates :	JDK-6237968 - Add AbstractQueuedLongSynchronizer, providing support for 64 bits of synchronization state
Relates :	JDK-6500694 - Need to backport fix for 6236036 to Java 5 from Java 6