RFR for JDK-7027502: Test failures in demo/jvmti/hprof testcases, need to be othervm

[David Holmes]

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.yan at 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.
>>