RFR JDK-8170089: nsk/jdi/EventSet/resume/resume008: ERROR: suspendCounts don't match for : Common-Cleaner
Chris Plummer
chris.plummer at oracle.com
Mon Aug 6 17:56:16 UTC 2018
On 8/6/18 4:16 AM, Gary Adams wrote:
> On 8/3/18, 6:38 PM, Chris Plummer wrote:
>> Hi Gary,
>>
>> Overall it looks good.
>>
>> Is the EventHandler.isDisconnected() check needed?
> This just follows the pattern used in other calls to setValue.
I'm not seeing any other examples of this. Can you point me to them?
Isn't it expected that you will always be connected, and it will only be
disconnected if there is something very wrong with the execution of the
test? Not producing an error in that case could actually be misleading,
causing the test to fail with some sort of state related error rather
than some sort of exception indicating it was disconnected.
Chris
> No point in attempting the operation, if you know the
> connection was lost. An exception at this point could
> be misleading, if some other error has already occurred.
>>
>> 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.
> The original bug was reported against resume008, so more time was
> spent in that
> particular test, including some line wrapping changes. I will restore
> the blank lines
> you mentioned before producing a final patch. The other tests had
> observed failures
> also during testing. Applying the same change fixed those failures as
> well.
>>
>> 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?
> Reformatting or refactoring these older tests would be at best a P5.
> I don't think it's worth filing a bug, but as we fix bugs in these
> test it's
> worth some minimal amount of cleanup while we are in the code.
>>
>> 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());
>>>>
>>>
>>
>
More information about the serviceability-dev
mailing list