You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Stefan Bodewig <bo...@apache.org> on 2018/06/28 07:54:09 UTC
[compress] close() and writing invalid streams
Hi all
https://issues.apache.org/jira/browse/COMPRESS-457 raises an issue that
I vaguely recall we've talked about in the past but I may be wrong.
Almost all our OutputStream close methods go along the lines of
public void close() throws IOException {
finishFormatSpecificStuff();
closeUnderlyingStreamsOrChannelsAndOtherResources();
}
as some formats need to write trailers in order to create valid
output. If for some reason finishFormatSpecificStuff() stuff fails the
underlying stream will not be closed leading to a resource leak - unless
you keep track of the underlying stream and close it yourself
(try-with-resources probably).
Do we want to do anything about this and if so what?
We could modify all close implementations to perform resource cleanup in
a finally block. Likewise we could add new (let's say unsafeClose())
methods that only perform resource cleanup. Or we could properly
document the behavior and ensure our examples contain resource cleanup
code.
I'm leaning toward "documenting" but want to ensure there are no leaks
the user cannot prevent. For example DeflateCompressorOutputStream
"end()"s the Deflater instance in a finally block while zip's
StreamCompressor holds a reference to a Deflater that may never get
closed.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] close() and writing invalid streams
Posted by Gary Gregory <ga...@gmail.com>.
I am saying that I prefer the option:
public void close() throws IOException {
try {
finishFormatSpecificStuff();
} finally {
closeUnderlyingStreamsOrChannelsAndOtherResources();
}
}
Gary
On Fri, Jun 29, 2018 at 10:45 AM Stefan Bodewig <bo...@apache.org> wrote:
> Hi Gary
>
> I'm afraid I don't really follow you here. close() doesn't open any
> resources so I'm not sure what try-with-resources can do here.
>
> Stefan
>
> On 2018-06-28, Gary Gregory wrote:
>
> > I'm for making it obvious and keeping clean ups localized, meaning add
> > try-with-resources blocks to all close() methods that need them.
>
>
> > On Thu, Jun 28, 2018 at 3:54 AM Stefan Bodewig <bo...@apache.org>
> wrote:
>
> >> Hi all
>
> >> https://issues.apache.org/jira/browse/COMPRESS-457 raises an issue that
> >> I vaguely recall we've talked about in the past but I may be wrong.
>
> >> Almost all our OutputStream close methods go along the lines of
>
> >> public void close() throws IOException {
> >> finishFormatSpecificStuff();
> >> closeUnderlyingStreamsOrChannelsAndOtherResources();
>
>
> >> as some formats need to write trailers in order to create valid
> >> output. If for some reason finishFormatSpecificStuff() stuff fails the
> >> underlying stream will not be closed leading to a resource leak - unless
> >> you keep track of the underlying stream and close it yourself
> >> (try-with-resources probably).
>
> >> Do we want to do anything about this and if so what?
>
> >> We could modify all close implementations to perform resource cleanup in
> >> a finally block. Likewise we could add new (let's say unsafeClose())
> >> methods that only perform resource cleanup. Or we could properly
> >> document the behavior and ensure our examples contain resource cleanup
> >> code.
>
> >> I'm leaning toward "documenting" but want to ensure there are no leaks
> >> the user cannot prevent. For example DeflateCompressorOutputStream
> >> "end()"s the Deflater instance in a finally block while zip's
> >> StreamCompressor holds a reference to a Deflater that may never get
> >> closed.
>
> >> Stefan
>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
Re: [compress] close() and writing invalid streams
Posted by Stefan Bodewig <bo...@apache.org>.
Hi Gary
I'm afraid I don't really follow you here. close() doesn't open any
resources so I'm not sure what try-with-resources can do here.
Stefan
On 2018-06-28, Gary Gregory wrote:
> I'm for making it obvious and keeping clean ups localized, meaning add
> try-with-resources blocks to all close() methods that need them.
> On Thu, Jun 28, 2018 at 3:54 AM Stefan Bodewig <bo...@apache.org> wrote:
>> Hi all
>> https://issues.apache.org/jira/browse/COMPRESS-457 raises an issue that
>> I vaguely recall we've talked about in the past but I may be wrong.
>> Almost all our OutputStream close methods go along the lines of
>> public void close() throws IOException {
>> finishFormatSpecificStuff();
>> closeUnderlyingStreamsOrChannelsAndOtherResources();
>> as some formats need to write trailers in order to create valid
>> output. If for some reason finishFormatSpecificStuff() stuff fails the
>> underlying stream will not be closed leading to a resource leak - unless
>> you keep track of the underlying stream and close it yourself
>> (try-with-resources probably).
>> Do we want to do anything about this and if so what?
>> We could modify all close implementations to perform resource cleanup in
>> a finally block. Likewise we could add new (let's say unsafeClose())
>> methods that only perform resource cleanup. Or we could properly
>> document the behavior and ensure our examples contain resource cleanup
>> code.
>> I'm leaning toward "documenting" but want to ensure there are no leaks
>> the user cannot prevent. For example DeflateCompressorOutputStream
>> "end()"s the Deflater instance in a finally block while zip's
>> StreamCompressor holds a reference to a Deflater that may never get
>> closed.
>> Stefan
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [compress] close() and writing invalid streams
Posted by Gary Gregory <ga...@gmail.com>.
I'm for making it obvious and keeping clean ups localized, meaning add
try-with-resources blocks to all close() methods that need them.
Gary
On Thu, Jun 28, 2018 at 3:54 AM Stefan Bodewig <bo...@apache.org> wrote:
> Hi all
>
> https://issues.apache.org/jira/browse/COMPRESS-457 raises an issue that
> I vaguely recall we've talked about in the past but I may be wrong.
>
> Almost all our OutputStream close methods go along the lines of
>
> public void close() throws IOException {
> finishFormatSpecificStuff();
> closeUnderlyingStreamsOrChannelsAndOtherResources();
> }
>
> as some formats need to write trailers in order to create valid
> output. If for some reason finishFormatSpecificStuff() stuff fails the
> underlying stream will not be closed leading to a resource leak - unless
> you keep track of the underlying stream and close it yourself
> (try-with-resources probably).
>
> Do we want to do anything about this and if so what?
>
> We could modify all close implementations to perform resource cleanup in
> a finally block. Likewise we could add new (let's say unsafeClose())
> methods that only perform resource cleanup. Or we could properly
> document the behavior and ensure our examples contain resource cleanup
> code.
>
> I'm leaning toward "documenting" but want to ensure there are no leaks
> the user cannot prevent. For example DeflateCompressorOutputStream
> "end()"s the Deflater instance in a finally block while zip's
> StreamCompressor holds a reference to a Deflater that may never get
> closed.
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>