Is It Necessary to Close Each Nested Outputstream and Writer Separately

Is it necessary to close each nested OutputStream and Writer separately?

Assuming all the streams get created okay, yes, just closing bw is fine with those stream implementations; but that's a big assumption.

I'd use try-with-resources (tutorial) so that any issues constructing the subsequent streams that throw exceptions don't leave the previous streams hanging, and so you don't have to rely on the stream implementation having the call to close the underlying stream:

try (
OutputStream outputStream = new FileOutputStream(createdFile);
GZIPOutputStream gzipOutputStream = new GZIPOutputStream(outputStream);
OutputStreamWriter osw = new OutputStreamWriter(gzipOutputStream);
BufferedWriter bw = new BufferedWriter(osw)
) {
// ...
}

Note you no longer call close at all.

Important note: To have try-with-resources close them, you must assign the streams to variables as you open them, you cannot use nesting. If you use nesting, an exception during construction of one of the later streams (say, GZIPOutputStream) will leave any stream constructed by the nested calls inside it open. From JLS §14.20.3:

A try-with-resources statement is parameterized with variables (known as resources) that are initialized before execution of the try block and closed automatically, in the reverse order from which they were initialized, after execution of the try block.

Note the word "variables" (my emphasis).

E.g., don't do this:

// DON'T DO THIS
try (BufferedWriter bw = new BufferedWriter(
new OutputStreamWriter(
new GZIPOutputStream(
new FileOutputStream(createdFile))))) {
// ...
}

...because an exception from the GZIPOutputStream(OutputStream) constructor (which says it may throw IOException, and writes a header to the underlying stream) would leave the FileOutputStream open. Since some resources have constructors that may throw and others don't, it's a good habit to just list them separately.

We can double-check our interpretation of that JLS section with this program:

public class Example {

private static class InnerMost implements AutoCloseable {
public InnerMost() throws Exception {
System.out.println("Constructing " + this.getClass().getName());
}

@Override
public void close() throws Exception {
System.out.println(this.getClass().getName() + " closed");
}
}

private static class Middle implements AutoCloseable {
private AutoCloseable c;

public Middle(AutoCloseable c) {
System.out.println("Constructing " + this.getClass().getName());
this.c = c;
}

@Override
public void close() throws Exception {
System.out.println(this.getClass().getName() + " closed");
c.close();
}
}

private static class OuterMost implements AutoCloseable {
private AutoCloseable c;

public OuterMost(AutoCloseable c) throws Exception {
System.out.println("Constructing " + this.getClass().getName());
throw new Exception(this.getClass().getName() + " failed");
}

@Override
public void close() throws Exception {
System.out.println(this.getClass().getName() + " closed");
c.close();
}
}

public static final void main(String[] args) {
// DON'T DO THIS
try (OuterMost om = new OuterMost(
new Middle(
new InnerMost()
)
)
) {
System.out.println("In try block");
}
catch (Exception e) {
System.out.println("In catch block");
}
finally {
System.out.println("In finally block");
}
System.out.println("At end of main");
}
}

...which has the output:


Constructing Example$InnerMost
Constructing Example$Middle
Constructing Example$OuterMost
In catch block
In finally block
At end of main

Note that there are no calls to close there.

If we fix main:

public static final void main(String[] args) {
try (
InnerMost im = new InnerMost();
Middle m = new Middle(im);
OuterMost om = new OuterMost(m)
) {
System.out.println("In try block");
}
catch (Exception e) {
System.out.println("In catch block");
}
finally {
System.out.println("In finally block");
}
System.out.println("At end of main");
}

then we get the appropriate close calls:


Constructing Example$InnerMost
Constructing Example$Middle
Constructing Example$OuterMost
Example$Middle closed
Example$InnerMost closed
Example$InnerMost closed
In catch block
In finally block
At end of main

