RFR: 7902982: jcstress: Add samples for some mutex algorithms

[Aleksey Shipilev]

On Mon, 21 Jun 2021 15:19:13 GMT, mmirwaldt for openjdk <github.com+86246875+mmirwaldt-openjdk at openjdk.org> wrote:

> I have implemented 3 more samples:
> *) the NoAlgorithm sample should show users of JCStress how they can define a critical section in a simple way
> *) one sample for the Peterson's algorithm 
> *) one sample for the Dekker's algorithm 
> I have translated the pseudo code implementations of the English wikipedia articles into Java.
> I have also tried out those examples: they compile and they run without any problems.

I think OCA had cleared. More comments.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurreny/mutex/Mutex_01_NoAlgorithm.java line 2:

> 1: /*
> 2:  * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved.

Should be "Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved."

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurreny/mutex/Mutex_01_NoAlgorithm.java line 49:

> 47:      */
> 48:     @JCStressTest
> 49:     @Outcome(id = {"1, 1", "1, 2", "2, 1", "2, 2"}, expect = ACCEPTABLE, desc = "Both actors can enter the critical section one by one but must not")

But must not what? The description seems to end prematurely.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurreny/mutex/Mutex_01_NoAlgorithm.java line 51:

> 49:     @Outcome(id = {"1, 1", "1, 2", "2, 1", "2, 2"}, expect = ACCEPTABLE, desc = "Both actors can enter the critical section one by one but must not")
> 50:     @State
> 51:     public static class NoAlgorithm {

Since there is only one test case in this file, you can move everything to the top class.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurreny/mutex/Mutex_01_NoAlgorithm.java line 61:

> 59:             } else {
> 60:                 r.r1 = 2;
> 61:             }

This kind of code introduces its own synchronization, so it might hide some outcomes. I believe you want to demonstrate the "already taken" detection code works? If so, I suggest doing it this way:


boolean taken1, taken2;

@Actor
public void actor1(ZZ_Result r) {
    taken1 = true;
    r.r1 = taken2;
    taken1 = false;
}

@Actor
public void actor2(ZZ_Result r) {
    taken2 = true;
    r.r2 = taken1;
    taken2 = false;
}

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurreny/mutex/Mutex_02_PetersonAlgorithm.java line 2:

> 1: /*
> 2:  * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved.

Same.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurreny/mutex/Mutex_02_PetersonAlgorithm.java line 52:

> 50:     @Outcome(id = {"1, 1"}, expect = ACCEPTABLE, desc = "Both actors could enter the critical section")
> 51:     @Outcome(id = {"1, 2", "2, 1", "2, 2"}, expect = FORBIDDEN, desc = "At least one actor couldn't enter the critical section")
> 52:     @Outcome(id = {"0, 0", "0, 1", "1, 0", "0, 2", "2, 0"}, expect = FORBIDDEN, desc = "At least one actor hang up in the loop")

You can drop `id` here, which would make this `@Outcome` to capture all other cases.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurreny/mutex/Mutex_02_PetersonAlgorithm.java line 59:

> 57:         private final AtomicInteger turn = new AtomicInteger();
> 58:         private final AtomicBoolean isInCriticalSection = new AtomicBoolean();
> 59: 

Again, single test, can move to the top class.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurreny/mutex/Mutex_02_PetersonAlgorithm.java line 64:

> 62:             flagForActor1.set(true);
> 63:             turn.set(2);
> 64:             while (flagForActor2.get() && turn.get() == 2) ;

Here and later. Excess whitespace before ` ;`?

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurreny/mutex/Mutex_02_PetersonAlgorithm.java line 70:

> 68:             } else {
> 69:                 r.r1 = 2;
> 70:             }

Here and later. Replace with new code for "taken" detection?

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurreny/mutex/Mutex_03_DekkerAlgorithm.java line 2:

> 1: /*
> 2:  * Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved.

Same.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurreny/mutex/Mutex_03_DekkerAlgorithm.java line 52:

> 50:     @Outcome(id = {"1, 1"}, expect = ACCEPTABLE, desc = "Both actors could enter the critical section")
> 51:     @Outcome(id = {"1, 2", "2, 1", "2, 2"}, expect = FORBIDDEN, desc = "At least one actor couldn't enter the critical section")
> 52:     @Outcome(id = {"0, 0", "0, 1", "1, 0", "0, 2", "2, 0"}, expect = FORBIDDEN, desc = "At least one actor hang up in one of the loops")

Again, can drop `id`.

jcstress-samples/src/main/java/org/openjdk/jcstress/samples/concurreny/mutex/Mutex_03_DekkerAlgorithm.java line 54:

> 52:     @Outcome(id = {"0, 0", "0, 1", "1, 0", "0, 2", "2, 0"}, expect = FORBIDDEN, desc = "At least one actor hang up in one of the loops")
> 53:     @State
> 54:     public static class DekkerAlgorithm {

Again, can move to top class.

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

Changes requested by shade (Committer).

PR: https://git.openjdk.java.net/jcstress/pull/85