RFR JDK-8185582, Update Zip implementation to use Cleaner, not finalizers

[Peter Levart]

Hi Mandy, Sherman, Roger,

On 10/31/17 00:25, mandy chung wrote:
>
>
> On 10/30/17 1:49 PM, Peter Levart wrote:
>>
>> ...above example lends itself as a use case for the following 
>> equivalent alternative using internal low-level API where ZStreamRef 
>> becomes the Cleanable itself:
>>
>> class ZStreamRef extends PhantomCleanable<Object> {
>>
>>     private final LongConsumer end;
>>     private volatile long address;
>>
>>     ZStreamRef (Object referent, LongSupplier init, LongConsumer end) {
>>         // here the registration MUST happen as 1st thing - enforced 
>> by javac
>>         super(referent, CleanerFactory.cleaner());
>>         this.end = end;
>>         this.address = init.getAsLong();
>>     }
>>
>>     long address() {
>>         return address;
>>     }
>>
>>     @Override
>>     protected void performCleanup() {
>>         long addr = address;
>>         address = 0;
>>         if (addr != 0) {
>>             end.accept(addr);
>>         }
>>     }
>> }
>>
>
> I was thinking something along this line what could ensure the 
> "cleanable" object is allocated before allocating the resources that 
> the cleanable is responsible for clean up.   I think it'd be a good 
> RFE to improve the Cleaner API to address the OOM case.
>
> Mandy

I played a little with an idea of how such additional Cleaner API might 
look like. Here's what I came up with, together with how this would 
apply to ZipFile/Inflater/Deflater:

http://cr.openjdk.java.net/~plevart/jdk10-dev/8185582_ZIP.cleaner/webrev.02/

@Sherman:

While looking at how to apply this to ZipFile Cleaner(s), I noticed that 
Releaser, as proposed, does not have to include opened streams or cached 
inflaters. Why?

- opened streams keep a reference to the ZipFile, so ZipFile's Cleaner 
will not fire automatically until all opened streams and ZipFile become 
phantom-reachable. Even more than that, ZipFileInflaterInputStream's 
Cleaner keeps a reference to ZipFile in order to call 
ZipFile.releaseInflater. So all ZipFileInflaterInputStream(s) have to be 
closed 1st (either cleaned automatically or explicitly) so that their 
inflaters are released back to cache and ZipFileInflaterInputStream's 
cleanup functions are released. Only then may ZipFile become 
phantom-reachable. When Releaser is finally triggered automatically, 
there are no more open ZipFileInflaterInputStream(s) only 
ZipFileInputStream(s) may be still open, but already phantom-reachable. 
PhantomReference specification says:

  * <p> Suppose the garbage collector determines at a certain point in time
  * that an object is <a href="package-summary.html#reachability">
  * phantom reachable</a>.  At that time it will atomically clear
  * all phantom references to that object and all phantom references to
  * any other phantom-reachable objects from which that object is reachable.
  * At the same time or at some later time it will enqueue those 
newly-cleared
  * phantom references that are registered with reference queues.

...which means that when ZipFile's Cleaner finally fires automatically, 
all opened stream's Cleaners have already fired or are going to fire 
together in same "batch" with the ZipFile's Cleaner. No need for ZipFile 
to include opened streams in its automatic cleanup. It only has to do 
that in explicit close().

The same reasoning goes for cached inflaters. They are only reachable 
from ZipFile. So when ZipFile becomes phantom-reachable, so do cached 
Inflaters at the same time and are therefore cleaned in the same "batch".

The only resource that ZipFile needs to clean in its automatic cleanup 
function is the shared, globally cached, reference counted Source (zsrc) 
object. And this is what I did with this new proposed Cleaner API in 
above webrev.02.


So, what do you think of this new Cleaner API extension?

Regards, Peter