RFR JDK-8170089: nsk/jdi/EventSet/resume/resume008: ERROR: suspendCounts don't match for : Common-Cleaner
Gary Adams
gary.adams at oracle.com
Mon Aug 6 12:27:36 UTC 2018
I'll need a sponsor for the patch attached.
On 8/6/18, 7: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.
> 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());
>>>>
>>>
>>
>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 8170089.patch
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180806/c818e48d/8170089-0001.patch>
More information about the serviceability-dev
mailing list