On 19/07/18 12:50, Andrew Haley wrote:
> On 07/13/2018 08:36 AM, Andrew Dinn wrote:
>> On 12 July 2018 at 02:34, Martin Balao <mbalao at redhat.com> wrote:>
>> I'd like to propose a fix for JDK-8207151 [1]:
>>>
>>> * http://cr.openjdk.java.net/~mbalao/webrevs/8207151/8207151.webrev.01/
>>> * http://cr.openjdk.java.net/~mbalao/webrevs/8207151/8207151.webrev.01.zip
>>>
>>> This fix has been already reviwed by Andrew Dinn (adinn).
>>
>> I am not an official jdk7u reviewer. However, I can still confirm that I
>> have unofficially reviewed Martin's fix and agree it is correct.
>
> Can you explain the change to me? It's pretty obscure, so a comment
> would not have gone amiss, but at least it'll be on record here.
Sue, I can provide a fix. A comment would indeed have helped but the fix
is actually fairly easy to follow.
The bug arises because of a stale (klass) oop. In the original code the
value of local var from_class is resolved from the incoming handle at
line 902
902 oop from_class_oop = JNIHandles::resolve(from);
903 klassOop from_class = (from_class_oop == NULL)
904 ? (klassOop)NULL
905 :
java_lang_Class::as_klassOop(from_class_oop);
Unfortunately, the call which follows at line 914 can enter the GC
914 jclass result = find_class_from_class_loader(env, h_name, init,
h_loader,
915 h_prot, true, thread);
After that call the handle from will definitely have been updated but
the resolved klassOop in from_class may be stale.
The fix is to re-resolve from_class in the subsequent branch before it
is pass to record_dpeendency. So, the patch changes
917 if (result != NULL) {
918 oop mirror = JNIHandles::resolve_non_null(result);
919 klassOop to_class = java_lang_Class::as_klassOop(mirror);
920 ClassLoaderDependencies::record_dependency(from_class,
to_class, CHECK_NULL);
921 }
to
917 if (result != NULL) {
918 from_class_oop = JNIHandles::resolve(from);
919 from_class = java_lang_Class::as_klassOop(from_class_oop);
920 oop mirror = JNIHandles::resolve_non_null(result);
921 klassOop to_class = java_lang_Class::as_klassOop(mirror);
922 ClassLoaderDependencies::record_dependency(from_class,
to_class, CHECK_NULL);
923 }
Prior to the fix the call to record_dependency was blowing up with a bad
value for the klassOop.
regards,
Andrew Dinn
-----------
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander
