RFR JDK-8170089: nsk/jdi/EventSet/resume/resume008: ERROR: suspendCounts don't match for : Common-Cleaner
Gary Adams
gary.adams at oracle.com
Wed Jul 18 19:45:03 UTC 2018
Answers below ...
On 7/18/18, 2:50 PM, Chris Plummer wrote:
> Hi Gary,
>
> Who does the resume for the breakpoint event?
>
> eventHandler.addListener(
> new EventHandler.EventListener() {
> public boolean eventReceived(Event event) {
> if (event instanceof BreakpointEvent &&
> bpRequest.equals(event.request())) {
> synchronized(eventHandler) {
> display("Received communication breakpoint
> event.");
> bpCount++;
> eventHandler.notifyAll();
> }
> return true;
> }
> return false;
> }
> }
> );
I believe you are looking for this sequence.
At the top of the loop a check is made if
resume() should be called "shouldRunAfterBreakpoint".
lines 96-99 is an early termination. And at the
bottom of the loop, line 240, is the normal
continue the test to the next case.
resume008.java :
...
94 for (int i = 0; ; i++) {
95
96 if (!shouldRunAfterBreakpoint()) {
97 vm.resume();
98 break;
99 }
100
101
102 display(":::::: case: # " + i);
103
104 switch (i) {
105
106 case 0:
107 eventRequest = settingThreadStartRequest (
108 SUSPEND_NONE,
"ThreadStartRequest1");
...
238
239 display("......--> vm.resume()");
240 vm.resume();
241 }
>
> Also:
>
>> 1. On a thread start event the debugee is suspended, line 141
> That's not true for the first ThreadStartEvent since SUSPEND_NONE was
> used.
The thread start event is set to SUSPEND_NONE for thread0, but when
the thread start event is observed the resume008 test suspends the vm
immediately after fetching the "number" property.
132 if ( !(newEvent instanceof ThreadStartEvent)) {
133 setFailedStatus("ERROR: new event is not
ThreadStartEvent");
134 } else {
135
136 String property = (String)
newEvent.request().getProperty("number");
137 display(" got new ThreadStartEvent with
propety 'number' == " + property);
138
139 display("......checking up on
EventSet.resume()");
140 display("......--> vm.suspend();");
141 vm.suspend();
>
> Chris
>
> On 7/18/18 4:52 AM, Gary Adams wrote:
>> There is nothing wrong with the breakpoint in methodForCommunication.
>> The test uses it to make sure the threads are each tested separately.
>> The breakpoint eventhandler just displays a message, increments a
>> counter
>> and returns.
>>
>> Let me step through resume008a the debugee to help clarify ...
>>
>> 1. The test thread is created and the synchronized break point is
>> observed. lines 101-102
>> 2. The thread is started. lines 104,135-137
>> 2a. The main thread blocks on a local object. lines 133, 139
>> 2b. The test thread is started. lines 137,
>> A run entered message is displayed, line 159
>> The main thread lock object is notified, line 167
>> 2b1. The main thread continues. line 167, 146
>> The next test thread is created. line 106
>> The synchronized breakpoint is observed, line 107
>> 2b2. A run exited message is displayed, line 169
>>
>> On the resume008 debugger side ...
>> 1. On a thread start event the debugee is suspended, line 141
>> 2. Messages are displayed and a first set of thread suspend counts
>> is acquired. lines 143-151
>> 3. The threads are resumed, line 152
>> --->
>> 4. Messages are displayed and a second set of thread suspend
>> counts is acquired. lines 154-159
>>
>> The way the test is written the expectation is the debugger steps
>> 2,3,4 will all happen
>> while the test thread is running.
>>
>> When the debugger resumes the debuggee threads (debugger step 3)
>> the debuggee continues from where it left off (debuggee steps
>> 2b,2b1,2b2)
>>
>> If we complete debuggee step 2b1 (line 107) before the debugger
>> completes step 4 line 159,
>> then the synchronized breakpoint will suspend the vm and the counts
>> will not match
>> for the SUSPEND_NONE test thread start.
>>
>> resume008a.java:
>>
>> 100 case 0:
>> 101 thread0 = new
>> Threadresume008a("thread0");
>> 102 methodForCommunication();
>> 103
>> 104 threadStart(thread0);
>> 105
>> 106 thread1 = new
>> Threadresume008a("thread1");
>> 107 methodForCommunication();
>> 108 break;
>>
>> ...
>> 135 static int threadStart(Thread t) {
>> 136 synchronized (waitnotifyObj) {
>> 137 t.start();
>> 138 try {
>> 139 waitnotifyObj.wait();
>> 140 } catch ( Exception e) {
>> 141 exitCode = FAILED;
>> 142 logErr(" Exception : " + e );
>> 143 return FAILED;
>> 144 }
>> 145 }
>> 146 return PASSED;
>> 147 }
>>
>> 149 static class Threadresume008a extends Thread {
>> ...
>> 157
>> 158 public void run() {
>> 159 log1(" 'run': enter :: threadName == " + tName);
>>
>> This is the proposed fix that will let the debugger complete it's second
>> acquisition of suspend counts while the test thread is still running.
>>
>> 160 // Yield, so the start thread event processing
>> can be completed.
>> 161 try {
>> 162 Thread.sleep(100);
>> 163 } catch (InterruptedException e) {
>> 164 // ignored
>> 165 }
>>
>> 166 synchronized (waitnotifyObj) {
>> 167 waitnotifyObj.notify();
>> 168 }
>> 169 log1(" 'run': exit :: threadName == " + tName);
>> 170 return;
>> 171 }
>> 172 }
>> 150
>> 151 String tName = null;
>> 152
>> 153 public Threadresume008a(String threadName) {
>> 154 super(threadName);
>> 155 tName = threadName;
>> 156 }
>> 157
>> 158 public void run() {
>> 159 log1(" 'run': enter :: threadName == " + tName);
>> 160 // Yield, so the start thread event processing
>> can be completed.
>> 161 try {
>> 162 Thread.sleep(100);
>> 163 } catch (InterruptedException e) {
>> 164 // ignored
>> 165 }
>> 166 synchronized (waitnotifyObj) {
>> 167 waitnotifyObj.notify();
>> 168 }
>> 169 log1(" 'run': exit :: threadName == " + tName);
>> 170 return;
>> 171 }
>> 172 }
>>
>>
>>
>> On 7/18/18, 2:38 AM, Chris Plummer wrote:
>>> Hi Gary,
>>>
>>> I've been having trouble following the control flow of this test.
>>> One thing I've stumbled across is the following:
>>>
>>> /* A debuggee class must define 'methodForCommunication'
>>> * method and invoke it in points of synchronization
>>> * with a debugger.
>>> */
>>> setCommunicationBreakpoint(debuggeeClass,"methodForCommunication");
>>>
>>> So why isn't this mode of synchronization good enough? Is it because
>>> it was not designed with the understanding that the debugger might
>>> be doing suspended thread counts, and suspending all threads at the
>>> breakpoint messes up the test?
>>>
>>> From what I can tell of the test, after the debuggee is started and
>>> hits the default breakpoint at the start of main(), the debugger
>>> then does a vm.resume() at the start of the for loop in the
>>> runTest() method. The debuggee then creates a thread and calls
>>> methodForCommunication(). There is already a breakpoint set there by
>>> the above debuggee code. It's unclear to me what happens as a result
>>> of this breakpoint and how it serves the test. Also unclear to me
>>> who is responsible for the vm.resume() after the breakpoint is hit.
>>>
>>> The debugger then requests all ThreadStart events, requesting that
>>> no threads be disabled when it is sent. I think you are saying that
>>> when the ThreadStart event comes in, sometimes we are at the
>>> methodForCommunication breakpoint, with all threads disabled, and
>>> this messes up the thread suspend counts. You want to delay 100ms so
>>> the breakpoint event can be processed and threads resumed again
>>> (although I can't see who actually resumes the thread after hitting
>>> the methodForCommunication breakpoint).
>>>
>>> Chris
>>>
>>> On 7/17/18 8: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