RFR 9: JDK-8146028 : Common Cleaner for finalization replacements in java.base

[Roger Riggs]

Hi Daniel,

webrev updated to revert changes to MeteredStream as too risky.

http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/index.html

On 1/5/2016 1:45 PM, Daniel Fuchs wrote:
> Hi Roger,
>
> Some early feedback:
>
> I see that prior to your changes, MeteredStream.close() would
> be called by finalize.
> This will no longer be the case after
> http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/index.html
In the case where finalize would call close(), there can be no 
queuedForCleanup activity
because there can be no strong reference from the KeepAliveStreamCleaner 
thread
or the queue that holds the pending cleanups.
It cannot be in the Cache of connections being kept alive or the thread 
that keeps them alive.

It might be possible that the underlying connection is still open; but 
there is no advantage
of trying to drain it.  There is the possibility of resurrecting it by 
virtue of deciding that
it should be queued for cleanup and starting a new thread to do the cleanup.

All in all, there is a risk of misunderstanding the dynamic behavior and 
it would be safer
to leave this using finalize.

>
> I see that MeteredStream has a subclass (KeepAliveStream) that
> overrides close() - so KeepAliveStream probably requires a cleanup
> as well - doesn't it?
>
> Finally I'd suggest making the variable 'pi' final - since you're
> passing that to the lambda you don't want to allow its value to
> be modified afterwards (fortunately it's not - and I believe it
> should be good to ensure it won't be).
ok, it would not go unnoticed, the compiler should complain if the 
variable is no effectively final.

Thanks, Roger

>
> I haven't looked at the other classes yet...
>
> best regards,
>
> -- daniel
>
> On 05/01/16 19:24, Roger Riggs wrote:
>> The follow on work to adding the Cleaner is to replace uses of
>> finalization with uses of the Cleaner.
>> For the 'easy' cases in the java.base module, it is useful to introduce
>> a private Cleaner for the
>> java.base module.  It is proposed to be held weakly, to allow it to
>> terminate on a lightly loaded
>> system.
>>
>> Webrev for Review:
>> http://cr.openjdk.java.net/~rriggs/webrev-cleaning-factory-8146028/
>>
>>
>> The 2nd step is using the Cleaner.
>>   - Empty finalize methods should (I think) be removed; but since they
>> are part of the public spec
>>     the process needs two full releases; so the proposal is to deprecate
>> them first.
>>     (The JEP 277 necessary changes will be updated when JEP 277
>> semantics are finalized)
>>
>> - In a few cases, the code in the finalizer is redundant and if removed
>> would allow
>>    an optimization of the handling of the finalizer and future removal
>> of the finalize method.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rriggs/webrev-cleaning-finalizers/index.html
>>
>> Thanks for comments and suggestions,  Roger
>>
>>
>>
>>
>