RFR 8030633: nsk/jvmti/RedefineClasses/StressRedefine failed invalid method ordering, length on Solaris

Coleen Phillimore coleen.phillimore at oracle.com
Thu Dec 19 11:57:24 PST 2013


Hi Dmitry,

Thank you for looking at the code.  I agree with you about the nits, but 
since I didn't change those lines, I want to leave them as is. They seem 
safe to change but then again, it's the end of the release and any 
changes could be risky.

On 12/19/2013 02:42 PM, Dmitry Samersoff wrote:
> Coleen,
>
> Looks good for me.
>
> Few cosmetic nits to mention:
>
> defaultMethods.cpp:
>
> ll.1042. 1052
>
> It might be more readable if we replace
> klass->methods()->length() to original_methods->length();
>
> ll. 1054
>
> It might be good to call
> sort(new_methods) only if (new_methods->length() > 0)

new_methods->length() will always be >0 because we only get here if 
there are overpass methods to add to the methods list.  It would be nice 
to encode that assumption but not just now.

Thanks,
Coleen
> -Dmitry
>
>
> On 2013-12-19 22:59, Coleen Phillimore wrote:
>> On 12/19/2013 12:42 PM, Karen Kinnear wrote:
>>> Coleen,
>>>
>>> Code looks good.
>>> In particular I was checking we handle both the null and
>>> empty_int_array cases - I assume
>>> that we need to handle both due to bootstrapping issues.
>> The original method array is the_empty_int_array() and not null but I
>> added null checks in it because Dan had the same comment too.
>>
>> See:
>>
>> http://cr.openjdk.java.net/~coleenp/8030633_2/
>>
>> Thanks,
>> Coleen
>>
>>> thanks,
>>> Karen
>>>
>>> On Dec 19, 2013, at 11:40 AM, Coleen Phillimore wrote:
>>>
>>>> Summary: A method with no declared methods was getting an AME
>>>> overpass method with the latest change.  The method_ordering array
>>>> was not updated for the new methods.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8030633/
>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8030633
>>>>
>>>> Tested with existing tests
>>>> nsk/jvmti/RedefineClasses/StressRedefine{WithoutBytecodeCorruption}.
>>>> Needs native code to create new test in jtreg, so I didn't add one.
>>>>
>>>> Thanks,
>>>> Coleen
>



More information about the hotspot-runtime-dev mailing list