Thanks for catching this! I placed it here by mistake. It should be before switch.
http://cr.openjdk.java.net/~lmesnik/8244537/webrev.03/ <http://cr.openjdk.java.net/~lmesnik/8244537/webrev.03/> Leonid > On Jul 28, 2020, at 5:45 PM, [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 >>>>>>> >>>>>> >>>>> >>>> >>> >> >
