UniversalMediaServer
UniversalMediaServer copied to clipboard
Try to fix the resource leaks when streams are not closed properly
@valib I've tried searching for answers about these kinds of changes and I am still confused. My concern is that returning the resource within a try-with-resources block will return a closed resource.
I found https://stackoverflow.com/questions/22947755/try-with-resources-and-return-statements-in-java and of course https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html
It is probably best to design our code to return a result of a closeable stream, rather than the stream itself, but we have a lot of code that already does this pattern so maybe it is best for now to make sure the consumers of these methods which return autocloseable resources are closing them when they are finished using them. What do you think?
You usually ignore my warnings, but I still can't not say anything about this one. It will completely break everything, try with resources
will close the stream when it is exited. The Eclipse warnings aren't divine, there's a reason it's possible to suppress them.
Streams are often opened and consumed within the same method, and when that's the case, using try with resources
is a nice way to protect against unforeseen events that break the normal logic so that the resource isn't closed, but it cannot be used if the stream is to ever leave the method.
For input streams it would theoretically be possible to consume them locally, close the stream and pass on the content, but that requires reading the entire resource into memory. This isn't always a good idea, especially when we're dealing with media files. For output streams it's simply impossible to get around this unless you already have all the data you with to write at the time you open the stream.
In short, in more "complex" situations where you work with a stream over time or using multiple methods, you can't use try with resources
. You must instead be vigorous and making sure that there are no paths, regardless of how unlikely, where the resource isn't closed - including when exceptions are thrown at any location where that is possible. If you fail to do that, you have a leak...
@SubJunk @Nadahar if the try-with-resources returns closed resource or not it depends on the Java implementation. E.g. the InputStream.close() called by the try-with-resources to close does nothink see the comment in the Java source
/** * Closes this input stream and releases any system resources associated * with the stream. *
The
close
method ofInputStream
does * nothing. * * @exception IOException if an I/O error occurs. */ public void close() throws IOException {}
Maybe that is why it works for me. Anyway I rewrite some changes.
@valib Yes, it's true that not all implementations implements close()
, but most do. InputStream
is an abstract class, so methods are often overridden in implementations. File-based streams will certainly fail if you try to access them after closure, but memory-based streams probably don't care (like ByteArrayInputStream
for example).
This got stale, closing