RFR: 8264760: JVM crashes when two threads encounter the same resolution error [v4]

[David Holmes]

On Fri, 23 Apr 2021 03:02:52 GMT, Wang Huang <whuang at openjdk.org> wrote:

>> As shown in JDK-8264760, I changed notes with @dholmes-ora and only fixed this issue by deleting the assert. ~~The other whole bigger issue will be fixed in the other issue.~~
>
> Wang Huang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix bugs

Hi,

Sorry for the delayed re-review. There are still a bunch of nits with the test logic that I'd like to see addressed. Sorry I should have flagged them earlier.

Thanks,
David

test/hotspot/jtreg/runtime/Nestmates/membership/TestNestHostErrorWithMultiThread.java line 29:

> 27:  * @summary JVM crashes when two threads encounter the same resolution error
> 28:  *
> 29:  * @library /test/lib

You don't use the test library

test/hotspot/jtreg/runtime/Nestmates/membership/TestNestHostErrorWithMultiThread.java line 33:

> 31:  * @compile HostNoNestMember.jcod
> 32:  *
> 33:  * @run main/othervm TestNestHostErrorWithMultiThread

No need for othervm in this test.

test/hotspot/jtreg/runtime/Nestmates/membership/TestNestHostErrorWithMultiThread.java line 42:

> 40:   public static void main(String args[]) {
> 41:     CountDownLatch latch1 = new CountDownLatch(1);
> 42:     CountDownLatch latch2 = new CountDownLatch(2);

Please rename latch1 as run, and latch2 as started. The names should aid in understanding the logic of the synchronization.

test/hotspot/jtreg/runtime/Nestmates/membership/TestNestHostErrorWithMultiThread.java line 45:

> 43: 
> 44:     new Thread(new Test(latch1, latch2)).start();
> 45:     new Thread(new Test(latch1, latch2)).start();

Nit: you only need a single Runnable instance as it is stateless. The Runnable could even be defined inline rather than as a separate named class.

test/hotspot/jtreg/runtime/Nestmates/membership/TestNestHostErrorWithMultiThread.java line 51:

> 49:       latch2.await();
> 50:       latch1.countDown();
> 51:     } catch (InterruptedException e) {}

Catch block should have:

throw new Error("Unexpected interrupt");

test/hotspot/jtreg/runtime/Nestmates/membership/TestNestHostErrorWithMultiThread.java line 70:

> 68:         latch1.await();
> 69:         HostNoNestMember h = new HostNoNestMember();
> 70:         h.test();

After the call to test you should have:

throw new Error("IllegalAccessError was not thrown as expected");

test/hotspot/jtreg/runtime/Nestmates/membership/TestNestHostErrorWithMultiThread.java line 79:

> 77:         }
> 78:         System.out.println("OK - got expected exception: " + expected);
> 79:       } catch (InterruptedException e) {}

Catch block should have:

throw new Error("Unexpected interrupt");

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

Changes requested by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3392