On Sun, 24 Nov 2024 01:39:30 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> Hi all,
>> The newly added test
>> `test/hotspot/jtreg/serviceability/sa/TestJhsdbJstackPrintVMLocks.java`
>> intermittent fails `the return value of
>> "jdk.test.lib.apps.LingeredApp.getProcess()" is null`. This PR check
>> `getProcess()` is null or not before invoke `destroyForcibly`. Test-fix
>> only, the change has been verified locally, no risk.
>
> test/hotspot/jtreg/serviceability/sa/TestJhsdbJstackPrintVMLocks.java line 82:
>
>> 80: throw new RuntimeException("Not able to find lock");
>> 81: } finally {
>> 82: if (theApp.getProcess() != null) {
>
> This a really nice and usefule improvement, however doesn't fix the failure.
> It is unclear why the process is null. Some exception should have been
> thrown, but we don't see it. The exception from finally block override the
> real problem.
> So after your fix the test should start failing with real exception. If you
> can reproduce, investigate and fix the problem then you can make it in this
> PR.
> Otherwise, make sense to rename summary to something like:
> `Improve error handling TestJhsdbJstackPrintVMLocks.java
> `
> and investigate the problem later when failure has more info
Agree that this PR doesn't fix the failure of LingeredApp throw exception
during the intialization.
I have run this test standalone many times and can't reproduce the same
failure. Maybe the failure only occur when run with other tests.
The summary has been renamed according your advice.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22342#discussion_r1855350530
