Hi Daniil,
Looks good.
Thank you for the update.
Sorry, I forgot to tell no new webrev is needed if you fix this. :)
Thanks,
Serguei
On 5/2/19 6:09 PM, Daniil Titov wrote:
Hi Serguei,
Please review a new version of the fix that includes the changes you suggested.
Webrev: http://cr.openjdk.java.net/~dtitov/8222667/webrev.03
Bug: https://bugs.openjdk.java.net/browse/JDK-8222667
Thanks!
--Daniil
On 5/2/19, 5:24 PM, "serguei.spit...@oracle.com" <serguei.spit...@oracle.com>
wrote:
Hi Daniil,
Could you, please, replace 'e' with 'event'?
I've never liked one-letter identifiers.
Other than that it looks good to me.
Thanks,
Serguei
On 5/2/19 9:48 AM, Daniil Titov wrote:
> Hi Gary,
>
> Please review a new version of the webrev that adds the comment you
mentioned.
>
> Regarding the reusable filters I think it makes sense to create them
when we will find that they could be reused more than in one place and this test
is a quite specific, it creates ThreadStartRequest and enables it before
restricting it to the test thread only (by calling addThreadFilter()) since it is
exactly what the test checks (that calling addThreadFilter() on enabled thread
start request throws InvalidRequestStateException.
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8222667/webrev.02
> Bug: https://bugs.openjdk.java.net/browse/JDK-8222667
>
> Thanks!
> --Danill
>
> On 5/2/19, 4:56 AM, "Gary Adams" <gary.ad...@oracle.com> wrote:
>
> It would be good to include a comment that the thread start is being
> allowed because of graal.
>
> Now that we have a series of graal specific alterations in the
tests,
> it might be useful to provide some reusable filters. $.02
>
> On 5/1/19, 9:07 PM, Daniil Titov wrote:
> > Please review the change that fixes the test that intermittently
fails with Graal on.
> >
> > The test tests addThreadFilter() method for com.sun.jdi.ThreadStartRequest
class. It starts a debuggee, creates ThreadStartRequest, enables it, and calls
addThreadFilter(). The test also uses breakpoints to synchronize with the debuggee, but the
problem here is that while waiting for a breakpoint event, occasionally, when Graal is on, the
event the test receives is a ThreadStartEvent for " HotSpotGraalManagement Bean
Registration" thread, rather than a breakpoint event. The test doesn't expect it and fails.
> >
> > The fix takes this into account and makes the test keep waiting
for a breakpoint event instead of failing.
> >
> > Webrev:http://cr.openjdk.java.net/~dtitov/8222667/webrev.01/
> > Bug:https://bugs.openjdk.java.net/browse/JDK-8222667
> >
> > Thanks!
> > --Daniil
> >
> >
>
>
>
>
>
