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());
