[
https://issues.apache.org/jira/browse/COMPRESS-357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15326728#comment-15326728
]
Richard Shapiro commented on COMPRESS-357:
------------------------------------------
I just want to point out that the real problem with this is the presence of a
finalize() method. Any code that requires that finalize() be run is almost
certainly incorrect, as you can never rely on it ever being called. Since the
class is thread-unsafe, there is in practice no additional performance cost to
synchronizing both flush() and finish(), since if the lock isn't essentially
free, you aren't using the class correctly either (i.e., you'd better be
calling write() and flush() inside some other synchronized block, or else
guarantee they are on the same thread).
The other thing to remember is that if it's incorrect, it doesn't matter how
fast something is, and bzip2 is already so slow that I doubt you will ever see
the synchronization cost even if you do multi-thread it.
By far the most correct solution is to get rid of finish().
> BZip2CompressorOutputStream can affect output stream incorrectly
> ----------------------------------------------------------------
>
> Key: COMPRESS-357
> URL: https://issues.apache.org/jira/browse/COMPRESS-357
> Project: Commons Compress
> Issue Type: Bug
> Components: Compressors
> Affects Versions: 1.9, 1.11
> Environment: multithreaded
> Reporter: Richard Shapiro
> Labels: easyfix
> Fix For: 1.12
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> BZip2CompressorOutputStream has an unsynchronized finished() method, and an
> unsynchronized finalize method. Finish checks to see if the output stream is
> null, and if it is not it calls various methods, some of which write to the
> output stream.
> Now, consider something like this sequence.
> BZip2OutputStream s = ...
> ...
> s.close();
> s = null;
> After the s = null, the stream is garbage. At some point the garbage
> collector call finalize(), which calls finish(). But, since the GC may be on
> a different thread, there is no guarantee that the assignment this.out = null
> in finish() has actually been made visible to the GC thread, which results in
> bad data in the output stream.
> This is not a theoretical problem; In a part of a large project I'm working
> on, this happens about 2% of the time.
> The fixes are simple
> 1) synchronize finish() or
>
> 2) don't call finish from finalize().
> A workaround is to derive a class and override the finalize() method.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
