commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sebb (JIRA)" <>
Subject [jira] [Commented] (COMPRESS-357) BZip2CompressorOutputStream can affect output stream incorrectly
Date Mon, 13 Jun 2016 08:59:21 GMT


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

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

> BZip2CompressorOutputStream can affect output stream incorrectly
> ----------------------------------------------------------------
>                 Key: COMPRESS-357
>                 URL:
>             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

View raw message