Multithread garbage collection in Java: Sharing object outside prevents GC?

Question

Consider the following method:

public void Parse(String[] S, Objects[] O) throws IOException {
    final int N_THREADS = Runtime.getRuntime().availableProcessors();
    BlockingQueue<Runnable> blockingQueue = new ArrayBlockingQueue<Runnable>(20);
    RejectedExecutionHandler rejectedExecutionHandler = new ThreadPoolExecutor.CallerRunsPolicy();
    ThreadPoolExecutor service =  new ThreadPoolExecutor(N_THREADS, N_THREADS, 0L, TimeUnit.MILLISECONDS, blockingQueue, rejectedExecutionHandler);
    final SomeObject RO = new SomeObject();
    for(String s : S){
        service.execute(new Runnable() {
            public void run() {
                // initialize variables
                for (Object o : O) {
                        V ps = RO.apply(sentence);
                        //more work on ps 
                }
                File f = new File("something");
                FileWriter fw = null;
                try {
                    fw = new FileWriter(f.getAbsoluteFile());
                    BufferedWriter bw = new BufferedWriter(fw);
                } catch (IOException e) {
                    System.out.println(f.getAbsoluteFile());
                }
                BufferedWriter bw = new BufferedWriter(fw);
                for (SentenceAnnotation entry : annotations) {
                    try {
                        bw.write(entry.toString());
                        bw.newLine();
                    } catch (IOException e) {
                        e.printStackTrace();
                    }
                }

                try {
                    bw.flush();
                    bw.close();
                    fw.close();
                } catch (IOException e) {
                    e.printStackTrace();
                }
            }
        });
    }
    service.shutdown();
    while (!service.isTerminated()) {
    }
    long timeEnd = System.currentTimeMillis();
}

where S is a large array (hundreds of thousands) and O is of say length 50. My question is regarding the RO object. It is created outside and "shared" if you will by all the threads. Now when this code has run for some time the heap space runs out which has puzzled me. I am inclined to think that the RO object still keeps the otherwise completed Runnables alive and slowly eat up the memory. Is that true? I have monitored the memory consumption of the linux system (latest version of Oracle JDK) using `free -m' and I can slowly but surely see the memory disappear. I am thankful for any advice you can give me.

score 1 · Accepted Answer · answered Oct 24 '13 at 20:52

 try {
   fw = new FileWriter(f.getAbsoluteFile());
   BufferedWriter bw = new BufferedWriter(fw);
 } catch (IOException e) {
   System.out.println(f.getAbsoluteFile());
 }
 BufferedWriter bw = new BufferedWriter(fw);

You're leaking an unclosed BufferedWriter in this section of code. You create the first one in the try clause's scope, and don't close it. The reference disappears, but whatever native handles the runtime creates are not freed. You don't notice because you create a new BufferedWriter to the same file immediately afterwards.

score 0 · Answer 2 · edited May 23 '17 at 12:05

As far as I can see, there is nothing looking suspicious in the code you showed.

Your best option is to get a heap dump of the application, and then check what is filling your memory.

You can generate a heap dump and perform basic analysis on it with JVisualVM, which is contained in the bin folder of you jdk. You will surely find many questions on SO regarding heap analysis, for instance How to find a Java Memory Leak .

score 0 · Answer 3 · answered Oct 24 '13 at 21:12

You seem to be creating the BufferedWriter twice. I'm not quite sure about the scope issues here, but it seems to me that this shouldn't even compile correctly. Try declaring the BufferedWriter before the "try" block and simply use it without the second creation:

        BufferedWriter bw;            
        try {
            fw = new FileWriter(f.getAbsoluteFile());
            bw = new BufferedWriter(fw);
        } catch (IOException e) {
            System.out.println(f.getAbsoluteFile());
        }
        for (SentenceAnnotation entry : annotations) {
            try {
                bw.write(entry.toString());
                bw.newLine();
            } catch (IOException e) {
                e.printStackTrace();
            }
        }

If I'm right, then you won't be generating "hundreds of thousands" of unnecessary BufferedWriter objects. No guarantees, though.

As a style issue, I'd consider combining the "try" blocks into one and use the one "catch" instead of two.... unless you intend to give different error messages, of course.

Hope that helps.

Achim

Multithread garbage collection in Java: Sharing object outside prevents GC?

3 Answers3