[ 
https://issues.apache.org/jira/browse/COMPRESS-357?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15325493#comment-15325493
 ] 

Sebb commented on COMPRESS-357:
-------------------------------

I understand, but AFAIK replacing synch. with a volatile changes the behaviour 
wrt. safe publication.

If the user code fails to run finish, then it won't set closed=true and 
finalize will attempt to run finish.
For finalize to work, it needs to have access to the latest values of other 
variables which have been updated in another thread.

In fact this is exactly the problem detailed in the description - the variable 
was not published safely.

The code now publishes 'closed' OK, so finalize won't run finish a second time, 
but what about other variables that finalize may need to run finish 
successfully?

> 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)

Reply via email to