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, "[email protected]"
<[email protected]> 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
>
>
