Re: 8222667: vmTestbase/nsk/jdi/ThreadStartRequest/addThreadFilter/addthreadfilter002/TestDescription.java failed with "event IS NOT a breakpoint"

[serguei . spitsyn]

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"  
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"  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
 >  >
 >  >
 >
 >
 >
 >
 >
 
 

Re: 8222667: vmTestbase/nsk/jdi/ThreadStartRequest/addThreadFilter/addthreadfilter002/TestDescription.java failed with "event IS NOT a breakpoint"

[Daniil Titov]

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"  
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"  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
>  >
>  >
>  
>  
>  
>
>

Re: 8222667: vmTestbase/nsk/jdi/ThreadStartRequest/addThreadFilter/addthreadfilter002/TestDescription.java failed with "event IS NOT a breakpoint"

[serguei . spitsyn]

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"  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
 >
 >
 
 
 

Re: RFR (M) 8223040: Add a AGCT test

[serguei . spitsyn]

On 5/2/19 5:13 PM, serguei.spit...@oracle.com wrote:

God suggestion!


Above is a typo, I wanted to say "Good suggestion".
But the typo is funny. :)

Thanks,
Serguei



Thanks,
Serguei

On 5/2/19 4:55 PM, Daniel D. Daugherty wrote:

I would use "AsyncGetCallTrace" for the top level directory name.
That would make it easier for someone searching the test space...

Dan


On 5/2/19 7:03 PM, Jean Christophe Beyler wrote:

Hi Serguei,

Thanks for the review, I fixed the bug name but have not yet changed 
the webrev. Does anyone else have an opinion of the naming of the 
tests?


Thanks all!
Jc

On Tue, Apr 30, 2019 at 5:10 PM > wrote:


Hi Jc,

I'd suggest to change the bug title to be:
   Add a AsyncGetCallTrace test

I'm not sure about the test names.
Maybe, it is Okay to keep the AGCT abbreviation.
But I'd like to hear other opinions.

Thanks,
Serguei

On 4/30/19 3:47 PM, Jean Christophe Beyler wrote:

Hi all,

As I start looking at working on the AGCT bugs, I wanted to at
least start creating a baseline of tests for AGCT. This is an
attempt to just have a "base" test (and infrastructure) that
tries to call AGCT and get back some sane information.

Next step will be to add a few more tests that will be exposing
the limitations of
https://bugs.openjdk.java.net/browse/JDK-8178287 for example.

Webrev: http://cr.openjdk.java.net/~jcbeyler/8223040/webrev.00/

Bug: https://bugs.openjdk.java.net/browse/JDK-8223040

This passed the test on my linux machine (the test is only for
linux due to the dlsym) and the submit-repo.

Thanks,
Jc




--

Thanks,
Jc

Re: RFR (M) 8223040: Add a AGCT test

[serguei . spitsyn]

God suggestion!

Thanks,
Serguei

On 5/2/19 4:55 PM, Daniel D. Daugherty wrote:

I would use "AsyncGetCallTrace" for the top level directory name.
That would make it easier for someone searching the test space...

Dan


On 5/2/19 7:03 PM, Jean Christophe Beyler wrote:

Hi Serguei,

Thanks for the review, I fixed the bug name but have not yet changed 
the webrev. Does anyone else have an opinion of the naming of the tests?


Thanks all!
Jc

On Tue, Apr 30, 2019 at 5:10 PM > wrote:


Hi Jc,

I'd suggest to change the bug title to be:
   Add a AsyncGetCallTrace test

I'm not sure about the test names.
Maybe, it is Okay to keep the AGCT abbreviation.
But I'd like to hear other opinions.

Thanks,
Serguei

On 4/30/19 3:47 PM, Jean Christophe Beyler wrote:

Hi all,

As I start looking at working on the AGCT bugs, I wanted to at
least start creating a baseline of tests for AGCT. This is an
attempt to just have a "base" test (and infrastructure) that
tries to call AGCT and get back some sane information.

Next step will be to add a few more tests that will be exposing
the limitations of
https://bugs.openjdk.java.net/browse/JDK-8178287 for example.

Webrev: http://cr.openjdk.java.net/~jcbeyler/8223040/webrev.00/

Bug: https://bugs.openjdk.java.net/browse/JDK-8223040

This passed the test on my linux machine (the test is only for
linux due to the dlsym) and the submit-repo.

Thanks,
Jc




--

Thanks,
Jc

Re: RFR (M) 8223040: Add a AGCT test

[Daniel D. Daugherty]

I would use "AsyncGetCallTrace" for the top level directory name.
That would make it easier for someone searching the test space...

Dan


On 5/2/19 7:03 PM, Jean Christophe Beyler wrote:

Hi Serguei,

Thanks for the review, I fixed the bug name but have not yet changed 
the webrev. Does anyone else have an opinion of the naming of the tests?


