RFR: 8301684: Fix test code to not get finalizer deprecation warnings [v2]

David Holmes dholmes at openjdk.org
Wed Mar 15 05:19:26 UTC 2023


On Tue, 14 Mar 2023 17:18:12 GMT, Afshin Zafari <duke at openjdk.org> wrote:

>> To replace the finalizer use in the code, the `Cleaner` approach is used as stated in [Oracle doc on deprecated finalize() method](https://docs.oracle.com/javase/10/docs/api/java/lang/Object.html#finalize()).
>> Briefly:
>> 1.  An instance of `Cleaner` (`java.lang.ref`) is created.
>> 2. Using the `Cleaner`, an object is registered with a `Runnable` callback that is notified when the object is no longer reachable (GC'ed).
>> 3. Write code in the callback to do other cleanings.
>> 
>> 
>> Cleaner c = new Cleaner();
>> Cleanable cleanable = c.register(an_obj, a_runnable);
>> ...
>>   //JVM notifies by calling the 
>>     a_runnable.run() {...}
>> 
>> ### Tests
>> local: 
>>      vmTestbase/nsk/monitoring/stress/classload
>>       vmTestbase/nsk/jvmti/AttachOnDemand
>>       vmTestbase/nsk/jdi/ObjectReference/referringObjects
>>       vmTestbase/nsk/sysdict
>> mach5: tier 1-5
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8301684: Fix test code to not get finalizer deprecation warnings

This looks much simpler/cleaner - thanks!

Just a few nits with comments. Nice try with replacing "finalize" by "reclaim" but it doesn't quite work grammatically in the comments.

test/hotspot/jtreg/vmTestbase/nsk/share/ClassUnloader.java line 52:

> 50:  * After setting the class loader to null, if within a timeout no notification
> 51:  * of its reclaim is received, class is considered still loaded and
> 52:  * <code>unloadClass()</code> method returns <i>false</i>.

All of the existing commentary in this test is somewhat lacking in terms of grammatical correctness, but we can at least make improvements to the new text. I suggest the following:

 * A class is eligible for unloading if its class loader has been reclaimed.
 * A Cleaner is used to inform the main test code when the class loader
 * becomes unreachable and is reclaimed.
 * If, after setting the class loader to null, no notification that it has become
 * reclaimed is received within the timeout interval, then the class is considered
 * to still be loaded and <code>unloadClass()</code> returns <i>false</i>.

test/hotspot/jtreg/vmTestbase/nsk/share/ClassUnloader.java line 80:

> 78: 
> 79:     /**
> 80:      * Whole amount of time in milliseconds to wait for class loader reclaim.

s/reclaim/to be reclaimed/

test/hotspot/jtreg/vmTestbase/nsk/share/ClassUnloader.java line 85:

> 83: 
> 84:     /**
> 85:      * Piece of time in milliseconds to wait in a loop for class loader reclaim.

Suggestion:

* Sleep time in milliseconds for the loop waiting for the class loader to be reclaimed.
* ```

test/hotspot/jtreg/vmTestbase/nsk/share/ClassUnloader.java line 90:

> 88: 
> 89:     /**
> 90:      * Has class loader been is_reclaimed or not.

s/is_reclaimed/reclaimed/

test/hotspot/jtreg/vmTestbase/nsk/share/ClassUnloader.java line 141:

> 139:         classObjects.removeAllElements();
> 140: 
> 141:         // When customClassLoader becomes unreachable, the lambda is called by JVM/GC.

Suggestion:

// Register a Cleaner to inform us when the class loader has been reclaimed.

test/hotspot/jtreg/vmTestbase/nsk/share/ClassUnloader.java line 157:

> 155:         classObjects.removeAllElements();
> 156: 
> 157:         // When customClassLoader becomes unreachable, the lambda is called by JVM/GC.

Suggestion:

// Register a Cleaner to inform us when the class loader has been reclaimed.

test/hotspot/jtreg/vmTestbase/nsk/share/ClassUnloader.java line 277:

> 275:         }
> 276: 
> 277:         // class loader has not been is_reclaimed

s/is_reclaimed/reclaimed/

-------------

Changes requested by dholmes (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12968


More information about the hotspot-runtime-dev mailing list