RFR: 8329351: Add runtime/Monitor/TestRecursiveLocking.java for recursive Java monitor stress testing

[Daniel D. Daugherty]

On Tue, 19 Nov 2024 13:09:22 GMT, Fredrik Bredberg <fbredberg at openjdk.org> wrote:

> @dcubed-ojdk is too busy, so I was asked to take over [https://github.com/openjdk/jdk/pull/18664](https://github.com/openjdk/jdk/pull/18664) and finish it.
> 
> That old PR contains all the details and also got some review comments, which I have tried to address in this new PR.
> 
> Tests run without failures on supported platforms.

Thumbs up. I have only editorial type questions/comments.

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 230:

> 228:             counter++;
> 229: 
> 230:             // Legacy mode has no lock stack. I.e. there is no limit

nit grammar/typography: s/. I.e. there/, i.e., there

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 238:

> 236:             }
> 237: 
> 238:             // We havn't reached the stack lock capasity (recursion

nit typo: s/capasity/capacity/

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 270:

> 268:         }
> 269: 
> 270:         // This test nests x recurcive locks of INNER, in x recursive

nit typo: s/recurcive/recursive/

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 271:

> 269: 
> 270:         // This test nests x recurcive locks of INNER, in x recursive
> 271:         // locks of OUTER.  The number x is taken from the max number

nit spacing: s/OUTER.  The/OUTER. The/

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 316:

> 314:                 } else {
> 315:                     // Second time we want to lock A, the lock stack
> 316:                     // looks like this [A, B].  Lightweight locking

nit spacing: s/B].  Lightweight/B]. Lightweight/

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 318:

> 316:                     // looks like this [A, B].  Lightweight locking
> 317:                     // doesn't allow interleaving ([A, B, A]), instead
> 318:                     // it inflates A and removed it from the lock

nit typo: s/removed/removes/

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 375:

> 373:             }
> 374:             // Implied else: for LM_MONITOR or LM_LIGHTWEIGHT it can be
> 375:             // either inflated or not point because A is not locked anymore

nit typo: s/either inflated or not point because/either inflated or not because/

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 399:

> 397:         System.err.println("where:");
> 398:         System.err.println("    n_secs  ::= > 0");
> 399:         System.err.println("            Default n_secs is 15.");

Based on L408 below, it looks like default n_secs is now `30`.

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 402:

> 400:         System.err.println("    mode    ::= 1 - outer and inner");
> 401:         System.err.println("            ::= 2 - alternate A and B");
> 402:         System.err.println("            Default mode is 1.");

Based on L407 below, it looks like default mode is now `2`.

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 503:

> 501:                     waiter.wait();
> 502:                 } catch (InterruptedException ie) {
> 503:                 }

Do you want to add this:

            // This should not happen.
            ie.printStackTrace();

here also?

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 541:

> 539:                     waiter.wait();
> 540:                 } catch (InterruptedException ie) {
> 541:                 }

Do you want to add this:

            // This should not happen.
            ie.printStackTrace();

here also?

test/hotspot/jtreg/runtime/Monitor/TestRecursiveLocking.java line 556:

> 554:                 waiter.wait();
> 555:             } catch (InterruptedException ie) {
> 556:             }

Do you want to add this:

            // This should not happen.
            ie.printStackTrace();

here also?

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

Marked as reviewed by dcubed (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22238#pullrequestreview-2446595738
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1849021244
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1849020338
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1849022916
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1849023663
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1849026085
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1849026539
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1849030340
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1849032905
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1849033663
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1849040443
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1849041350
PR Review Comment: https://git.openjdk.org/jdk/pull/22238#discussion_r1849042052