ok, let change it to following
diff -r f489d5d13a51
test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java
---
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java
Thu Jul 23 16:36:44 2020 -0400
+++
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java
Tue Jul 28 23:17:22 2020 -0700
@@ -368,6 +368,7 @@
throw new JDITestRuntimeException("** default case 2
**");
}
+ vm.suspend();
if (eventRequest1 instanceof StepRequest) {
try {
log2("......eventRequest1.setEnabled(true);
IllegalThreadStateException is expected");
@@ -405,6 +406,7 @@
} catch ( InvalidRequestStateException e ) {
log2(" InvalidRequestStateException");
}
+ vm.resume();
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
}
webrev: http://cr.openjdk.java.net/~lmesnik/8244537/webrev.04/
<http://cr.openjdk.java.net/~lmesnik/8244537/webrev.04/>
Leonid
> On Jul 28, 2020, at 11:06 PM, [email protected] wrote:
>
> http://cr.openjdk.java.net/~lmesnik/8244537/webrev.03/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java.frames.html
>
> <http://cr.openjdk.java.net/~lmesnik/8244537/webrev.03/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java.frames.html>
>
> I'd suggest to simplify it:
>
> - insert suspend before the line:
> 371 if (eventRequest1 instanceof StepRequest) {
> - keep resume at the line:
> 411 vm.resume();
> - these lines are not needed:
> 389 vm.resume();
> 394 vm.suspend();
>
> Thanks,
> Serguei
>
>
> On 7/28/20 21:15, Leonid Mesnik wrote:
>> Included in webrev.03
>> http://cr.openjdk.java.net/~lmesnik/8244537/webrev.03
>> <http://cr.openjdk.java.net/~lmesnik/8244537/webrev.03>
>>
>> Leonid
>>
>>> On Jul 28, 2020, at 6:44 PM, [email protected]
>>> <mailto:[email protected]> wrote:
>>>
>>> http://cr.openjdk.java.net/~lmesnik/8244537/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java.frames.html
>>>
>>> <http://cr.openjdk.java.net/~lmesnik/8244537/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java.frames.html>
>>> 371 if (eventRequest1 instanceof StepRequest) {
>>> 372 try {
>>> 373 log2("......eventRequest1.setEnabled(true);
>>> IllegalThreadStateException is expected");
>>> 374 eventRequest1.setEnabled(true);
>>> 375 testExitCode = FAILED;
>>> 376 log3("ERROR: NO IllegalThreadStateException for
>>> StepRequest");
>>> 377 } catch ( IllegalThreadStateException e ) {
>>> 378 log2(" IllegalThreadStateException");
>>> 379 }
>>> 380 try {
>>> 381 log2("......eventRequest1.setEnabled(false);
>>> IllegalThreadStateException is not expected");
>>> 382 eventRequest1.setEnabled(false);
>>> 383 log2(" no IllegalThreadStateException for
>>> StepRequest");
>>> 384 } catch ( IllegalThreadStateException e ) {
>>> 385 testExitCode = FAILED;
>>> 386 log3("ERROR: IllegalThreadStateException");
>>> 387 }
>>> 388 }
>>> Above is one more case where suspend/resume is needed, I guess.
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 7/28/20 17:45, [email protected]
>>> <mailto:[email protected]> wrote:
>>>> http://cr.openjdk.java.net/~lmesnik/8244537/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003.java.frames.html
>>>>
>>>> <http://cr.openjdk.java.net/~lmesnik/8244537/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003.java.frames.html>
>>>> 288 case 0:
>>>> 289 thread1 = (ThreadReference)
>>>> 290
>>>> debuggeeClass.getValue(debuggeeClass.fieldByName(threadName1));
>>>> 291
>>>> 292 log2("......setting up StepRequest");
>>>> 293 eventRequest1 = eventRManager.createStepRequest
>>>> 294 (thread1, StepRequest.STEP_MIN,
>>>> StepRequest.STEP_INTO);
>>>> 295
>>>> 296 vm.suspend();
>>>> ...
>>>> 360 default:
>>>> 361 throw new JDITestRuntimeException("** default
>>>> case 2 **");
>>>> 362 }
>>>> 363 vm.resume();
>>>>
>>>> Sorry, the fix is not going to work correctly.
>>>> The first vm.suspend() has to be before the switch statement to work for
>>>> all 3 cases.
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>> On 7/28/20 16:49, Leonid Mesnik wrote:
>>>>> I've update to suspend/resume in all cases.
>>>>>
>>>>> new webrev: http://cr.openjdk.java.net/~lmesnik/8244537/webrev.01/
>>>>> <http://cr.openjdk.java.net/~lmesnik/8244537/webrev.01/>
>>>>>
>>>>> Leonid
>>>>>
>>>>>> On Jul 28, 2020, at 2:06 PM, [email protected]
>>>>>> <mailto:[email protected]> wrote:
>>>>>>
>>>>>> I prefer to suspend/resume in all cases, so we avoid all these
>>>>>> unexpected failures.
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>>
>>>>>> On 7/28/20 13:59, Leonid Mesnik wrote:
>>>>>>> It should be failure anyway if we managed to enable events, so we don't
>>>>>>> expect to really enable anything in these cases.
>>>>>>> However I agree that adding suspend/resume shouldn't make it worse,
>>>>>>> just possible cleaner log (in very rare cases also). If you feel it is
>>>>>>> need I will just add suspension for all cases.
>>>>>>>
>>>>>>> Leonid
>>>>>>>
>>>>>>>> On Jul 28, 2020, at 1:54 PM, [email protected]
>>>>>>>> <mailto:[email protected]> wrote:
>>>>>>>>
>>>>>>>> Does it mean, you did not fix cases 0 and 2 because the related
>>>>>>>> failures have never been observed?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Serguei
>>>>>>>>
>>>>>>>>
>>>>>>>> On 7/28/20 13:51, Leonid Mesnik wrote:
>>>>>>>>> Test should fail in cases 0 and 2 with IllegalThreadStateException if
>>>>>>>>> we can enable events. Such failures should be easily identified by
>>>>>>>>> reading logs.
>>>>>>>>>
>>>>>>>>> Leonid
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> On Jul 27, 2020, at 10:28 PM, [email protected]
>>>>>>>>>> <mailto:[email protected]> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Leonid,
>>>>>>>>>>
>>>>>>>>>> The fix looks good in general.
>>>>>>>>>> You missed to explain that the suspend/resume are added to avoid
>>>>>>>>>> actual generation of event that cause this issue.
>>>>>>>>>> The reason is that these events are not actually required.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~lmesnik/8244537/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003.java.frames.html
>>>>>>>>>>
>>>>>>>>>> <http://cr.openjdk.java.net/~lmesnik/8244537/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003.java.frames.html>
>>>>>>>>>> 316 case 1:
>>>>>>>>>> 317 vm.suspend();
>>>>>>>>>> ...
>>>>>>>>>> 336 vm.resume();
>>>>>>>>>>
>>>>>>>>>> Q: Why is only in case 1 suspend/resume used?
>>>>>>>>>> What about cases 0 and 2?
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Serguei
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 7/27/20 18:08, Leonid Mesnik wrote:
>>>>>>>>>>> Hi
>>>>>>>>>>>
>>>>>>>>>>> Could you please review following fix which suspends debugger VM
>>>>>>>>>>> while enabling/disabling events.
>>>>>>>>>>>
>>>>>>>>>>> All changed tests fail intermittently getting unexpected events
>>>>>>>>>>> instead of breakpoint used for communication between
>>>>>>>>>>> debugger/debuggee VM. The tests request different events and verify
>>>>>>>>>>> request's properties but
>>>>>>>>>>> don't process/verify events themselves. Test doesn't
>>>>>>>>>>> aware if events are generated or not. The vm suspension doesn't
>>>>>>>>>>> affect JDWP native agent and it still should get and verify JDWP
>>>>>>>>>>> commands.
>>>>>>>>>>>
>>>>>>>>>>> webrev: http://cr.openjdk.java.net/~lmesnik/8244537/webrev.00/
>>>>>>>>>>> <http://cr.openjdk.java.net/~lmesnik/8244537/webrev.00/>
>>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8244537
>>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8244537>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Leonid
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
