On 9/01/2014 9:07 PM, Tristan Yan wrote:
Thank you Paul
I change turn to local variable as well.
http://cr.openjdk.java.net/~tyan/JDK-7027502/webrev.03/
<http://cr.openjdk.java.net/%7Etyan/JDK-7027502/webrev.03/>
Okay I think I get it now. Both MonitorTest and WaitersTest use the
Context class, so if both tests run in the same VM the second test will
see the static total_turns_taken and "turn" in the state they were left
from the first test - hence the second test will always fail. The bug
report suggests making the tests othervm to avoid the problem but
instead you have changed from using static state to instance state so
that there is no interference.
But you can't pass a reference to a simple int, for total_turns_taken,
hence you turned it into the only mutable Integer type: AtomicInteger.
I'm okay with this final form of the change. othervm would have been
simpler :) These are ugly tests even with your changes.
BTW:
107 Thread.yield();
108 Thread.sleep(sleepTime);
The yield() before the sleep is completely pointless.
David
-----
I am not sure I understand your second suggestion here, sum up
thread_turns of each Context(This is a fixed value) doesn't equal
total_turns_taken.
Regards
Tristan
On 01/09/2014 06:28 PM, Paul Sandoz wrote:
On Jan 9, 2014, at 10:52 AM, Tristan Yan <tristan....@oracle.com> wrote:
Can someone else give a second review on this.
In a comment the bug you state: "here total_turns_taken is a static
variable, it could affect by other tests" I don't quite know under
what test circumstances that can happen, but if so is the following
also an issue: 52 private final static TurnChecker turn = new
TurnChecker(-1); ? FWIW an alternative to using an AtomicInteger would
be for the main loop to sum up thread_turns of each Context, since
read/writes are all performed in a synchronized block. Paul.
