On Fri, 31 May 2024 19:55:56 GMT, Chris Plummer <cjplum...@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,
>
> test/hotspot/jtreg/vmTestbase/nsk/share/jpda/DebugeeProcess.java line 215:
>
>> 213: if (binder != null) {
>> 214: binder.close();
>> 215: }
>
> Won't this be skipped if there is an exception during `waitForDebugee` or
> `waitForRedirectors`?
Really, this all code is not going to work if any exception thrown.
The adding
The process is not destroyed, and
the 'waitForRedirectors' is skipped in exception in waitForDebugee happens.
The 'waitForRedirectors' tries to close 3 redirectors and fail to close all of
them if exception happens.
So all failure handling logic should be updated.
I changed it to try/finally so it close pipe, binder and destroy process.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19505#discussion_r1623281071
