RFR: 8305083: Remove finalize() from test/hotspot/jtreg/vmTestbase/nsk/share/ and /jpda that are used in serviceability/dcmd/framework tests [v4]

[Afshin Zafari]

On Sun, 7 May 2023 21:41:23 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits:
>> 
>>  - Merge branch 'master' into _8305083
>>  - 8305083: 8305083: Remove finalize() from test/hotspot/jtreg/vmTestbase/nsk/share/ and /jpda that are used in serviceability/dcmd/framework tests
>>  - 8305083: Remove finalize() from test/hotspot/jtreg/vmTestbase/nsk/share/ and /jpda that are used in serviceability/dcmd/framework tests
>>  - 8305083: Remove finalize() from test/hotspot/jtreg/vmTestbase/nsk/share/ and /jpda that are used in serviceability/dcmd/framework tests
>
> test/hotspot/jtreg/vmTestbase/nsk/share/Finalizable.java line 30:
> 
>> 28:  * Finalizable interface allows <tt>Finalizer</tt> to perform finalization of an object.
>> 29:  * Each object that requires finalization at VM shutdown time should implement this
>> 30:  * interface and call the <tt>registerClenup</tt> to activate a <tt>Finalizer</tt> hook.
> 
> Typo: Clenup

Corrected.

> test/hotspot/jtreg/vmTestbase/nsk/share/Finalizable.java line 61:
> 
>> 59:      */
>> 60:     default public void registerCleanup() {
>> 61:        // install finalizer to print errors summary at exit
> 
> This was moved from Log but isn't appropriate for the interface. No need to say anything here.

Done.

> test/hotspot/jtreg/vmTestbase/nsk/share/Finalizable.java line 65:
> 
>> 63:        finalizer.activate();
>> 64: 
>> 65:        // register the cleanup method to be called when this Log instance becomes unreachable.
> 
> Remove "Log" - actually again this is not needed. You can see what the method does and it is already documented in the doc comment.

Done.

> test/hotspot/jtreg/vmTestbase/nsk/share/FinalizableObject.java line 37:
> 
>> 35:     /**
>> 36:      * All instances of this class, should implement their own cleanup method
>> 37:      * to clean appropriately the objects they used.
> 
> "instances" don't implement methods. Suggestion:
> 
> "Subclasses should override this method to provide the specific cleanup actions that they need."

Suggested text is replaced.

> test/hotspot/jtreg/vmTestbase/nsk/share/MainWrapper.java line 50:
> 
>> 48:         finalizableObject.registerCleanup();
>> 49: 
>> 50: 
> 
> Extra blank line not needed

Done.

> test/hotspot/jtreg/vmTestbase/nsk/share/jpda/BindServer.java line 410:
> 
>> 408:      *
>> 409:      * This is replacement of the deprecated finalize() and is called
>> 410:      * when this instance becomes unreachable.
> 
> I don't think this is needed.

Done.

> test/hotspot/jtreg/vmTestbase/nsk/share/jpda/DebugeeBinder.java line 555:
> 
>> 553:      *
>> 554:      * This is replacement of the finalize() method and is called when this
>> 555:      * instance becomes unreachable.
> 
> Again not needed.

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13420#discussion_r1188303150
PR Review Comment: https://git.openjdk.org/jdk/pull/13420#discussion_r1188304787
PR Review Comment: https://git.openjdk.org/jdk/pull/13420#discussion_r1188305029
PR Review Comment: https://git.openjdk.org/jdk/pull/13420#discussion_r1188305875
PR Review Comment: https://git.openjdk.org/jdk/pull/13420#discussion_r1188306333
PR Review Comment: https://git.openjdk.org/jdk/pull/13420#discussion_r1188307967
PR Review Comment: https://git.openjdk.org/jdk/pull/13420#discussion_r1188308771