Correct idiom for managing multiple chained resour

2019-01-03 21:23发布

The Java 7 try-with-resources syntax (also known as ARM block (Automatic Resource Management)) is nice, short and straightforward when using only one AutoCloseable resource. However, I am not sure what is the correct idiom when I need to declare multiple resources that are dependent on each other, for example a FileWriter and a BufferedWriter that wraps it. Of course, this question concerns any case when some AutoCloseable resources are wrapped, not only these two specific classes.

I came up with the three following alternatives:

1)

The naive idiom I have seen is to declare only the top-level wrapper in the ARM-managed variable:

static void printToFile1(String text, File file) {
    try (BufferedWriter bw = new BufferedWriter(new FileWriter(file))) {
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
}

This is nice and short, but it is broken. Because the underlying FileWriter is not declared in a variable, it will never be closed directly in the generated finally block. It will be closed only through the close method of the wrapping BufferedWriter. The problem is, that if an exception is thrown from the bw's constructor, its close will not be called and therefore the underlying FileWriter will not be closed.

2)

static void printToFile2(String text, File file) {
    try (FileWriter fw = new FileWriter(file);
            BufferedWriter bw = new BufferedWriter(fw)) {
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
}

Here, both the underlying and the wrapping resource are declared in the ARM-managed variables, so both of them will certainly be closed, but the underlying fw.close() will be called twice: not only directly, but also through the wrapping bw.close().

This should not be a problem for these two specific classes that both implement Closeable (which is a subtype of AutoCloseable), whose contract states that multiple calls to close are permitted:

Closes this stream and releases any system resources associated with it. If the stream is already closed then invoking this method has no effect.

However, in a general case, I can have resources that implement only AutoCloseable (and not Closeable), which doesn't guarantee that close can be called multiple times:

Note that unlike the close method of java.io.Closeable, this close method is not required to be idempotent. In other words, calling this close method more than once may have some visible side effect, unlike Closeable.close which is required to have no effect if called more than once. However, implementers of this interface are strongly encouraged to make their close methods idempotent.

3)

static void printToFile3(String text, File file) {
    try (FileWriter fw = new FileWriter(file)) {
        BufferedWriter bw = new BufferedWriter(fw);
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
}

This version should be theoretically correct, because only the fw represents a real resource that needs to be cleaned up. The bw doesn't itself hold any resource, it only delegates to the fw, so it should be sufficient to only close the underlying fw.

On the other hand, the syntax is a bit irregular and also, Eclipse issues a warning, which I believe is a false alarm, but it is still a warning that one has to deal with:

Resource leak: 'bw' is never closed


So, which approach to go for? Or have I missed some other idiom that is the correct one?

8条回答
Rolldiameter
2楼-- · 2019-01-03 21:44

Since your resources are nested, your try-with clauses should also be:

try (FileWriter fw=new FileWriter(file)) {
    try (BufferedWriter bw=new BufferedWriter(fw)) {
        bw.write(text);
    } catch (IOException ex) {
        // handle ex
    }
} catch (IOException ex) {
    // handle ex
}
查看更多
走好不送
3楼-- · 2019-01-03 21:52

My solution is to do a "extract method" refactoring, as following:

static AutoCloseable writeFileWriter(FileWriter fw, String txt) throws IOException{
    final BufferedWriter bw  = new BufferedWriter(fw);
    bw.write(txt);
    return new AutoCloseable(){

        @Override
        public void close() throws IOException {
            bw.flush();
        }

    };
}

printToFile can be written either

static void printToFile(String text, File file) {
    try (FileWriter fw = new FileWriter(file)) {
        AutoCloseable w = writeFileWriter(fw, text);
        w.close();
    } catch (Exception ex) {
        // handle ex
    }
}

or

static void printToFile(String text, File file) {
    try (FileWriter fw = new FileWriter(file);
        AutoCloseable w = writeFileWriter(fw, text)){

    } catch (Exception ex) {
        // handle ex
    }
}

For class lib designers, I will suggest them extend the AutoClosable interface with an additional method to suppress the close. In this case we can then manually control the close behavior.

For language designers, the lesson is that adding a new feature could mean adding a lot others. In this Java case, obviously ARM feature will work better with a resource ownership transfer mechanism.

UPDATE

Originally the code above requires @SuppressWarning since the BufferedWriter inside the function requires close().

As suggested by a comment, if flush() to be called before close the writer, we need to do so before any return (implicit or explicit) statements inside the try block. There is currently no way to ensure the caller doing this I think, so this must be documented for writeFileWriter.

UPDATE AGAIN

The above update makes @SuppressWarning unnecessary since it require the function to return the resource to the caller, so itself does not necessary being closed. Unfortunately, this pull us back to the beginning of the situation: the warning is now moved back to the caller side.

So to properly solve this, we need a customised AutoClosable that whenever it closes, the underline BufferedWriter shall be flush()ed. Actually, this shows us another way to bypass the warning, since the BufferWriter is never closed in either way.

查看更多
登录 后发表回答