Thanks all!
Jc

On Tue, Apr 30, 2019 at 5:10 PM > wrote:


Hi Jc,

I'd suggest to change the bug title to be:
   Add a AsyncGetCallTrace test

I'm not sure about the test names.
Maybe, it is Okay to keep the AGCT abbreviation.
But I'd like to hear other opinions.

Thanks,
Serguei

On 4/30/19 3:47 PM, Jean Christophe Beyler wrote:

Hi all,

As I start looking at working on the AGCT bugs, I wanted to at
least start creating a baseline of tests for AGCT. This is an
attempt to just have a "base" test (and infrastructure) that
tries to call AGCT and get back some sane information.

Next step will be to add a few more tests that will be exposing
the limitations of
https://bugs.openjdk.java.net/browse/JDK-8178287 for example.

Webrev: http://cr.openjdk.java.net/~jcbeyler/8223040/webrev.00/

Bug: https://bugs.openjdk.java.net/browse/JDK-8223040

This passed the test on my linux machine (the test is only for
linux due to the dlsym) and the submit-repo.

Thanks,
Jc




--

Thanks,
Jc

Re: RFR (M) 8223040: Add a AGCT test

[serguei . spitsyn]

Hi Jc,

I've not reviewed it yet but it is on my list.

Thanks,
Serguei


On 5/2/19 4:03 PM, Jean Christophe Beyler wrote:

Hi Serguei,

Thanks for the review, I fixed the bug name but have not yet changed 
the webrev. Does anyone else have an opinion of the naming of the tests?


Thanks all!
Jc

On Tue, Apr 30, 2019 at 5:10 PM > wrote:


Hi Jc,

I'd suggest to change the bug title to be:
   Add a AsyncGetCallTrace test

I'm not sure about the test names.
Maybe, it is Okay to keep the AGCT abbreviation.
But I'd like to hear other opinions.

Thanks,
Serguei

On 4/30/19 3:47 PM, Jean Christophe Beyler wrote:

Hi all,

As I start looking at working on the AGCT bugs, I wanted to at
least start creating a baseline of tests for AGCT. This is an
attempt to just have a "base" test (and infrastructure) that
tries to call AGCT and get back some sane information.

Next step will be to add a few more tests that will be exposing
the limitations of
https://bugs.openjdk.java.net/browse/JDK-8178287 for example.

Webrev: http://cr.openjdk.java.net/~jcbeyler/8223040/webrev.00/

Bug: https://bugs.openjdk.java.net/browse/JDK-8223040

This passed the test on my linux machine (the test is only for
linux due to the dlsym) and the submit-repo.

Thanks,
Jc




--

Thanks,
Jc

Re: RFR (M) 8223040: Add a AGCT test

[Jean Christophe Beyler]

Hi Serguei,

Thanks for the review, I fixed the bug name but have not yet changed the
webrev. Does anyone else have an opinion of the naming of the tests?

Thanks all!
Jc

On Tue, Apr 30, 2019 at 5:10 PM  wrote:

> Hi Jc,
>
> I'd suggest to change the bug title to be:
>Add a AsyncGetCallTrace test
>
> I'm not sure about the test names.
> Maybe, it is Okay to keep the AGCT abbreviation.
> But I'd like to hear other opinions.
>
> Thanks,
> Serguei
>
> On 4/30/19 3:47 PM, Jean Christophe Beyler wrote:
>
> Hi all,
>
> As I start looking at working on the AGCT bugs, I wanted to at least start
> creating a baseline of tests for AGCT. This is an attempt to just have a
> "base" test (and infrastructure) that tries to call AGCT and get back some
> sane information.
>
> Next step will be to add a few more tests that will be exposing the
> limitations of https://bugs.openjdk.java.net/browse/JDK-8178287 for
> example.
>
> Webrev: http://cr.openjdk.java.net/~jcbeyler/8223040/webrev.00/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8223040
>
> This passed the test on my linux machine (the test is only for linux due
> to the dlsym) and the submit-repo.
>
> Thanks,
> Jc
>
>
>

-- 

Thanks,
Jc

Re: RFR (S): 8223177: Data race on JvmtiEnvBase::_tag_map in double-checked locking

[serguei.spit...@oracle.com]

Hi David and Nan,

