RFR 4613913: Four EventRequest methods are invokable on deleted request

[David Holmes]

Daniil,

Even as far back as 2007 there was concern that changing the current 
behaviour might break existing code. That has to be an even bigger 
concern now!

Further the spec is sloppy here:

" Once the eventRequest is deleted, no operations (for example, 
EventRequest.setEnabled(boolean)) are permitted."

This is too loose. What is an "operation"? Is a query like isEnabled() 
really an "operation"? I would not consider it so. And if we can delete 
requests why is there no "isDeleted" query? The spec seems incomplete 
and too vague.

To me this something that should have been clarified in the spec first 
and then the implementation brought into alignment. But that should have 
happened many years ago. Changing this now seems risky to me.

This change in long standing behaviour also requires a CSR request if it 
is to proceed.

David
-----


On 30/03/2018 8:36 AM, 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 at oracle.com" <serguei.spitsyn at 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
>      >
>      >
>      
>      
> 
>