Re: RFR: 8150758: [TESTBUG] need jvmti tests for module aware agents

[Dmitry Dmitriev]

Dmitry, Sergeui, thank you for the review!

For the record, webrev.02: 
http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.02/ 



Dmitry

On 20.09.2016 13:39, Dmitry Samersoff wrote:

Dmitry,

Looks good for me!

Minor formatting nits:

libMAAClassFileLoadHook.c

103: missed space after ,

libMAAClassFileLoadHook.c:
266: extra semicolon

-Dmitry

On 2016-09-20 13:17, Dmitry Dmitriev wrote:

Serguei, Dmitry,

Thank you for the feedback! Here is an updated webrev.01;
http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.01/


Dmitry

On 20.09.2016 12:48, Dmitry Samersoff wrote:

Dmitry,

Please, also change

!strcmp(...) to strcmp(...) == 0

because semantically strcmp result is not a boolean false.

-Dmitry

On 2016-09-20 06:38, serguei.spit...@oracle.com wrote:

Hi Dmitry,


Thanks a lot for this additional test coverage and discovering new bug:
https://bugs.openjdk.java.net/browse/JDK-8165681

The tests look pretty good to me.

A couple of minor comments.

Dots are missed in the .c files comments.


http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/test/serviceability/jvmti




   265 printf("Expecting to find '%s' class in ClassLoad events
duringVM start phase.\n", EXPECTED_SIGNATURE);
   266 if (class_in_class_load_events_vm_start == JNI_FALSE) {
   267 throw_exc(env, "Unable to find expected class in
ClassLoad events duringstart phase!\n");
   268 return FAILED;
   269 }
   270
   271 if (class_prepare_events_vm_start_count == 0) {
   272 throw_exc(env, "Didn't get ClassPrepare events in
start
phase!\n");
   273 return FAILED;
   274 }
   275
   276 printf("Expecting to find '%s' class in ClassPrepare
events
duringVM start phase.\n", EXPECTED_SIGNATURE);
   277 if (class_in_class_prepare_events_vm_start == JNI_FALSE) {
   278 throw_exc(env, "Unable to find expected class in
ClassPrepare events duringstart phase!\n");
   279 return FAILED;
   280 }


 Could you, please, replace "start phase" with "early start phase"?
It will be more clear that the start phase mode is "early".

   288 if (class_in_class_prepare_events_vm_start == JNI_TRUE) {
   289 throw_exc(env, "Class is found in ClassLoad events
duringVM Start phase!\n");
   290 return FAILED;
   291 }
   292
   293 printf("Expecting that '%s' class is absent in
ClassPrepare
events.\n", EXPECTED_SIGNATURE);
   294 if (class_in_class_prepare_events_vm_start == JNI_TRUE) {
   295 throw_exc(env, "Class is found in ClassPrepare events
duringVM Start phase!\n");
   296 return FAILED;
   297 }

 Could you, please, replace "VM Start phase" with "normal VM start
phase"?It will be more clear that the start phase mode is "normal".
Thanks, Serguei On 9/19/16 05:47, Dmitry Dmitriev wrote:

Hello, Please review new tests for module aware agents. There are 3
tests: 1) MAAClassFileLoadHook.java - verifies ClassFileLoadHook event
with different combinations of can_generate_early_vmstart and
can_generate_early_class_hook_events JVMTI capabilities. Expects to
find(or not) class from java.base module in the right VM phase. 2)
MAAClassLoadPrepare.java - verifies ClassLoad and ClassPrepare events
with and without can_generate_early_vmstart JVMTI capability. Expects
to find(or not) class from java.base module in the VM start phase. 3)
MAAThreadStart.java - verifies ThreadStart event with
can_generate_early_vmstart JVMTI capability. Expect to find events in
the VM start phase. JBS:
https://bugs.openjdk.java.net/browse/JDK-8150758 webrev.00:
http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/
 Testing:
RBT on all platforms Thanks, Dmitry

Re: RFR: 8150758: [TESTBUG] need jvmti tests for module aware agents

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

Hi Dmitry,

The fix looks good, thank you for the update.
Thumbs up.

Thanks,
Serguei


On 9/20/16 03:17, Dmitry Dmitriev wrote:

Serguei, Dmitry,

Thank you for the feedback! Here is an updated webrev.01; 
http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.01/ 



Dmitry

On 20.09.2016 12:48, Dmitry Samersoff wrote:

Dmitry,

Please, also change

!strcmp(...) to strcmp(...) == 0

because semantically strcmp result is not a boolean false.

-Dmitry

On 2016-09-20 06:38, serguei.spit...@oracle.com wrote:

Hi Dmitry,


Thanks a lot for this additional test coverage and discovering new bug:
   https://bugs.openjdk.java.net/browse/JDK-8165681

The tests look pretty good to me.

A couple of minor comments.

Dots are missed in the .c files comments.


http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/test/serviceability/jvmti 





  265 printf("Expecting to find '%s' class in ClassLoad events
duringVM start phase.\n", EXPECTED_SIGNATURE);
  266 if (class_in_class_load_events_vm_start == JNI_FALSE) {
  267 throw_exc(env, "Unable to find expected class in
ClassLoad events duringstart phase!\n");
  268 return FAILED;
  269 }
  270
  271 if (class_prepare_events_vm_start_count == 0) {
  272 throw_exc(env, "Didn't get ClassPrepare events in 
start

phase!\n");
  273 return FAILED;
  274 }
  275
  276 printf("Expecting to find '%s' class in ClassPrepare 
events

duringVM start phase.\n", EXPECTED_SIGNATURE);
  277 if (class_in_class_prepare_events_vm_start == 
JNI_FALSE) {

  278 throw_exc(env, "Unable to find expected class in
ClassPrepare events duringstart phase!\n");
  279 return FAILED;
  280 }


Could you, please, replace "start phase" with "early start phase"?
It will be more clear that the start phase mode is "early".

  288 if (class_in_class_prepare_events_vm_start == JNI_TRUE) {
  289 throw_exc(env, "Class is found in ClassLoad events
duringVM Start phase!\n");
  290 return FAILED;
  291 }
  292
  293 printf("Expecting that '%s' class is absent in 
ClassPrepare

events.\n", EXPECTED_SIGNATURE);
  294 if (class_in_class_prepare_events_vm_start == JNI_TRUE) {
  295 throw_exc(env, "Class is found in ClassPrepare events
duringVM Start phase!\n");
  296 return FAILED;
  297 }

Could you, please, replace "VM Start phase" with "normal VM start
phase"?It will be more clear that the start phase mode is "normal".
Thanks, Serguei On 9/19/16 05:47, Dmitry Dmitriev wrote:

Hello, Please review new tests for module aware agents. There are 3
tests: 1) MAAClassFileLoadHook.java - verifies ClassFileLoadHook event
with different combinations of can_generate_early_vmstart and
can_generate_early_class_hook_events JVMTI capabilities. Expects to
find(or not) class from java.base module in the right VM phase. 2)
MAAClassLoadPrepare.java - verifies ClassLoad and ClassPrepare events
with and without can_generate_early_vmstart JVMTI capability. Expects
to find(or not) class from java.base module in the VM start phase. 3)
MAAThreadStart.java - verifies ThreadStart event with
can_generate_early_vmstart JVMTI capability. Expect to find events in
the VM start phase. JBS:
https://bugs.openjdk.java.net/browse/JDK-8150758 webrev.00:
http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/
 Testing:
RBT on all platforms Thanks, Dmitry

Re: RFR: 8150758: [TESTBUG] need jvmti tests for module aware agents

[Dmitry Samersoff]

Dmitry,

Looks good for me!

Minor formatting nits:

libMAAClassFileLoadHook.c

103: missed space after ,

libMAAClassFileLoadHook.c:
266: extra semicolon

-Dmitry

On 2016-09-20 13:17, Dmitry Dmitriev wrote:
> Serguei, Dmitry,
> 
> Thank you for the feedback! Here is an updated webrev.01;
> http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.01/
> 
> 
> Dmitry
> 
> On 20.09.2016 12:48, Dmitry Samersoff wrote:
>> Dmitry,
>>
>> Please, also change
>>
>> !strcmp(...) to strcmp(...) == 0
>>
>> because semantically strcmp result is not a boolean false.
>>
>> -Dmitry
>>
>> On 2016-09-20 06:38, serguei.spit...@oracle.com wrote:
>>> Hi Dmitry,
>>>
>>>
>>> Thanks a lot for this additional test coverage and discovering new bug:
>>>https://bugs.openjdk.java.net/browse/JDK-8165681
>>>
>>> The tests look pretty good to me.
>>>
>>> A couple of minor comments.
>>>
>>> Dots are missed in the .c files comments.
>>>
>>>
>>> http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/test/serviceability/jvmti
>>>
>>>
>>>
>>>
>>>   265 printf("Expecting to find '%s' class in ClassLoad events
>>> duringVM start phase.\n", EXPECTED_SIGNATURE);
>>>   266 if (class_in_class_load_events_vm_start == JNI_FALSE) {
>>>   267 throw_exc(env, "Unable to find expected class in
>>> ClassLoad events duringstart phase!\n");
>>>   268 return FAILED;
>>>   269 }
>>>   270
>>>   271 if (class_prepare_events_vm_start_count == 0) {
>>>   272 throw_exc(env, "Didn't get ClassPrepare events in
>>> start
>>> phase!\n");
>>>   273 return FAILED;
>>>   274 }
>>>   275
>>>   276 printf("Expecting to find '%s' class in ClassPrepare
>>> events
>>> duringVM start phase.\n", EXPECTED_SIGNATURE);
>>>   277 if (class_in_class_prepare_events_vm_start == JNI_FALSE) {
>>>   278 throw_exc(env, "Unable to find expected class in
>>> ClassPrepare events duringstart phase!\n");
>>>   279 return FAILED;
>>>   280 }
>>>
>>>
>>> Could you, please, replace "start phase" with "early start phase"?
>>> It will be more clear that the start phase mode is "early".
>>>
>>>   288 if (class_in_class_prepare_events_vm_start == JNI_TRUE) {
>>>   289 throw_exc(env, "Class is found in ClassLoad events
>>> duringVM Start phase!\n");
>>>   290 return FAILED;
>>>   291 }
>>>   292
>>>   293 printf("Expecting that '%s' class is absent in
>>> ClassPrepare
>>> events.\n", EXPECTED_SIGNATURE);
>>>   294 if (class_in_class_prepare_events_vm_start == JNI_TRUE) {
>>>   295 throw_exc(env, "Class is found in ClassPrepare events
>>> duringVM Start phase!\n");
>>>   296 return FAILED;
>>>   297 }
>>>
>>> Could you, please, replace "VM Start phase" with "normal VM start
>>> phase"?It will be more clear that the start phase mode is "normal".
>>> Thanks, Serguei On 9/19/16 05:47, Dmitry Dmitriev wrote:
 Hello, Please review new tests for module aware agents. There are 3
 tests: 1) MAAClassFileLoadHook.java - verifies ClassFileLoadHook event
 with different combinations of can_generate_early_vmstart and
 can_generate_early_class_hook_events JVMTI capabilities. Expects to
 find(or not) class from java.base module in the right VM phase. 2)
 MAAClassLoadPrepare.java - verifies ClassLoad and ClassPrepare events
 with and without can_generate_early_vmstart JVMTI capability. Expects
 to find(or not) class from java.base module in the VM start phase. 3)
 MAAThreadStart.java - verifies ThreadStart event with
 can_generate_early_vmstart JVMTI capability. Expect to find events in
 the VM start phase. JBS:
 https://bugs.openjdk.java.net/browse/JDK-8150758 webrev.00:
 http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/
  Testing:
 RBT on all platforms Thanks, Dmitry
>>
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.

Re: RFR: 8150758: [TESTBUG] need jvmti tests for module aware agents

[Dmitry Dmitriev]

Serguei, Dmitry,

Thank you for the feedback! Here is an updated webrev.01; 
http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.01/ 



Dmitry

On 20.09.2016 12:48, Dmitry Samersoff wrote:

Dmitry,

Please, also change

!strcmp(...) to strcmp(...) == 0

because semantically strcmp result is not a boolean false.

-Dmitry

On 2016-09-20 06:38, serguei.spit...@oracle.com wrote:

Hi Dmitry,


Thanks a lot for this additional test coverage and discovering new bug:
   https://bugs.openjdk.java.net/browse/JDK-8165681

The tests look pretty good to me.

A couple of minor comments.

Dots are missed in the .c files comments.


http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/test/serviceability/jvmti



  265 printf("Expecting to find '%s' class in ClassLoad events
duringVM start phase.\n", EXPECTED_SIGNATURE);
  266 if (class_in_class_load_events_vm_start == JNI_FALSE) {
  267 throw_exc(env, "Unable to find expected class in
ClassLoad events duringstart phase!\n");
  268 return FAILED;
  269 }
  270
  271 if (class_prepare_events_vm_start_count == 0) {
  272 throw_exc(env, "Didn't get ClassPrepare events in start
phase!\n");
  273 return FAILED;
  274 }
  275
  276 printf("Expecting to find '%s' class in ClassPrepare events
duringVM start phase.\n", EXPECTED_SIGNATURE);
  277 if (class_in_class_prepare_events_vm_start == JNI_FALSE) {
  278 throw_exc(env, "Unable to find expected class in
ClassPrepare events duringstart phase!\n");
  279 return FAILED;
  280 }


Could you, please, replace "start phase" with "early start phase"?
It will be more clear that the start phase mode is "early".

  288 if (class_in_class_prepare_events_vm_start == JNI_TRUE) {
  289 throw_exc(env, "Class is found in ClassLoad events
duringVM Start phase!\n");
  290 return FAILED;
  291 }
  292
  293 printf("Expecting that '%s' class is absent in ClassPrepare
events.\n", EXPECTED_SIGNATURE);
  294 if (class_in_class_prepare_events_vm_start == JNI_TRUE) {
  295 throw_exc(env, "Class is found in ClassPrepare events
duringVM Start phase!\n");
  296 return FAILED;
  297 }

Could you, please, replace "VM Start phase" with "normal VM start
phase"?It will be more clear that the start phase mode is "normal".
Thanks, Serguei On 9/19/16 05:47, Dmitry Dmitriev wrote:

Hello, Please review new tests for module aware agents. There are 3
tests: 1) MAAClassFileLoadHook.java - verifies ClassFileLoadHook event
with different combinations of can_generate_early_vmstart and
can_generate_early_class_hook_events JVMTI capabilities. Expects to
find(or not) class from java.base module in the right VM phase. 2)
MAAClassLoadPrepare.java - verifies ClassLoad and ClassPrepare events
with and without can_generate_early_vmstart JVMTI capability. Expects
to find(or not) class from java.base module in the VM start phase. 3)
MAAThreadStart.java - verifies ThreadStart event with
can_generate_early_vmstart JVMTI capability. Expect to find events in
the VM start phase. JBS:
https://bugs.openjdk.java.net/browse/JDK-8150758 webrev.00:
http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/
 Testing:
RBT on all platforms Thanks, Dmitry

Re: RFR: 8150758: [TESTBUG] need jvmti tests for module aware agents

[Dmitry Samersoff]

Dmitry,

Please, also change

!strcmp(...) to strcmp(...) == 0

because semantically strcmp result is not a boolean false.

-Dmitry

On 2016-09-20 06:38, serguei.spit...@oracle.com wrote:
> Hi Dmitry,
> 
> 
> Thanks a lot for this additional test coverage and discovering new bug:
>   https://bugs.openjdk.java.net/browse/JDK-8165681
> 
> The tests look pretty good to me.
> 
> A couple of minor comments.
> 
> Dots are missed in the .c files comments.
> 
> 
> http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/test/serviceability/jvmti
> 
> 
> 
>  265 printf("Expecting to find '%s' class in ClassLoad events
> duringVM start phase.\n", EXPECTED_SIGNATURE);
>  266 if (class_in_class_load_events_vm_start == JNI_FALSE) {
>  267 throw_exc(env, "Unable to find expected class in
> ClassLoad events duringstart phase!\n");
>  268 return FAILED;
>  269 }
>  270
>  271 if (class_prepare_events_vm_start_count == 0) {
>  272 throw_exc(env, "Didn't get ClassPrepare events in start
> phase!\n");
>  273 return FAILED;
>  274 }
>  275
>  276 printf("Expecting to find '%s' class in ClassPrepare events
> duringVM start phase.\n", EXPECTED_SIGNATURE);
>  277 if (class_in_class_prepare_events_vm_start == JNI_FALSE) {
>  278 throw_exc(env, "Unable to find expected class in
> ClassPrepare events duringstart phase!\n");
>  279 return FAILED;
>  280 }
> 
> 
>Could you, please, replace "start phase" with "early start phase"?   
> It will be more clear that the start phase mode is "early".
> 
>  288 if (class_in_class_prepare_events_vm_start == JNI_TRUE) {
>  289 throw_exc(env, "Class is found in ClassLoad events
> duringVM Start phase!\n");
>  290 return FAILED;
>  291 }
>  292
>  293 printf("Expecting that '%s' class is absent in ClassPrepare
> events.\n", EXPECTED_SIGNATURE);
>  294 if (class_in_class_prepare_events_vm_start == JNI_TRUE) {
>  295 throw_exc(env, "Class is found in ClassPrepare events
> duringVM Start phase!\n");
>  296 return FAILED;
>  297 }
> 
>Could you, please, replace "VM Start phase" with "normal VM start
> phase"?It will be more clear that the start phase mode is "normal".
> Thanks, Serguei On 9/19/16 05:47, Dmitry Dmitriev wrote:
>> Hello, Please review new tests for module aware agents. There are 3
>> tests: 1) MAAClassFileLoadHook.java - verifies ClassFileLoadHook event
>> with different combinations of can_generate_early_vmstart and
>> can_generate_early_class_hook_events JVMTI capabilities. Expects to
>> find(or not) class from java.base module in the right VM phase. 2)
>> MAAClassLoadPrepare.java - verifies ClassLoad and ClassPrepare events
>> with and without can_generate_early_vmstart JVMTI capability. Expects
>> to find(or not) class from java.base module in the VM start phase. 3)
>> MAAThreadStart.java - verifies ThreadStart event with
>> can_generate_early_vmstart JVMTI capability. Expect to find events in
>> the VM start phase. JBS:
>> https://bugs.openjdk.java.net/browse/JDK-8150758 webrev.00:
>> http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/
>>  Testing:
>> RBT on all platforms Thanks, Dmitry 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.

Re: RFR: 8150758: [TESTBUG] need jvmti tests for module aware agents

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

Hi Dmitry,


Thanks a lot for this additional test coverage and discovering new bug:
  https://bugs.openjdk.java.net/browse/JDK-8165681

The tests look pretty good to me.

A couple of minor comments.

Dots are missed in the .c files comments.


http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/test/serviceability/jvmti


 265 printf("Expecting to find '%s' class in ClassLoad events duringVM start 
phase.\n", EXPECTED_SIGNATURE);
 266 if (class_in_class_load_events_vm_start == JNI_FALSE) {
 267 throw_exc(env, "Unable to find expected class in ClassLoad events 
duringstart phase!\n");
 268 return FAILED;
 269 }
 270
 271 if (class_prepare_events_vm_start_count == 0) {
 272 throw_exc(env, "Didn't get ClassPrepare events in start 
phase!\n");
 273 return FAILED;
 274 }
 275
 276 printf("Expecting to find '%s' class in ClassPrepare events duringVM 
start phase.\n", EXPECTED_SIGNATURE);
 277 if (class_in_class_prepare_events_vm_start == JNI_FALSE) {
 278 throw_exc(env, "Unable to find expected class in ClassPrepare 
events duringstart phase!\n");
 279 return FAILED;
 280 }


   Could you, please, replace "start phase" with "early start phase"? 
   It will be more clear that the start phase mode is "early".


 288 if (class_in_class_prepare_events_vm_start == JNI_TRUE) {
 289 throw_exc(env, "Class is found in ClassLoad events duringVM Start 
phase!\n");
 290 return FAILED;
 291 }
 292
 293 printf("Expecting that '%s' class is absent in ClassPrepare 
events.\n", EXPECTED_SIGNATURE);
 294 if (class_in_class_prepare_events_vm_start == JNI_TRUE) {
 295 throw_exc(env, "Class is found in ClassPrepare events duringVM 
Start phase!\n");
 296 return FAILED;
 297 }

   Could you, please, replace "VM Start phase" with "normal VM start 
phase"?It will be more clear that the start phase mode is "normal". 
Thanks, Serguei On 9/19/16 05:47, Dmitry Dmitriev wrote:
Hello, Please review new tests for module aware agents. There are 3 
tests: 1) MAAClassFileLoadHook.java - verifies ClassFileLoadHook event 
with different combinations of can_generate_early_vmstart and 
can_generate_early_class_hook_events JVMTI capabilities. Expects to 
find(or not) class from java.base module in the right VM phase. 2) 
MAAClassLoadPrepare.java - verifies ClassLoad and ClassPrepare events 
with and without can_generate_early_vmstart JVMTI capability. Expects 
to find(or not) class from java.base module in the VM start phase. 3) 
MAAThreadStart.java - verifies ThreadStart event with 
can_generate_early_vmstart JVMTI capability. Expect to find events in 
the VM start phase. JBS: 
https://bugs.openjdk.java.net/browse/JDK-8150758 webrev.00: 
http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/ 
 Testing: 
RBT on all platforms Thanks, Dmitry 

RFR: 8150758: [TESTBUG] need jvmti tests for module aware agents

[Dmitry Dmitriev]

Hello,

Please review new tests for module aware agents. There are 3 tests:
1) MAAClassFileLoadHook.java - verifies ClassFileLoadHook event with 
different combinations of can_generate_early_vmstart and 
can_generate_early_class_hook_events JVMTI capabilities. Expects to 
find(or not) class from java.base module in the right VM phase.
2) MAAClassLoadPrepare.java - verifies ClassLoad and ClassPrepare events 
with and without can_generate_early_vmstart JVMTI capability. Expects to 
find(or not) class from java.base module in the VM start phase.
3) MAAThreadStart.java - verifies ThreadStart event with 
can_generate_early_vmstart JVMTI capability. Expect to find events in 
the VM start phase.


JBS: https://bugs.openjdk.java.net/browse/JDK-8150758
webrev.00: http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/ 


Testing: RBT on all platforms

Thanks,
Dmitry