Closing ByteArrayOutputStreams

[Scott Harper]

I've read in a couple places (the Essential Java Classes trail of the Sun Java
Tutorial being one), that "A program should close a stream as soon as it is
done with it, in order to free up system resources." Makes sense on face
value, but as I studied this more, I have a few questions.

First, if I have a locally declared stream (any stream) inside a method, when
the method returns and the stream object goes out of scope, wouldn't it
automatically be cleaned up, closed, and returned to the system? I think of
this the way other objects behave, and when their reference counts go to zero,
they are garbage collected. However, one colleague is of the opinion that
this is not the case, and that I have to close the stream before it can be
finalized and handed over to the garbage collector.

In this case, I am dealing primarily with ByteArrayOutputStreams, and it seems
counter-intuitive that they are consuming a lot of system resources anyway. A
little more digging into the API spec shows that

"Closing a ByteArrayOutputStream has no effect. The methods in this class can
be called after the stream has been closed without generating an IOException."

and further more, for OutputStreams in general:

"The close method of OutputStream does nothing."

So that really confuses me. I suspect that an ancestor of OutputStream
implements the Closeable interface, and therefore it gets inherited down to
OutputStream and ByteArrayOutputStream. It also makes sense that experienced
Java programmers just get into the habit of closing streams when they are
done, so by *not* implementing closeable on OutputStreams it could just be
confusing as well.

The only "problem" I have now is that (in theory anyway) ByteArrayOutputStream
can throw an IOException. And now I have to handle that and pass it all the
way up the call stack until some method can handle it intelligently. I have
my doubts that closing a ByteArrayOutputStream would ever generate an
IOException.

So what's the consensus? Don't close ByteArrayOutputStreams? Close them, but
ignore the possible IOException? Or go the full mile and be prepared to
handle the exception?


Thanks,
scott

 

[VisionSet]

So what's the consensus? Don't close ByteArrayOutputStreams? Close them, but
ignore the possible IOException? Or go the full mile and be prepared to
handle the exception?

Do what you like it, it is an empty implementation.
Personally I wouldn't bother calling close(), so long as polymorphism isn't
involved. Just bung a comment in, if you feel the need.

 

[Roedy Green]

Do what you like it, it is an empty implementation.
Personally I wouldn't bother calling close(), so long as polymorphism isn't
involved. Just bung a comment in, if you feel the need.

my advice would be the opposite. That close does nothing is but a
temporary side effect of the implementation. In some future
implementation or some subclass it could well do something important.
Streams get closed. It is part of the contract.

Not calling close violates the principle of encapsulation. Method
bodies are supposed to be treated like black boxes.

 

[Adam Maass]

I've read in a couple places (the Essential Java Classes trail of the Sun
Java
Tutorial being one), that "A program should close a stream as soon as it
is
done with it, in order to free up system resources." Makes sense on face
value, but as I studied this more, I have a few questions.

First, if I have a locally declared stream (any stream) inside a method,
when
the method returns and the stream object goes out of scope, wouldn't it
automatically be cleaned up, closed, and returned to the system? I think
of
this the way other objects behave, and when their reference counts go to
zero,
they are garbage collected.

Almost true. The objects may be garbage collected when they are no longer
reachable. Note that there is no guarantee that the objects will ever be
garbage collected, nor is there any guarantee of how timely any garbage
collection will be. The only guarantee is that all objects that are eligible
for garbage collection will have actually been garbage collected before the
VM throws an OutOfMemoryError.

In particular, if your streams are local to a method, when the method
returns, the streams are eligible for garbage collection. This does not
necessarily mean that they will be garbage collected, only that they could
be the next time the VM runs a garbage collection cycle.

However, one colleague is of the opinion that
this is not the case, and that I have to close the stream before it can be
finalized and handed over to the garbage collector.

This is almost certainly not true. Having said that, it is good practice to
explicitly close streams when you're through with them... many of them have
underlying OS resources that are freed on close.

In this case, I am dealing primarily with ByteArrayOutputStreams, and it
seems
counter-intuitive that they are consuming a lot of system resources
anyway. A
little more digging into the API spec shows that

"Closing a ByteArrayOutputStream has no effect. The methods in this class
can
be called after the stream has been closed without generating an
IOException."

ByteArrayOutputStreams are a special case. Their 'close' methods do nothing.

and further more, for OutputStreams in general:

"The close method of OutputStream does nothing."

What this means is that the 'close' method on the OutputStream class itself
does nothing. Subclasses of OutputStream generally override the version in
OutputStream to do something more interesting than 'nothing'.

So that really confuses me. I suspect that an ancestor of OutputStream
implements the Closeable interface, and therefore it gets inherited down
to
OutputStream and ByteArrayOutputStream. It also makes sense that
experienced
Java programmers just get into the habit of closing streams when they are
done, so by *not* implementing closeable on OutputStreams it could just be
confusing as well.

The 'Closeable' interface is new in Java 5. All it requires is a method
'close' with no parameters. It's supposed to clean up any system resources
occupied by the object it's implemented on. In the case of OutputStream,
this is a no-op because OutputStream is abstract -- there is no concrete
system resource to clean up in OutputStream itself. In the case of
ByteArrayOutputStream, it is also a no-op because ByteArrayOutputStream does
not occupy any system resources -- other than memory, which is handled
through garbage collection.

OutputStream is the top of the hierarchy. The only (class) ancestor of
OutputStream is Object.

The only "problem" I have now is that (in theory anyway)
ByteArrayOutputStream
can throw an IOException. And now I have to handle that and pass it all
the
way up the call stack until some method can handle it intelligently. I
have
my doubts that closing a ByteArrayOutputStream would ever generate an
IOException.

