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

Chris Plummer chris.plummer at oracle.com
Wed Jul 18 18:50:49 UTC 2018


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

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.

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 resume008 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