[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR 4613913: Four EventRequest methods are invokable on deleted request
From:       "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date:       2018-03-30 0:16:29
Message-ID: 3f3cad45-5d88-c825-e9ec-3b2697f68b49 () oracle ! com
[Download RAW message or body]

Looks good.
Thank you for the update!

Thanks,
Serguei


On 3/29/18 15:36, Daniil Titov wrote:
> Hi Serguei,
> 
> Please review a new version of the fix that has these places corrected.
> 
> Webreb: http://cr.openjdk.java.net/~dtitov/4613913/webrev.03
> Bug: https://bugs.openjdk.java.net/browse/JDK-4613913
> 
> Thanks!
> 
> Best regards,
> Daniil
> 
> On 3/29/18, 11:46 AM, "serguei.spitsyn@oracle.com" <serguei.spitsyn@oracle.com> \
> wrote: 
> Hi Daniil,
> 
> It looks good in general.
> One minor comment is that it would be nice to make a cleanup
> (as we already discussed) for all places like this:
> 
> 202             if (isEnabled() || deleted) {
> 203                 throw invalidState();
> 204             }
> 
> As the isEnabled() now checks for deleted and throws the invalidState()
> then we can simplify these fragments to be:
> 
> 202             if (isEnabled()) {
> 203                 throw invalidState();
> 204             }
> 
> 
> Thanks,
> Serguei
> 
> 
> On 3/29/18 10:27, Daniil Titov wrote:
> > Please review the changes that ensure that no operation on deleted \
> > com.sun.jdi.request.EventRequest objects are permitted as per JDI specification \
> > for com.sun.jdi.request.EventRequestManager.deleteEventRequest(com.sun.jdi.request.EventRequest) \
> > method.  The fix makes the following 4 methods in class com.sun.tools.jdi. \
> > EventRequestManagerImpl$EventRequestImpl to throw \
> >                 com.sun.jdi.request.InvalidRequestStateException if the request \
> >                 is deleted:
> > - getProperty()
> > - putProperty(Object, Object)
> > - suspendPolicy()
> > - isEnabled()
> > 
> > Bug: https://bugs.openjdk.java.net/browse/JDK-4613913
> > Webrev: http://cr.openjdk.java.net/~dtitov/4613913/webrev.02/
> > 
> > Best regards,
> > Daniil
> > 
> > 
> 
> 
> 
> 


[prev in list] [next in list] [prev in thread] [next in thread]