(Yes, two calls to InnerMost#close is correct; one is from Middle, the other from try-with-resources.)

Correct way to close nested streams and writers in Java

I usually do the following. First, define a template-method based class to deal with the try/catch mess

import java.io.Closeable;
import java.io.IOException;
import java.util.LinkedList;
import java.util.List;

public abstract class AutoFileCloser {
// the core action code that the implementer wants to run
protected abstract void doWork() throws Throwable;

// track a list of closeable thingies to close when finished
private List<Closeable> closeables_ = new LinkedList<Closeable>();

// give the implementer a way to track things to close
// assumes this is called in order for nested closeables,
// inner-most to outer-most
protected final <T extends Closeable> T autoClose(T closeable) {
closeables_.add(0, closeable);
return closeable;
}

public AutoFileCloser() {
// a variable to track a "meaningful" exception, in case
// a close() throws an exception
Throwable pending = null;

try {
doWork(); // do the real work

} catch (Throwable throwable) {
pending = throwable;

} finally {
// close the watched streams
for (Closeable closeable : closeables_) {
if (closeable != null) {
try {
closeable.close();
} catch (Throwable throwable) {
if (pending == null) {
pending = throwable;
}
}
}
}

// if we had a pending exception, rethrow it
// this is necessary b/c the close can throw an
// exception, which would remove the pending
// status of any exception thrown in the try block
if (pending != null) {
if (pending instanceof RuntimeException) {
throw (RuntimeException) pending;
} else {
throw new RuntimeException(pending);
}
}
}
}
}

Note the "pending" exception -- this takes care of the case where an exception thrown during close would mask an exception we might really care about.

The finally tries to close from the outside of any decorated stream first, so if you had a BufferedWriter wrapping a FileWriter, we try to close the BuffereredWriter first, and if that fails, still try to close the FileWriter itself. (Note that the definition of Closeable calls for close() to ignore the call if the stream is already closed)

You can use the above class as follows:

try {
// ...

new AutoFileCloser() {
@Override protected void doWork() throws Throwable {
// declare variables for the readers and "watch" them
FileReader fileReader =
autoClose(fileReader = new FileReader("somefile"));
BufferedReader bufferedReader =
autoClose(bufferedReader = new BufferedReader(fileReader));

// ... do something with bufferedReader

// if you need more than one reader or writer
FileWriter fileWriter =
autoClose(fileWriter = new FileWriter("someOtherFile"));
BufferedWriter bufferedWriter =
autoClose(bufferedWriter = new BufferedWriter(fileWriter));

// ... do something with bufferedWriter
}
};

// .. other logic, maybe more AutoFileClosers

} catch (RuntimeException e) {
// report or log the exception
}

Using this approach you never have to worry about the try/catch/finally to deal with closing files again.

If this is too heavy for your use, at least think about following the try/catch and the "pending" variable approach it uses.

IOUtils: is it required to close OutputStream?

IOUtils.copy(InputStream, OutputStream) must not close the OutputStream. You can use it for example to concatenate different InputStreams and in this case it would be a bad idea if it would close the provided Input- and/or OutputStream.

As a rule of thumb any method should close only those steams it opens.
See also Should I close the servlet outputstream?

Should streams be closed after every use?

If you open a stream, you should close it. The code in the question doesn't do that, it just abandons previous streams and closes only the very last ones it creates.

It's not clear why those stream variables should be static rather than local within receive. If they were local within receive, you could use try-with-resources to automatically clean them up:

public static void receive(){
try (
ByteArrayInputStream byteIn = new ByteArrayInputStream();
ObjectInputStream objectIn = new ObjectInputStream(new BufferedInputStream(byteIn));
) {
//do something
}
}

They'll be closed automatically when control passes out of the try.

If they have to be static class members for some reason, just close and release each one you create (but it's a lot easier to have bugs in your code that means it has execution paths that fail to do that).

// All done
objectIn.close();
objectIn = null;
byteIn.close();
byteIn = null;

Side note: With those three stream types, you may not need byteIn as a separate variable. More in this question's answers.

How many of these wrapped Java I/O objects do I have to close?

In general closing the outermost wrapper is enough; each wrapper closes the underlaying stream when it's closed.

It is possible to create a wrapper that doesn't do this, but that could be considered a bug since the documentation for FilterOutputStream requires that a call to close "releases any system resources associated with the stream."

Is it necessary to close a FileWriter, provided it is written through a BufferedWriter?

It is not necessary to close it, because BufferedWriter takes care of closing the writer it wraps.

To convince you, this is the source code of the close method of BufferedWriter:

public void close() throws IOException {
synchronized (lock) {
if (out == null) {
return;
}
try {
flushBuffer();
} finally {
out.close();
out = null;
cb = null;
}
}
}

Do I need to keep variables for all writers/readers to close them all manually?

With BufferedWriter, specifically, you can just use close on the BufferedWriter and it will call close on the underlying FileWriter.

But, as far as I'm aware, that's not documented, and not required of Writer implementations that wrap other Writers (and similarly, streams). I'm paranoid, and tend to close things explicitly (in reverse order of opening them).

You can see the close operation in the BufferedWriter source (this example is from JDK 11.0.1), though you have to look fairly closely:

public void close() throws IOException {
synchronized (lock) {
if (out == null) {
return;
}
try (Writer w = out) {
flushBuffer();
} finally {
out = null;
cb = null;
}
}
}

Note the use of try-with-resources to auto-close out (via w).

Can I close underlying outputstream and let the decorator BufferedOutputStream unclosed

Decorating classes usually do not open system resources (like an operating system file-handle) that must be closed to prevent resource leakage. The given OutputStream parameter must be closed (because it is probably associated with an open system resource), but as you mention, this is the responsibility of the caller.

But if you do not close the decorating class, you must take care of method calls that normally take place in the close() method. In case of BufferedOutputStream you can see that flush() is called in the super class FilterOutputStream close() method.

To accommodate these missing method calls, create a separate NonClosingBufferedOutputStream class that extends BufferedOutputStream that contains a modified version of the close() method from the FilterOutputStream (simply comment out the line out.close();). You can now use this class as a normal Closeable and avoid warnings from your IDE/compiler.

I was going to recommend using the CloseShieldOutputStream from Apache commons-io, but looking at the source code, it appears that flush() is not called in the close() method. Despite this shortcoming, replacing the underlying outputstream with a ClosedOutputStream does appear to be a good idea: it will show programming mistakes much sooner.

Note that for other decorating classes like the GZIPOutputStream, other methods besides flush() could be appropriate. In GZIPOutputStream's case, creating a NonClosingGZIPOutputStream class with a modified close() method that calls finish() could do the trick.

multi thread write only one outputstream and data loss

Try to use ConcurrentLinkedQueue<String> for buffer, with methods offer and poll instead of += and = "" on String reference.



Related Topics



Leave a reply



Submit