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
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 6
6 b51Fixed
Related Reports
Duplicate :  
Duplicate :  
Relates :  
Relates :  
Relates :  
Relates :  
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 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

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 Will be fixed as part of jsr166-2005 import ###@###.### 2005-03-04 04:15:39 GMT
2005-03-04