RFR: 8244537: JDI tests fail due to "ERROR: Exception : nsk.share.jdi.JDITestRuntimeException: JDITestRuntimeException : ** event IS NOT a breakpoint **"

Leonid Mesnik leonid.mesnik at oracle.com
Wed Jul 29 17:41:26 UTC 2020


Thank you for review and noticing issues. Still waiting for a second review.

Leonid

> On Jul 29, 2020, at 12:08 AM, serguei.spitsyn at oracle.com wrote:
> 
> It looks good.
> Thank you for the update!
> 
> Thanks,
> Serguei
> 
> 
> On 7/28/20 23:26, Leonid Mesnik wrote:
>> 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, serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com> 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, serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com> 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, serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com> 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, serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com> 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, serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com> 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, serguei.spitsyn at oracle.com <mailto:serguei.spitsyn at oracle.com> 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
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>>> 
>>>> 
>>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20200729/8684b519/attachment-0001.htm>


More information about the serviceability-dev mailing list