views:

563

answers:

7

This simple sample code demonstrates the problem. I create an ArrayBlockingQueue, and a thread that waits for data on this queue using take(). After the loop is over, in theory both the queue and the thread can be garbage collected, but in practice I soon get an OutOfMemoryError. What is preventing this to be GC'd, and how can this be fixed?

/**
 * Produces out of memory exception because the thread cannot be garbage
 * collected.
 */
@Test
public void checkLeak() {
    int count = 0;
    while (true) {

        // just a simple demo, not useful code.
        final ArrayBlockingQueue<Integer> abq = new ArrayBlockingQueue<Integer>(2);
        final Thread t = new Thread(new Runnable() {
            @Override
            public void run() {
                try {
                    abq.take();
                } catch (final InterruptedException e) {
                    e.printStackTrace();
                }
            }
        });
        t.start();

        // perform a GC once in a while
        if (++count % 1000 == 0) {
            System.out.println("gc");
            // this should remove all the previously created queues and threads
            // but it does not
            System.gc();
        }
    }
}

I am using Java 1.6.0.

UPDATE: perform GC after a few iterations, but this does not help.

A: 

You start the thread, so all those new threads will be running asynchronously while the loop continues to create new ones.

Since your code is locking, the threads are life references in the system and cannot be collected. But even if they were doing some work, the threads are unlikely to be terminating as quickly as they are created (at least in this sample), and therefore the GC cannot collect all memory and will eventually fail with an OutOfMemoryException.

Creating as many threads is neither efficient nor efficient. If it is not a requirement to have all those pending operations run in parallel, you may want to use a thread pool and a queue of runnables to process.

Lucero
I don't think thats the problem, even when I run System.gc() in each loop the threads are not destroyed.
martinus
@martinus read more carefully. The amount of time it takes to garbage collect the threads is longer than the amount of time it takes to create the threads. If you start filling an unstoppered bathtub and you add water faster than the tub drains, the tub will eventually fill up and overflow. It's a resource allocation/deallocation race condition.What happens if you only create 100 threads, and continue in the while loop? Do the threads get collected eventually?
Wedge
@wedge, no, they dont ever get collected! or do something else. Even when I wait one second at the end of each loop nothing is ever GC'd.
martinus
It has nothing to do with creation vs. destruction times. The objects are not collected because the thread never terminates, because it waits upon a queue that is never filled. (see the other posts)
cadrian
@cadrian: true, it will wait, but my statement is still correct that he just creates them before they are released. And that's the main point, even if he were to add a put(0) or whatever to unblock the thread he would eventually (somewhat later) get into this situation.
Lucero
No, because GC would kick in before OutOfMemory occurs, thus freeing the objects. OutOfMemory only occurs if memory cannot be released, which would not be the case in that scenario.
Robin
@Robin, as long as the threads are running, they will not be collected. Creation in this tight loop will be way faster than the lifecycle of one thread, therefore they will accumulate and eventually lead to a OutOfMemory situation.
Lucero
True enough, but I am not sure if the queue, thread creation and thread start would actually be shorter than the thread lifecycle in this simple case. Anything more complex (and realistic) would cause this to occur. Of course the example in the question is not very realistic anyway.
Robin
I guess some people don't like me. I've been voted down 5 times for my answer, which did initially not state that the thread was blocking, but which was accurate about the issue causing OutOfMemoryException. Oh well.
Lucero
+5  A: 

You are creating threads indefinitely because they all block until ArrayBlockingQueue<Integer> abq has some entry. So eventually you'll get a OutOfMemoryError.

(edit)

Each thread you create will never end because it blocks until the abq queue as one entry. If the thread is running, the GC isn't going to collect any object that the thread is referencing including the queue abq and the thread itself.

bruno conde
yes but the queue is not referenced any more when the loop is over
martinus
Why do you say the loop is over? the loop continues forever ...
bruno conde
I mean when the end of the loop starts again on the top, the previously created queue and thread are not referenced any more.
martinus
Yes they are because the thread never stops !!!
bruno conde
A: 

Your while loop is an infinite loop and its creating new threads continuously. Although you starting the thread execution as soon as its created but the time its taking to complete the task by the thread is greater then the time its taking to create the thread.

Also what are doing with the abq parameter by declaring it inside the while loop?

Based on your edits and other comments. System.gc() doesn't not guarantee a GC cycle. Read my statement above the speed of execution of your thread is lower than the speed of creation.

I checked the comment for the take() method "Retrieves and removes the head of this queue, waiting if no elements are present on this queue." I see you define the ArrayBlockingQueue but you not adding any elements to it so all your thread are just waiting on that method, that is why you getting OOM.

Bhushan
+1  A: 
abq.put(0);

should save your day.

Your threads all wait on their queue's take() but you never put anything in those queues.

cadrian
This is obviously the correct answer!
ng
A: 

I do not know how threads are implemented in Java, but one possible reason comes to mind why the queues and threads are not collected: The threads may be wrappers for system threads using system synchronization primitives, in which case the GC cannot automatically collect a waiting thread, since it cannot tell whether the thread is alive or not, i.e. the GC simply does not know that a thread cannot be woken.

I can't say what's the best way to fix it, since I'd need to know what you are trying to do, but you could look at java.util.concurrent to see if it has classes for doing what you need.

TrayMan
A: 

The System.gc call does nothing because there is nothing to collect. When a thread starts it increments the threads reference count, not doing so will mean the thread will terminate indeterminately. When the thread's run method completes, then the thread's reference count is decremented.

while (true) {
    // just a simple demo, not useful code.
    // 0 0 - the first number is thread reference count, the second is abq ref count
    final ArrayBlockingQueue<Integer> abq = new ArrayBlockingQueue<Integer>(2);
    // 0 1
    final Thread t = new Thread(new Runnable() {
        @Override
        public void run() {
            try {
                abq.take();
                // 2 2
            } catch (final InterruptedException e) {
                e.printStackTrace();
            }
        }
    });
    // 1 1
    t.start();
    // 2 2 (because the run calls abq.take)
    // after end of loop
    // 1 1 - each created object's reference count is decreased
}

Now, there is a potential race condition - what if the main loop terminates and does garbage collection before the thread t has a chance to do any processing, i.e. it is suspended by the OS before the abq.take statement is executed? The run method will try to access the abq object after the GC has released it, which would be bad.

To avoid the race condition, you should pass the object as a parameter to the run method. I'm not sure about Java these days, it's been a while, so I'd suggest passing the object as a constructor parameter to a class derived from Runnable. That way, there's an extra reference to abq made before the run method is called, thus ensuring the object is always valid.

Skizz

Skizz
Java does not use reference counting for GC.
TrayMan
A quick search confirms there's no reference counting. Replace 'reference count' with 'number of references to the object'. The memory used by objects is only released when there are no references to the object. In the sample code, there are references being made the OP didn't realise were there.
Skizz
+4  A: 

Threads are top level objects. They are 'special' so they do not follow the same rules as other objects. The do not rely on references to keep them 'alive' (i.e. safe from GC). A thread will not get garbage collected until it has ended. Which doesn't happen in your sample code, since the thread is blocked. Of course, now that the thread object is not garbage collected, then any other object referenced by it (the queue in your case) also cannot be garbage collected.

Robin