I have a method which accepts an InputStream
(of binary data) and serializes it to XML. In order to do so, it wraps the stream with a base64 encoder and a Reader
to convert it to character data. However, since the InputStream
is passed in as a parameter, I would consider it a harmful side effect to close the stream, and the contract for Reader.close()
says it would do just that. If I don't close the reader, the compiler warns me that I have a
Resource leak: reader is never closed
So, I can add a @SuppressWarnings( "resource" )
to the reader declaration, but is that the right thing to do? Am I missing something?
Here is the actual code:
/**
* Writes base64 encoded text read from the binary stream.
*
* @param binaryStream
* The binary stream to write from
* @return <code>this</code> XmlWriter (for chaining)
* @throws IOException
*/
public XmlWriter binary( InputStream binaryStream ) throws IOException {
Reader reader = new InputStreamReader(
new Base64InputStream( binaryStream, true, base64LineLength, base64LineSeparator.getBytes( charset ) ) );
int bufferSize = 2048;
int charsRead;
char[] buffer = new char[bufferSize];
while ( (charsRead = reader.read( buffer, 0, bufferSize )) >= 0 ) {
writer.write( buffer, 0, charsRead );
}
return this;
}
Closing the returned InputStream will close the associated socket.
Therefore, if we forget to close the stream, the underlying channel will remain open and then we would end up with a resource leak.
Streams have a BaseStream. close() method and implement AutoCloseable, but nearly all stream instances do not actually need to be closed after use. Generally, only streams whose source is an IO channel (such as those returned by Files. lines(Path, Charset)) will require closing.
Closes this stream and releases any system resources associated with it. If the stream is already closed then invoking this method has no effect. So, if you don't close(), system resources may be still associated with the reader which may cause memory leak.
If you are a happy Java 7 user, try this:
try(InputStream binaryStream = /* ... */) {
xmlWriter.binary(binaryStream);
}
and stream is closed for you. If you can't use Java 7, I agree that it's not the responsibility of binary()
method to close()
the stream. Just ignore the warning and don't let tools drive your design. It's fine.
As a last resort you can write a lightweight Reader
wrapper ignoring close()
, but I don't advice it as it makes following the program flow harder.
Also let Apache Commons IO help you with IOUtils.copy()
:
public XmlWriter binary( InputStream binaryStream ) throws IOException {
Reader reader = new InputStreamReader(
new Base64InputStream( binaryStream, true, base64LineLength, base64LineSeparator.getBytes( charset ) ) );
IOUtils.copy(reader, writer);
return this;
}
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