ByteArrayOutputStream won't. (You can examine the source for yourself.)
However, if you ever switch to a different type of OutputStream...

So what's the consensus? Don't close ByteArrayOutputStreams? Close them,
but
ignore the possible IOException? Or go the full mile and be prepared to
handle the exception?

Tradeoffs, tradeoffs, tradeoffs.

IMHO, close the ByteArrayOutputStream because it is the expected thing to
do. (Or add copious comments about why you chose not to, and why doing it
this way is expected to work.) You now need to decide what to do with the
possible exception. Choices: ignore, log, propogate. Ignoring an exception
is almost always bad news. Logging the exception might be the fastest way
around this, especially since it should never happen. Propogate if there's
some chance that someone higher up the call chain needs to know that
something bad happened.

Note that you need not propogate the IOException itself, which might not be
such a bad idea:

try
{
// your stuff
bytearrayoutputstream.close();
}
catch(IOException e)
{
throw new FancyDancyAPIException(e);
}

 

[Chris Uppal]

First, if I have a locally declared stream (any stream) inside a method,
when the method returns and the stream object goes out of scope, wouldn't
it automatically be cleaned up, closed, and returned to the system? I
think of this the way other objects behave, and when their reference
counts go to zero, they are garbage collected. However, one colleague is
of the opinion that this is not the case, and that I have to close the
stream before it can be finalized and handed over to the garbage
collector.

Your colleague is technically wrong; if the stream is cleaned up by
finalisation at all, then it will be cleaned up whether or not you have called
close() explicitly. However, the general point is good -- you should not rely
on finalisation if you can possibly avoid it.

BTW, there are some cases where it is appropriate /not/ to close a stream. For
instance, if you have a decorator stream (such a BufferedOutputStream) wrapped
around a FileStream, and you want to stop using buffering without closing the
underlying file, then you would flush() the BufferedOutputStream, and then
discard the reference to it without close()ing that stream. Needless to say,
you should explicitly close() the underlying FileStream when you have finished
with it.

In this case, I am dealing primarily with ByteArrayOutputStreams, and it
seems counter-intuitive that they are consuming a lot of system resources
anyway. A little more digging into the API spec shows that

ByteArrayOutputStream is a bit of an oddity. And there is also a mistake in
the design which makes this more awkward that it should be.

If your code doesn't "know" that it is writing to a ByteArrayOutputStream (e.g.
you have utility code which could be creating a byte[] array or writing to a
file, or to the network -- and it doesn't care which), then clearly that code
should close() the stream (if it close()es it at all) no matter where the
stream is really writing to. If that is the case, then just don't worry about
close() not doing anything -- it's doing everything it /should/ do, and your
code will work correctly no matter what kind of stream you use.

OTOH, your code might "know" that it /is/ using a ByteArrayOutputStream.
That's quite possible too. In that case you should have the choice between two
sensible ways of coding the "unnecessary" clean-up, but Sun have messed up the
API. You can just say, well we don't need to close() this stream, so I won't
bother. A perfectly legitimate choice, and what I'd probably do myself.
Alternatively, you might want to future-proof your code a little, and close()
the stream explicitly, even though it's unnecessary, as a bit of insurance in
case you ever make that code more general. (Or maybe in response to company
coding standards, or because some automated code checking tool complains.)
That's a legitimate choice too, but Sun have unfortunately declared
ByteArrayOutputStream.close() to throw IOException even though it doesn't do
so. That error[*] means that even though your current code may know that it is
using a ByteArrayOutputStream (i.e. the stream variable is declared with that
specific type rather than a more general one), it is /still/ necessary to add
code to handle an exception which cannot be thrown.

([*] And I believe it /is/ an error. A bit of mis-design, possibly caused by a
historical copy-and-paste mistake.)

So, I think that how you should handle this depends on how your code is
designed. If it is really /necessary/ that the variable in question holds a
ref to a ByteArrayOutputStream, then I wouldn't bother closing it at all.
Otherwise I wouldn't declare it with a more general type than
ByteArrayOutputStream and would close() it properly and would include proper
handling for the exception.

Oh, another option would be to create a trivial subclass of
ByteArrayOutputStream which overrode close() and guaranteed (to the compiler)
that it wouldn't throw anything.

-- chris

 

[VisionSet]

my advice would be the opposite. That close does nothing is but a
temporary side effect of the implementation. In some future
implementation or some subclass it could well do something important.
Streams get closed. It is part of the contract.

Not calling close violates the principle of encapsulation. Method
bodies are supposed to be treated like black boxes.

Fair enough Roedy, you are correct of course.

 

[Chris Uppal]

Fair enough Roedy, you are correct of course.

I don't think so; I think you were correct the first time (see my other post in
this thread). The effect of ByteArrayOutputStream.close() is /documented/ and
cannot change in a future release (or at least, is no more subject to change
than the existence of close() itself).

I would agree, though, that ByteArrayOutputStream ought to be final, since it
is making promises that subclasses could conceivably change.

-- chris

 

[VisionSet]

I don't think so; I think you were correct the first time (see my other post in
this thread). The effect of ByteArrayOutputStream.close() is /documented/ and
cannot change in a future release (or at least, is no more subject to change
than the existence of close() itself).

I would agree, though, that ByteArrayOutputStream ought to be final, since it
is making promises that subclasses could conceivably change.

I suppose it comes down to the situation in which you use it. If you write
library code of any kind or are *only* going to be an end user. If the
latter then if you don't call close you should make sure you make it
impossible for someone to switch stream implementation.