Sorry, my suggestion was not aligned with usual practice and added some 
extra work. :(


Thanks,
Serguei


On 5/1/19 19:42, David Holmes wrote:

On 2/05/2019 11:00 am, Man Cao wrote:

Thank everyone for the review!
Renamed and final webrev:
https://cr.openjdk.java.net/~manc/8223177/webrev.02/


No do not rename! Sorry Serguei but for these accesses with 
OrderAccess semantics the placement of the acquire/release reflects 
the barrier semantics of load_acquire and release_store. So we use 
foo_acquire to load foo with acquire semantics; while release_set_foo 
performs a release barrier followed by set_foo. This convention is 
used throughout the VM for these kinds of methods.


David
-


-Man


On Wed, May 1, 2019 at 5:21 PM serguei.spit...@oracle.com 
 > wrote:


    Hi Man,

    Looks good to me.

    Minor comment:
    I'd suggest to rename tag_map_acquire to acquire_tag_map to be
    consistent with release_set_tag_map.


    Thanks,
    Serguei


    On 4/30/19 18:51, Man Cao wrote:

    Hi all,

    Can I have reviews for this small change that adds memory fences
    for double-checked locking?
    We found this race while working on the Java ThreadSanitizer 
project.


    Webrev: https://cr.openjdk.java.net/~manc/8223177/webrev.00/
    Bug: https://bugs.openjdk.java.net/browse/JDK-8223177

    -Man

Re: 8222667: vmTestbase/nsk/jdi/ThreadStartRequest/addThreadFilter/addthreadfilter002/TestDescription.java failed with "event IS NOT a breakpoint"

[Chris Plummer]

Looks good.

Chris

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"  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
 >
 >
 
 
 

Re: 8222667: vmTestbase/nsk/jdi/ThreadStartRequest/addThreadFilter/addthreadfilter002/TestDescription.java failed with "event IS NOT a breakpoint"

[Daniil Titov]

Hi Gary,

Thank you for reviewing this.  I added a diagnostic that dumped this unexpected 
event and printed the thread name that caused it.

Best regards,
Daniil

On 5/2/19, 10:08 AM, "Gary Adams"  wrote:

Looks good to me.

BTW, how did you track down this failure.
Did you use some tracing option,
or did you add a diagnostic to dump the
stray event?


On 5/2/19, 12:48 PM, 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"  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
>  >
>  >
>
>
>
>
>

Re: 8222667: vmTestbase/nsk/jdi/ThreadStartRequest/addThreadFilter/addthreadfilter002/TestDescription.java failed with "event IS NOT a breakpoint"

[Gary Adams]

Looks good to me.

BTW, how did you track down this failure.
Did you use some tracing option,
or did you add a diagnostic to dump the
stray event?


On 5/2/19, 12:48 PM, 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"  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
 >
 >

Re: 8222667: vmTestbase/nsk/jdi/ThreadStartRequest/addThreadFilter/addthreadfilter002/TestDescription.java failed with "event IS NOT a breakpoint"

[Daniil Titov]

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"  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
>
>

Re: RFR: 8222667: vmTestbase/nsk/jdi/ThreadStartRequest/addThreadFilter/addthreadfilter002/TestDescription.java failed with "event IS NOT a breakpoint"

[Gary Adams]

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

Re: 8157947: SA: Javascript engine can't access internal packages of jdk.hotspot.agent

[Yasumasa Suenaga]

> Would it be possible re-outline the problem and/or include the 
exception that is being observed?


We can see TypeError thrown by Nashorn when we attach CLHSDB.
It means JS in jdk.hotspot.agent (sa.js) could not call 
sun.jvm.hotspot.runtime.VM::getVM .


It is caused that jdk.hotspot.agent does not export to 
jdk.scripting.nashorn.scripts module.

I tried to open all SA packages to everyone, it works fine [1].

IMHO it would be convenience that module API provides method(s) to open 
to everyone, or --add-opens option can open modules to everyone.


Anyway, I think this issue may occur any applications that JS calls Java 
world. Thus I hope this issue would be resolved in scripting API or 
module system.

If so, SA should use it :-)


Thanks,

Yasumasa



[1] http://cr.openjdk.java.net/~ysuenaga/JDK-8157947/proposal/



On 2019/05/02 15:30, Alan Bateman wrote:

On 02/05/2019 06:47, Sundararajan Athijegannathan wrote:

Hi Yasumasa,

Sorry for delayed response. Your webrev looks like a good start! 
Unfortunately, dependency on nashorn is inevitable. But there is a 
similar (but different) object API for Graal.js. In future, someone 
may have to do some porting work.
This exports sun.jvm.hotspot.utilities.soql.wrapper unconditionally so 
it's adding a JDK-specific API. Would it be possible re-outline the 
problem and/or include the exception that is being observed? If 
jdk.hotspot.agent is going to export an API then it will need a lot of 
javadoc work and of course a CSR (and no guarantee that the CSR would be 
approved because we agreed in JDK 9 that jdk.hotspot.agent would be a 
supported interface).


-Alan

Re: 8157947: SA: Javascript engine can't access internal packages of jdk.hotspot.agent

[Alan Bateman]

On 02/05/2019 07:30, Alan Bateman wrote:
(and no guarantee that the CSR would be approved because we agreed in 
JDK 9 that jdk.hotspot.agent would be a supported interface).

This should say "would not be a supported interface" of course.