JDK-8137184 : Memory leak from ConcurrentLinkedQueue#remove(Object)
  • Type: Bug
  • Component: core-libs
  • Sub-Component: java.util
  • Affected Version: 8u60
  • Priority: P2
  • Status: Closed
  • Resolution: Duplicate
  • OS: linux
  • CPU: x86_64
  • Submitted: 2015-09-21
  • Updated: 2015-09-26
  • Resolved: 2015-09-26
Related Reports
Duplicate :  
Duplicate :  
Description
FULL PRODUCT VERSION :
java version "1.8.0_51"
Java(TM) SE Runtime Environment (build 1.8.0_51-b16)
Java HotSpot(TM) 64-Bit Server VM (build 25.51-b03, mixed mode)


ADDITIONAL OS VERSION INFORMATION :
Linux Tile440 3.19.0-28-generic #30-Ubuntu SMP Mon Aug 31 15:52:51 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux


A DESCRIPTION OF THE PROBLEM :
when removing the last object from a ConcurrentLinkedQueue, the item is nulled but the holder is never unlinked from the list.

Thus the attached program results in a forever increasing list of null item entries.  This is slow and a memory leak.

This was found and reported by Max Urech in https://bugs.eclipse.org/bugs/show_bug.cgi?id=477817

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
If you run the attached test case you will see each iteration takes longer and longer. Inspection with a debugger reveals the link list growing with null entries. 

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
The node should have both its item nulled and be removed from the list by having the pred node's next pointer nulled.  The list should not grow forever.
ACTUAL -
The remove code both nulls the item value and then attempts to delink the pred next node pointer.  However, if the current node next pointer is null (ie the last item), then the pred node next is not nulled. Thus the list grows forever!

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
import java.util.concurrent.ConcurrentLinkedQueue;

public class TestLeak 
{
    public static void main(String[] args) 
    {
        ConcurrentLinkedQueue<Object> queue=new ConcurrentLinkedQueue<>();
        queue.add(new Object()); //Required for the leak to appear.
        Object object=new Object();
        int loops=0;
        Runtime rt=Runtime.getRuntime();
        long last=System.currentTimeMillis();
        while(true)
        {
            if(loops%10000==0)
            {
                long now=System.currentTimeMillis();
                long duration=now-last;
                last=now;
                System.err.printf("duration=%d q.size=%d memory max=%d free=%d total=%d%n",duration,queue.size(),rt.maxMemory(),rt.freeMemory(),rt.totalMemory());                            
            }
            queue.add(object);
            queue.remove(object);
            ++loops;
        }
    }
}

---------- END SOURCE ----------


Comments
Regression test for this: http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/jtreg/util/concurrent/ConcurrentLinkedQueue/RemoveLeak.java?view=markup Fix for this appears to be JSR166 CVS commit: revision 1.110 date: 2015/01/16 22:38:54; author: jsr166; state: Exp; lines: +15 -10 fix for 8054446: Repeated offer and remove on ConcurrentLinkedQueue lead to an OutOfMemoryError $ cvs diff -r 1.109 -r 1.110 ./main/java/util/concurrent/ConcurrentLinkedQueue.java Index: ./main/java/util/concurrent/ConcurrentLinkedQueue.java =================================================================== RCS file: /export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/ConcurrentLinkedQueue.java,v retrieving revision 1.109 retrieving revision 1.110 diff -u -r1.109 -r1.110 --- ./main/java/util/concurrent/ConcurrentLinkedQueue.java 4 Jan 2015 09:15:11 -0000 1.109 +++ ./main/java/util/concurrent/ConcurrentLinkedQueue.java 16 Jan 2015 22:38:54 -0000 1.110 @@ -447,18 +447,23 @@ */ public boolean remove(Object o) { if (o != null) { - Node<E> pred = null; - for (Node<E> p = first(); p != null; p = succ(p)) { + Node<E> next, pred = null; + for (Node<E> p = first(); p != null; pred = p, p = next) { + boolean removed = false; E item = p.item; - if (item != null && - o.equals(item) && - casItem(p, item, null)) { - Node<E> next = succ(p); - if (pred != null && next != null) - casNext(pred, p, next); - return true; + if (item != null) { + if (!o.equals(item)) { + next = succ(p); + continue; + } + removed = casItem(p, item, null); } - pred = p; + + next = succ(p); + if (pred != null && next != null) // unlink + casNext(pred, p, next); + if (removed) + return true; } } return false;
26-09-2015