On Sat, 1 Jun 2024 21:11:26 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> The fix removes finalization cleanup from vmTestbase.
>> The last to classes that use it are: DebugeeBinder and SocketIOPipe.
>> The DebugeeBinder is used in jdi and jdwp tests and is always linked with
>> debuggee process. So the DebugeeProcess.waitFor() is the good place to close
>> binder and free all it's resources.
>> The SocketIOPipe is used directly in AOD tests where it should be closed
>> after test execution.
>>
>> The OPipe (child of SocketIOPipe) also used in jdi and jdwp tests where it
>> is connected directly in tests. However is also connected with debuggee and
>> could be closed in DebugeeProcess.waitFor().
>>
>> The VMOutOfMemoryException001 test is fixed to release some memory after
>> throwing OOME so Sytem.exit() could complete successfully. Previously some
>> memory freed during VM shutdown hook.
>>
>> I verified that cleanup printed that corresponding 'close' method has been
>> already called before VM shutdown phase for debugger process.
>> Additionally, run all vmTestbase tests to verify there are no failures,
>
> Leonid Mesnik has updated the pull request incrementally with one additional
> commit since the last revision:
>
> fixed try/finally
test/hotspot/jtreg/vmTestbase/nsk/share/jpda/DebugeeProcess.java line 201:
> 199: try {
> 200: int exitCode = waitForDebugee();
> 201: return exitCode;
I don't think I've ever run across a try block with a return statement before,
especially when there is also a finally block. The reader is likely to miss the
fact that before the return is done the finally block is executed. It's also
odd because now there is no return statement at the end of the method. Although
the compiler is smart enough to recognize that this is ok, it is another point
of confusion for the reader. Any reason not to just instead do the return at
the end of the method?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19505#discussion_r1625109575
