[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8244537: JDI tests fail due to "ERROR: Exception : nsk.share.jdi.JDITestRuntimeException: J
From: Leonid Mesnik <leonid.mesnik () oracle ! com>
Date: 2020-07-29 17:41:26
Message-ID: 0F59DEE6-1065-47CA-ACB2-003068D5516F () oracle ! com
[Download RAW message or body]
Thank you for review and noticing issues. Still waiting for a second review.
Leonid
> On Jul 29, 2020, at 12:08 AM, serguei.spitsyn@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@oracle.com \
> > > <mailto:serguei.spitsyn@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@oracle.com \
> > > > > <mailto:serguei.spitsyn@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@oracle.com \
> > > > > <mailto:serguei.spitsyn@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@oracle.com \
> > > > > > > > <mailto:serguei.spitsyn@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@oracle.com \
> > > > > > > > > > <mailto:serguei.spitsyn@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@oracle.com \
> > > > > > > > > > > > <mailto:serguei.spitsyn@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.
> > > > > > > > > > > >
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > \
> > > > > > > > > > > > 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/ \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > \
> > > > > > > > > > > > > bug: https://bugs.openjdk.java.net/browse/JDK-8244537 \
> > > > > > > > > > > > > <https://bugs.openjdk.java.net/browse/JDK-8244537>
> > > > > > > > > > > > >
> > > > > > > > > > > > > Leonid
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
[Attachment #3 (unknown)]
<html><head><meta http-equiv="Content-Type" content="text/html; \
charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: \
space; line-break: after-white-space;" class="">Thank you for review and noticing \
issues. Still waiting for a second review.<div class=""><br class=""></div><div \
class="">Leonid<br class=""><div><br class=""><blockquote type="cite" class=""><div \
class="">On Jul 29, 2020, at 12:08 AM, <a href="mailto:serguei.spitsyn@oracle.com" \
class="">serguei.spitsyn@oracle.com</a> wrote:</div><br \
class="Apple-interchange-newline"><div class="">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" class="">
<div class="">
<div class="moz-cite-prefix">It looks good.<br class="">
Thank you for the update!<br class="">
<br class="">
Thanks,<br class="">
Serguei<br class="">
<br class="">
<br class="">
On 7/28/20 23:26, Leonid Mesnik wrote:<br class="">
</div>
<blockquote type="cite" \
cite="mid:5CA977F6-2B49-4E47-B5BD-E4EA68F361FA@oracle.com" class="">ok, let change \
it to following <div class=""><br class="">
</div>
<div class="">
<div class="">diff -r f489d5d13a51
test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java</div>
<div class="">---
a/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java
Thu Jul 23 16:36:44 2020 -0400</div>
<div class="">+++
b/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java
Tue Jul 28 23:17:22 2020 -0700</div>
<div class="">@@ -368,6 +368,7 @@</div>
<div class=""> \
throw new JDITestRuntimeException("** default case 2 **");</div>
<div class=""> }</div>
<div class=""><br class="">
</div>
<div class="">+ vm.suspend();</div>
<div class=""> if \
(eventRequest1 instanceof StepRequest) {</div>
<div class=""> \
try {</div>
<div class=""> \
log2("......eventRequest1.setEnabled(true);
IllegalThreadStateException is expected");</div>
<div class="">@@ -405,6 +406,7 @@</div>
<div class=""> } catch (
InvalidRequestStateException e ) {</div>
<div class=""> \
log2(" InvalidRequestStateException");</div>
<div class=""> }</div>
<div class="">+ vm.resume();</div>
<div class=""><br class="">
</div>
<div class="">
//~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~</div>
<div class=""> }</div>
<div class=""><br class="">
</div>
<div class="">webrev: <a \
href="http://cr.openjdk.java.net/~lmesnik/8244537/webrev.04/" class="" \
moz-do-not-send="true">http://cr.openjdk.java.net/~lmesnik/8244537/webrev.04/</a></div>
<div class="">Leonid</div>
<div class="">
<div class=""><br class="">
<blockquote type="cite" class="">
<div class="">On Jul 28, 2020, at 11:06 PM, <a \
href="mailto:serguei.spitsyn@oracle.com" class="" \
moz-do-not-send="true">serguei.spitsyn@oracle.com</a> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">
<div class="moz-cite-prefix"><a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~lmesnik/8244537/webrev.03/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java.frames.html" \
moz-do-not-send="true">http://cr.openjdk.java.net/~lmesnik/8244537/webrev.03/test/hots \
pot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java.frames.html</a><br \
class=""> <br class="">
I'd suggest to simplify it:<br class="">
<br class="">
- insert suspend before the line: <br class="">
<pre class=""> 371 if (eventRequest1 instanceof \
StepRequest) { </pre>
- keep resume at the line:<br class="">
<pre class=""><span class="new"> 411 vm.resume();
</span></pre>
- these lines are not needed:<br class="">
<pre class=""><span class="new"> 389 vm.resume();
</span><span class="new"> 394 vm.suspend();</span>
<span class="new"></span></pre>
<br class="">
Thanks,<br class="">
Serguei<br class="">
<br class="">
<br class="">
On 7/28/20 21:15, Leonid Mesnik wrote:<br class="">
</div>
<blockquote type="cite" \
cite="mid:C7A72111-2EBB-406E-863F-0F80B159A8F1@oracle.com" class="">Included in \
webrev.03 <div class=""><a \
href="http://cr.openjdk.java.net/~lmesnik/8244537/webrev.03" class="" \
moz-do-not-send="true">http://cr.openjdk.java.net/~lmesnik/8244537/webrev.03</a></div>
<div class=""><br class="">
</div>
<div class="">Leonid<br class="">
<div class=""><br class="">
<blockquote type="cite" class="">
<div class="">On Jul 28, 2020, at 6:44 PM, <a \
href="mailto:serguei.spitsyn@oracle.com" class="" \
moz-do-not-send="true">serguei.spitsyn@oracle.com</a> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">
<div class="moz-cite-prefix"><a \
class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~lmesnik/8244537/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java.frames.html" \
moz-do-not-send="true">http://cr.openjdk.java.net/~lmesnik/8244537/webrev.01/test/hots \
pot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled002.java.frames.html</a><br \
class="">
<pre class=""> 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 }</pre>
Above is one more case where
suspend/resume is needed, I guess.<br class="">
<br class="">
Thanks,<br class="">
Serguei<br class="">
<br class="">
<br class="">
On 7/28/20 17:45, <a class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com" \
moz-do-not-send="true">serguei.spitsyn@oracle.com</a> wrote:<br class="">
</div>
<blockquote type="cite" \
cite="mid:11e29ad2-8e06-ad30-df48-6dcfea0bd4dd@oracle.com" class=""> <div \
class="moz-cite-prefix"><a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~lmesnik/8244537/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003.java.frames.html" \
moz-do-not-send="true">http://cr.openjdk.java.net/~lmesnik/8244537/webrev.01/test/hots \
pot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003.java.frames.html</a><br \
class=""> <pre class=""> 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
<span class="new"><b class=""> 296 vm.suspend();</b>
...
</span></pre>
<pre class=""> 360 default:
361 throw new JDITestRuntimeException("** default case 2 **");
362 }
<span class="new"> 363 vm.resume();</span></pre>
<br class="">
Sorry, the fix is not going to work
correctly.<br class="">
The first vm.suspend() has to be
before the switch statement to work
for all 3 cases.<br class="">
<br class="">
Thanks,<br class="">
Serguei<br class="">
<br class="">
<br class="">
On 7/28/20 16:49, Leonid Mesnik wrote:<br class="">
</div>
<blockquote type="cite" \
cite="mid:D045BF0E-A452-45AF-8D02-9FF37E4458A1@oracle.com" class="">I've update to \
suspend/resume in all cases.
<div class=""><br class="">
</div>
<div class="">new webrev: <a \
href="http://cr.openjdk.java.net/~lmesnik/8244537/webrev.01/" class="" \
moz-do-not-send="true">http://cr.openjdk.java.net/~lmesnik/8244537/webrev.01/</a></div>
<div class=""><br class="">
</div>
<div class="">Leonid<br class="">
<div class=""><br class="">
<blockquote type="cite" class="">
<div class="">On Jul 28, 2020,
at 2:06 PM, <a \
href="mailto:serguei.spitsyn@oracle.com" class="" \
moz-do-not-send="true">serguei.spitsyn@oracle.com</a> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div class="">
<div class="moz-cite-prefix">I
prefer to suspend/resume
in all cases, so we avoid
all these unexpected
failures.<br class="">
<br class="">
Thanks,<br class="">
Serguei<br class="">
<br class="">
<br class="">
On 7/28/20 13:59, Leonid
Mesnik wrote:<br class="">
</div>
<blockquote type="cite" \
cite="mid:4CF129DD-1733-47A0-91F9-B27891BCC63E@oracle.com" class="">It should be \
failure anyway if we managed to enable events,
so we don't expect to
really enable anything in
these cases.
<div class="">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.</div>
<div class=""><br class="">
</div>
<div class="">Leonid<br class="">
<div class=""><br class="">
<blockquote type="cite" class="">
<div class="">On Jul
28, 2020, at 1:54
PM, <a \
href="mailto:serguei.spitsyn@oracle.com" class="" \
moz-do-not-send="true">serguei.spitsyn@oracle.com</a> wrote:</div>
<br \
class="Apple-interchange-newline"> <div class="">
<div class="">
<div \
class="moz-cite-prefix">Does it mean, you
did not fix
cases 0 and 2
because the
related
failures have
never been
observed?<br class="">
<br class="">
Thanks,<br class="">
Serguei<br class="">
<br class="">
<br class="">
On 7/28/20
13:51, Leonid
Mesnik wrote:<br class="">
</div>
<blockquote type="cite" \
cite="mid:68FBDE47-11DA-45C3-AD07-8200DCC9BA29@oracle.com" class="">Test should fail \
in cases 0 and 2
with
IllegalThreadStateException
if we can
enable events.
Such failures
should be
easily
identified by
reading logs.
<div class=""><br class="">
</div>
<div class="">Leonid<br \
class="">
<div class=""><br class="">
<div class="">
<div class=""><br class="">
<blockquote type="cite" \
class=""> <div class="">On
Jul 27, 2020,
at 10:28 PM, <a \
href="mailto:serguei.spitsyn@oracle.com" class="" \
moz-do-not-send="true">serguei.spitsyn@oracle.com</a> wrote:</div>
<br \
class="Apple-interchange-newline"> <div class="">
<div class="">
<div \
class="moz-cite-prefix">Hi
Leonid,<br class="">
<br class="">
The fix looks
good in
general.<br class="">
You missed to
explain that
the
suspend/resume
are added to
avoid actual
generation of
event that
cause this
issue.<br class="">
The reason is
that these
events are not
actually
required. <br class="">
<br class="">
<br class="">
<a \
class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~lmesnik/8244537/webrev.00/test/hotspot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003.java.frames.html" \
moz-do-not-send="true">http://cr.openjdk.java.net/~lmesnik/8244537/webrev.00/test/hots \
pot/jtreg/vmTestbase/nsk/jdi/EventRequest/setEnabled/setenabled003.java.frames.html</a><br \
class="">
<pre class=""> 316 \
case 1: <span class="new"> 317 vm.suspend();
...
</span><span class="new"> 336 vm.resume();</span>
<span class="new"></span></pre>
<br class="">
Q: Why is only
in case 1
suspend/resume
used?<br class="">
What about
cases 0 and 2?<br class="">
<br class="">
Thanks,<br class="">
Serguei<br class="">
<br class="">
<br class="">
On 7/27/20
18:08, Leonid
Mesnik wrote:<br class="">
</div>
<blockquote type="cite" \
cite="mid:16EB9126-382A-4092-BB61-14E9C3CF208C@oracle.com" \
class="">Hi
<div class=""><br class="">
</div>
<div class="">Could
you please
review
following fix
which suspends
debugger VM
while
enabling/disabling
events. </div>
<div class=""><br class="">
</div>
<div class="">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.</div>
<div class=""><br class="">
</div>
<div \
class="">webrev: <a \
href="http://cr.openjdk.java.net/~lmesnik/8244537/webrev.00/" class="" \
moz-do-not-send="true">http://cr.openjdk.java.net/~lmesnik/8244537/webrev.00/</a></div>
<div class="">bug: <a href="https://bugs.openjdk.java.net/browse/JDK-8244537" \
class="" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8244537</a></div>
<div class=""><br class="">
</div>
<div class=""><br class="">
</div>
<div class="">Leonid</div>
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</div>
</div>
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</blockquote>
<br class="">
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</blockquote>
<br class="">
</div>
</div>
</blockquote>
</div>
<br class="">
</div>
</div>
</blockquote>
<br class="">
</div>
</div></blockquote></div><br class=""></div></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic