91

I am expecting the buffered reader and file reader to close and the resources released if the exception is throw.

public static Object[] fromFile(String filePath) throws FileNotFoundException, IOException
{
    try (BufferedReader br = new BufferedReader(new FileReader(filePath)))
    {
        return read(br);
    } 
}

However, is there a requirement to have a catch clause for successful closure?

EDIT:

Essentially, is the above code in Java 7 equivalent to the below for Java 6:

public static Object[] fromFile(String filePath) throws FileNotFoundException, IOException
{

    BufferedReader br = null;

    try
    {
        br = new BufferedReader(new FileReader(filePath));

        return read(br);
    }
    catch (Exception ex)
    {
        throw ex;
    }
    finally
    {
        try
        {
            if (br != null) br.close();
        }
        catch(Exception ex)
        {
        }
    }

    return null;
}
yair
  • 8,945
  • 4
  • 31
  • 50
Cheetah
  • 13,785
  • 31
  • 106
  • 190
  • After reading your question again, I'm not sure if I understand it well. Can you please explain it? – Maroun Jul 15 '13 at 09:39
  • Hi. Cheetah, I'm trying to understand the role of the first `catch` of your example for Java 6. I.e. `catch (Exception ex) { throw ex; }` — it's just re-throwing the exception, it's doing nothing, it can be easily removed without any hurt. Or am I missing something? – Sasha Sep 05 '17 at 08:10

2 Answers2

107

It's correct and there's no requirement for catch clause. Oracle java 7 doc says the resource will be closed regardless of whether an exception is actually thrown or not.

You should use a catch clause only if you want to react upon the exception. The catch clause will be executed after the resource is closed.

Here's a snippet from Oracle's tutorial:

The following example reads the first line from a file. It uses an instance of BufferedReader to read data from the file. BufferedReader is a resource that must be closed after the program is finished with it:

static String readFirstLineFromFile(String path) throws IOException {
    try (BufferedReader br =
                   new BufferedReader(new FileReader(path))) {
        return br.readLine();
    }
} // In this example, the resource declared in the try-with-resources statement is a BufferedReader.

... Because the BufferedReader instance is declared in a try-with-resource statement, it will be closed regardless of whether the try statement completes normally or abruptly (as a result of the method BufferedReader.readLine throwing an IOException).

EDIT

Regarding the new edited question:

The code in Java 6 executes the catch and afterwards the finally block. This causes the resources to be still potentially opened in the catch block.

In Java 7 syntax, resources are closed before the catch block, so resources are already closed during the catch block execution. This is documented in the above link:

In a try-with-resources statement, any catch or finally block is run after the resources declared have been closed.

yair
  • 8,945
  • 4
  • 31
  • 50
70

Your usage of try-with-resources will work fine in this particular case, but it is not quite correct in general. You should not chain resources like that because it may lead to unpleasant surprises. Assume you have a variable buffer size:

public static Object[] fromFile(String filePath) throws FileNotFoundException, IOException
{
    int sz = /* get buffer size somehow */
    try (BufferedReader br = new BufferedReader(new FileReader(filePath), sz))
    {
        return read(br);
    } 
}

Assume something went wrong and you ended up with sz being negative. In this case your file resource (created via new FileReader(filePath)) will NOT be closed.

To avoid this problem you should specify each resource separately like this:

public static Object[] fromFile(String filePath) throws FileNotFoundException, IOException
{
    int sz = /* get buffer size somehow */
    try (FileReader file = new FileReader(filePath);
         BufferedReader br = new BufferedReader(file, sz))
    {
        return read(br);
    } 
}

In this case even if initialization of br fails file still gets closed. You can find more details here and here.

Andrii Polunin
  • 1,306
  • 11
  • 15
  • I'm trying to understand why the resource created via `new FileReader(filePath))` would not close in case an `IllegalArgumentException` is thrown when sz is negative. Doesn't the try-with-resources close all the `AutoClosable` resources irrespective of any exceptions thrown? – Prasoon Joshi Dec 23 '15 at 11:08
  • 4
    @PrasoonJoshi No, it only calls `.close()` for the variables which have been declared in the try-with-resources initializer. That's why separating it into two declarations in this example does the trick. – Mario Carneiro Feb 02 '16 at 23:20
  • 4
    Andrii and @Mario You are both right and wrong. In the first example, the FileReader is not closed by the try-with-resource logic. But the when the BufferedReader is closed, it will close the wrapped FileReader as well. For proof, have a look at the source of java.io.BufferedReader.close(). As a consequence, the code from the first example should be prefered, because it is more concise. – jschreiner Apr 21 '16 at 14:32
  • 7
    @jschreiner True, although Andrii's (somewhat contrived) issue in which `sz < 0` causes the constructor to throw an exception will in fact cause the resource to leak. – Mario Carneiro Apr 22 '16 at 04:29
  • 5
    @mario I agree. The outer constructor might fail, and the inner resource would be leaked. I didn't see that before, thank you. – jschreiner Apr 24 '16 at 20:00