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.spit...@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.spit...@oracle.com > <mailto:serguei.spit...@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.spit...@oracle.com >>>> <mailto:serguei.spit...@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.spit...@oracle.com >>>>>> <mailto:serguei.spit...@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.spit...@oracle.com >>>>>>>> <mailto:serguei.spit...@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 >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
