[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8150758: [TESTBUG] need jvmti tests for module aware agents
From:       Dmitry Dmitriev <dmitry.dmitriev () oracle ! com>
Date:       2016-09-20 10:45:46
Message-ID: 2de3da26-eaf3-2346-079d-d96dd1d3b354 () oracle ! com
[Download RAW message or body]

Dmitry, Sergeui, thank you for the review!

For the record, webrev.02: 
http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.02/ 
<http://cr.openjdk.java.net/%7Eddmitriev/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/
>> <http://cr.openjdk.java.net/%7Eddmitriev/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.spitsyn@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/
>>>>> <http://cr.openjdk.java.net/%7Eddmitriev/8150758/webrev.00/> Testing:
>>>>> RBT on all platforms Thanks, Dmitry
>

[prev in list] [next in list] [prev in thread] [next in thread]