RFR JDK-8170089: nsk/jdi/EventSet/resume/resume008: ERROR: suspendCounts don't match for : Common-Cleaner

[Chris Plummer]

Hi Gary,

Overall it looks good.

Is the EventHandler.isDisconnected() check needed?

In resume008a.java you removed a lot of empty lines. In some places it's 
fine, but the lines at 132 and 134 should have remained. Also, for the 
ones that were ok to remove, I don't see you doing the same thing in the 
other files. I think probably it's best to be consistent, and for this 
webrev probably best not to do them since it distracts too much from the 
important changes.

Seems like there is a lot of abstraction that could have been done with 
these tests to share a lot of code, but since so far that hasn't been 
done, probably not a good idea to start doing that with this fix. Do you 
think it's worth filing an enhancement request for?

thanks,

Chris

On 8/3/18 11:04 AM, Gary Adams wrote:
> Here is an updated webrev with the alternate solution implemented for
> resume 1 to 10. The debugger sets testCase variable in the debuggee
> when each test case completes in the debugger. By having the debuggee
> wait for the debugger to complete with test case 0, it avoids the 
> interference
> that occurs by proceeding to the breakpoint set in MethodForCommunication
> before the debugger has compared expected suspend counts.
>
>   Webrev: http://cr.openjdk.java.net/~gadams/8170089/webrev.01/index.html
>
> On 7/17/18, 11:33 AM, Gary Adams wrote:
>> A race condition exists between the debugger and the debuggee.
>>
>> The first test thread is started with SUSPEND_NONE policy set.
>> While processing the thread start event the debugger captures
>> an initial set of thread suspend counts and resumes the
>> debuggee vm. If the debuggee advances quickly it reaches
>> the breakpoint set for methodForCommunication. Since the breakpoint
>> carries with it SUSPEND_ALL policy, when the debugger captures a second
>> set of suspend counts, it will not match the expected counts for
>> a SUSPEND_NONE scenario.
>>
>> The proposed fix introduces a yield in the debuggee test thread run 
>> method
>> to allow the debugger to get the expected sampled values.
>>
>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8170089
>>   Webrev: http://cr.openjdk.java.net/~gadams/8170089/webrev.00/
>>
>>
>> test/hotspot/jtreg/vmTestbase/nsk/share/jdi/TestDebuggerType1.java:
>> ...
>>    186        private void setCommunicationBreakpoint(ReferenceType 
>> refType, String methodName) {
>>    187            Method method = debuggee.methodByName(refType, 
>> methodName);
>>    188            Location location = null;
>>    189            try {
>>    190                location = method.allLineLocations().get(0);
>>    191            } catch (AbsentInformationException e) {
>>    192                throw new Failure(e);
>>    193            }
>>    194            bpRequest = debuggee.makeBreakpoint(location);
>>    195
>>
>>    196 bpRequest.setSuspendPolicy(EventRequest.SUSPEND_ALL);
>>
>>    197            bpRequest.putProperty("number", "zero");
>>    198            bpRequest.enable();
>>    199
>>    200            eventHandler.addListener(
>>    201                 new EventHandler.EventListener() {
>>    202                     public boolean eventReceived(Event event) {
>>    203                        if (event instanceof BreakpointEvent && 
>> bpRequest.equals(event.request())) {
>>    204                            synchronized(eventHandler) {
>>    205                                display("Received communication 
>> breakpoint event.");
>>    206                                bpCount++;
>>    207                                eventHandler.notifyAll();
>>    208                            }
>>    209                            return true;
>>    210                        }
>>    211                        return false;
>>    212                     }
>>    213                 }
>>    214            );
>>    215        }
>>
>>
>> test/hotspot/jtreg/vmTestbase/nsk/jdi/EventSet/resume/resume008.java:
>> ...
>>    140                    display("......--> vm.suspend();");
>>    141                    vm.suspend();
>>    142
>>    143                    display("        getting : Map<String, 
>> Integer> suspendsCounts1");
>>    144
>>    145                    Map<String, Integer> suspendsCounts1 = new 
>> HashMap<String, Integer>();
>>    146                    for (ThreadReference threadReference : 
>> vm.allThreads()) {
>>    147 suspendsCounts1.put(threadReference.name(), 
>> threadReference.suspendCount());
>>    148                    }
>>    149                    display(suspendsCounts1.toString());
>>    150
>>    151                    display("        eventSet.resume;");
>>    152                    eventSet.resume();
>>    153
>>    154                    display("        getting : Map<String, 
>> Integer> suspendsCounts2");
>>
>> This is where the breakpoint is encountered before the second set of 
>> suspend counts is acquired.
>>
>>    155                    Map<String, Integer> suspendsCounts2 = new 
>> HashMap<String, Integer>();
>>    156                    for (ThreadReference threadReference : 
>> vm.allThreads()) {
>>    157 suspendsCounts2.put(threadReference.name(), 
>> threadReference.suspendCount());
>>    158                    }
>>    159                    display(suspendsCounts2.toString());
>>
>