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.
No, you do not need to close the underlying FileWriter.
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 thetry
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.)
You can close the outer most stream, in fact you don't need to retain all the streams wrapped and you can use Java 7 try-with-resources.
try (BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(
new GZIPOutputStream(new FileOutputStream(createdFile)))) {
// write to the buffered writer
}
If you subscribe to YAGNI, or you-aint-gonna-need-it, you should be only adding code you actually need. You shouldn't be adding code you imagine you might need but in reality doesn't do anything useful.
Take this example and imagine what could possibly go wrong if you didn't do this and what the impact would be?
try (
OutputStream outputStream = new FileOutputStream(createdFile);
GZIPOutputStream gzipOutputStream = new GZIPOutputStream(outputStream);
OutputStreamWriter osw = new OutputStreamWriter(gzipOutputStream);
BufferedWriter bw = new BufferedWriter(osw)
) {
// ...
}
Lets start with FileOutputStream which calls open
to do all the real work.
/**
* Opens a file, with the specified name, for overwriting or appending.
* @param name name of file to be opened
* @param append whether the file is to be opened in append mode
*/
private native void open(String name, boolean append)
throws FileNotFoundException;
If the file is not found, there is no underlying resource to close, so closing it won't make any difference. If The file exists, it should be throwing a FileNotFoundException. So there is nothing to be gained by trying to close the resource from this line alone.
The reason you need to close the file is when the file is opened successfully, but you later get an error.
Lets look at the next stream GZIPOutputStream
There is code which can throw an exception
private void writeHeader() throws IOException {
out.write(new byte[] {
(byte) GZIP_MAGIC, // Magic number (short)
(byte)(GZIP_MAGIC >> 8), // Magic number (short)
Deflater.DEFLATED, // Compression method (CM)
0, // Flags (FLG)
0, // Modification time MTIME (int)
0, // Modification time MTIME (int)
0, // Modification time MTIME (int)
0, // Modification time MTIME (int)
0, // Extra flags (XFLG)
0 // Operating system (OS)
});
}
This writes the header of the file. Now it would be very unusual for you to be able to open a file for writing but not be able to write even 8 bytes to it, but lets imagine this could happen and we don't close the file afterwards. What does happen to a file if it is not closed?
You don't get any unflushed writes, they are discarded and in this case, there is no successfully written bytes to the stream which isn't buffered at this point anyway. But a file which is not closed doesn't live forever, instead FileOutputStream has
protected void finalize() throws IOException {
if (fd != null) {
if (fd == FileDescriptor.out || fd == FileDescriptor.err) {
flush();
} else {
/* if fd is shared, the references in FileDescriptor
* will ensure that finalizer is only called when
* safe to do so. All references using the fd have
* become unreachable. We can call close()
*/
close();
}
}
}
If you don't close a file at all, it gets closed anyway, just not immediately (and like I said, data which is left in a buffer will be lost this way, but there is none at this point)
What is the consequence of not closing the file immediately? Under normal conditions, you potentially lose some data, and you potentially run out of file descriptors. But if you have a system where you can create files but you can't write anything to them, you have a bigger problem. i.e. it hard to imagine why you are repeatedly trying to create this file despite the fact you are failing.
Both OutputStreamWriter and BufferedWriter don't throw IOException in their constructors, so it not clear what problem they would cause. In The case of BufferedWriter, you could get an OutOfMemoryError. In this case it will immediately trigger a GC, which as we have seen will close the file anyway.
If all of the streams have been instantiated then closing only the outermost is just fine.
The documentation on Closeable
interface states that close method:
Closes this stream and releases any system resources associated with it.
The releasing system resources includes closing streams.
It also states that:
If the stream is already closed then invoking this method has no effect.
So if you close them explicitly afterwards, nothing wrong will happen.
I'd rather use try(...)
syntax (Java 7), e.g.
try (OutputStream outputStream = new FileOutputStream(createdFile)) {
...
}
It will be fine if you only close the last stream - the close call will be send to the underlying streams, too.
No, the topmost level Stream
or reader
will ensure that all underlying streams / readers are closed.
Check the close()
method implementation of your topmost level stream.
In Java 7, there is a feature try-with-resources. You no need to explicitly close your streams, it will take care of that.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With