[
https://issues.apache.org/jira/browse/COMPRESS-357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15327027#comment-15327027
]
Sebb commented on COMPRESS-357:
-------------------------------
I agree about dropping the code.
However, rather than eliminate the finalize method entirely, maybe change it to
log/print a warning that the stream has not been properly closed.
This goes some way towards supporting the previously advertised functionality -
but of course only for cases where the finalizer is actually called.
Regarding synch. of flush() and finish(): that isn't sufficient to solve the
safe publication issue.
Happens-before ordering depends on using the same lock for the writer and
reader threads.
Lock release happens-before lock acquire, and actions (e.g. writes) in a single
thread happen-before actions that come later in the program order.
Unless all methods called by the main thread are synch. then the main thread
can write variables after the last shared lock.
There are various ways to ensure the fields are safely published.
However as has been pointed out, finalize() may never get run, so app code
cannot rely on finish() being called.
So it does not matter if finish() does not get called even if the finalize()
method does get run.
> 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)
