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

Roger Riggs Roger.Riggs at Oracle.com
Fri Jan 8 15:51:58 UTC 2016

Thanks for the review and comments:

While integrating the comments, some comments identified finalizers that 
were unneeded should be removed.
I split that into a separate bug, as they are not being replaced, just 
removed. [2]

The CleaningFactory is moved to jdk.internal.ref so it can be exported 
selectively to specific
OpenJDK modules.  The suggested refactoring of the subclassable 
XXXCleanables can be done separately
if it turns out to be useful outside the base module.

Webrev for shared CleaningFactory and its uses:

Webrev for removed finalizers:

Thanks, Roger

[1] https://bugs.openjdk.java.net/browse/JDK-8146028 Common Cleaner for 
finalization replacements in OpenJDK
[2] https://bugs.openjdk.java.net/browse/JDK-8146567 Remove dead code 
finalizer methods

On 1/6/2016 2:02 PM, Peter Levart wrote:
> On 01/05/2016 10:16 PM, 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
> Hi Roger,
> I briefly skimmed over the webrev, and found the following issue in 
> ProcessImpl:
>  420         // Register a cleaning function to close the handle
>  421         CleanerFactory.getCleaner().register(this, () -> 
> closeHandle(handle));
>  422
> ... 'handle' is an instance variable, which means that 'this' is 
> captured by the lambda. You will have to assign handle to a local var 
> and capture it instead to prevent 'this' to be captured.
> Regards, Peter
>> 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

More information about the core-libs-dev mailing list