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

[Peter Levart]

Hi Sherman,

On 10/31/17 00:32, Xueming Shen wrote:
> On 10/30/2017 01:34 PM, Peter Levart wrote:
>>
>> The Inflater and Deflater now look fine (except you don't have to 
>> check for cleanable != null any more in Inflater.end()).
>>
>> But what shall we do with ZipFile?
>>
>
> Peter,
>
> The fundamental issue here is the gap between the resource allocation
> and the cleaner registration, as far as these two are not an atomic
> operation, it's hard to handle it in the normal use scenario. You might be
> able to workaround it by registering the cleaner first and then send the
> resource reference into the cleaner, as we are going to do for the 
> deflater/
> inflater. 

RIght. Finalization, as designed, usually guides the programmer to not 
mess this order. The finalizable instance is registered as part of 
Object.<init> constructor call. When resource allocation happens in the 
subclass constructor(s), all is well, because code of subclass 
constructors is executed after Object.<init> successfully returns. 
Subclassing PhantomCleanable<T> has this same good property when 
resource allocation is performed in the PhantomCleanable's subclass 
constructor(s).

> But it appears to be a more general issue and in some circumstance
> even the "register-first-allocate-assign-in-later" approach might fail.
> For example, you have N resource to allocate, something goes wrong
> between the first and the Nth resource allocation completed/or simply say
> before you can reach the "register()". 

If you really do "register-first-allocate-assign-in-later" then there is 
no problem as those resources that are assigned are the resources that 
were allocated. The cleanup function should selectively deallocate the 
resources that were assigned (using null or some special default value 
to identify resources that weren't).

> The question then I would like to ask
> again is if this is really something we want to support/cover with the 
> Cleaner
> mechanism. It doesn't appear to be supported under the finalize() 
> mechanism.
> For example, if someone throws out an exception inside the constructor, in
> normal use scenario, regardless you catch or not catch that exception
> (outside the constructor), the finalize() method is probably not going to
> be called by the finalizer for this partially/incompletely constructed 
> object,
> if you don't purposely do something special to publish the object. 


Wrong. Try this example:


public class TestFinalization {

     final int resource;

     public TestFinalization() {
         if (true) {
             // by the time this is thrown, the object is already registered
            // for finalization....
             throw new RuntimeException();
         }
         resource = 42;
     }

     @SuppressWarnings("deprecation")
     @Override
     protected void finalize() {
         System.out.println("finalize() called: resource=" + resource);
     }

     public static void main(String[] args) throws Exception {
         try {
             new TestFinalization();
         } catch (RuntimeException e) {
             System.out.println("Exception caught");
         }
         System.gc();
         Thread.sleep(1000L);
     }
}


It prints the following:

Exception caught
finalize() called: resource=0

> Sure
> arguably It appears to be out of the scope of Cleaner API and the only 
> thing
> needs to be addressed here is whether or not register(..) operation 
> fails and
> throws an Error. But it might be helpful to see if there is any 
> different opinion
> before going further?

I don't see much difference between finalization and Cleaner mechanism. 
The main difference is that with finalization, the object that is being 
tracked is also the object that contains the state and logic needed for 
cleanup. Cleaner API decouples those two aspects. The low-level 
(extending PhantomCleanable<T>) API also encourages the correct order of 
things: 1st register then allocate. If we either expose the low-level 
API to the public or make some additions to the high-level API to 
encourage the same, all will be well.

Regards, Peter

>
> Sherman
>
>
>
>
>