views:

550

answers:

3

We have a rather large and complex application written in Java which is running on top of the Gridgain package. The problem I am having is that this application will sit there processing requests for approximately a day before every request starts resulting in an exception of type java.nio.channels.ClosedByInterruptException.

My supposition is that the application is not releasing file handles and after a day of continuous usage it runs out and can no longer continue to process requests (each request requires the reading of multiple files from each grid node). We have wrapped most of our file IO operations in classes such as this one

package com.vlc.edge;

import com.vlc.common.VlcRuntimeException;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
import java.io.Reader;

public final class BufferedReaderImpl implements BufferedReader {
    private java.io.BufferedReader reader;

    public BufferedReaderImpl(final String source) {
        this(new File(source));
    }

    public BufferedReaderImpl(final File source) {
        try {
            reader = new java.io.BufferedReader(new FileReader(source));
        } catch (FileNotFoundException e) {
            throw new VlcRuntimeException(e);
        }
    }

    public BufferedReaderImpl(final Reader reader) {
        this.reader = new java.io.BufferedReader(reader);
    }

    public String readLine() {
        try {
            return reader.readLine();
        } catch (IOException e) {
            throw new VlcRuntimeException(e);
        }
    }

    public void close() {
        try {
            reader.close();
        } catch (IOException e) {
            throw new VlcRuntimeException(e);
        }
    }
}

I think the problem is that this design doesn't explicitly release the file handle, my proposed solution is to add a finalize method like this

    protected void finalize() throws Throwable
    {
        reader.close();
        super.finalize();   
    }

which will do this explicitly. The question (finally) is whether or not this is likely to have any effect. Do classes such as java.io.BufferedReader already have some mechanism for dealing with this type of problem?

EDIT: Also greatly appreciated here would be ways of checking whether this is actually the problem... ie is there a way to query a running JVM and ask about it's file handle allocations?

+6  A: 

Finalizers can't be relied on to be called. It's not a good approach for resource management. The standard construct in Java for this is:

InputStream in = null;
try {
  in = ...;
  // do stuff
} catch (IOException e) {
  // error
} finally {
  if (in != null) { try { in.close(); } catch (Exception e) { } }
  in = null;
}

You may want to wrap these handles inside a class but that's not a robust approach.

cletus
+1 ... but if you open the stream in the initialization expression for 'in', you can get rid of the 'null' test in the 'finally' block. Also, the nulling of 'in' at the end may be redundant.
Stephen C
+1  A: 

Java specification says that it is not guarantee that 'finalize()' will be executed. Your code must explicitly close FileReader yourself.

NawaMan
+2  A: 

There is little point in overriding finalize(). If the handle is getting GCed and finalized, then so is the instance of java.io.BufferedReader and it will get closed.

It is possible (according to the spec) that the handle is being GCed but not finalized, but this is not very likely.

You could try using PhantomReferences to clean up unused file handles, but my guess is that your instances of BufferedReaderImpl are still referenced from somewhere (e.g. values in a Map from filenames to open handles) and that is what is preventing them from being closed (in which case finalizers will not help.)

finnw
Ah that is a very good point, and exactly the type of answer I was looking for... will double check and come back to accept
Jamie Cook
Was that the solution to your problem?
david