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

[Daniel Fuchs]

Hi Roger,

I was wondering whether URLJarFile::finalize could be simply removed.

The change you are proposing looks strange to me: if this method
remains, then either the method should continue to call close() or
it should call super.finalize(), shouldn't it?

Since this is a sun.net class then maybe simply removing it would
be the safest thing...

best regards,

-- daniel

On 05/01/16 22:16, Roger Riggs wrote:
> 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
>>>
>>>
>>>
>>>
>>
>