RFR(M) 8192921 Improve CDS support for custom loaders

Ioi Lam ioi.lam at oracle.com
Wed Feb 13 13:53:55 UTC 2019


Hi Yumin,

Thanks for the bug report.

Try running your test case without CDS, and see if the 
(failed_type_symbol != NULL) error ever happens. If not, that means when 
running with CDS, the itable was not restored properly.

I suspect it has something to do with the code in 
SystemDictionaryShared::check_hierarchy() and SuperTypeReplacer. To 
diagnose what's wrong, try modifying the following

SystemDictionary::check_signature_loaders(m->signature(),
method_holder_loader,
interface_loader,
                                                       true, CHECK);
           if (failed_type_symbol != NULL) {
+ SystemDictionary::check_signature_loaders(m->signature(),
+ method_holder_loader,
+ interface_loader,
+                                                     true, CHECK);

and set a break point in the newly added code. When the breakpoint is 
hit, step inside to see exactly where the failure happens. That way we 
can find out whether SuperTypeReplacer did the replacements correctly.

Thanks

- Ioi


On 2/13/19 12:46 AM, yumin qi wrote:
> Hi, Ioi
>
>   When I port this to jdk8u, there is a failure at the case like:
>
> #0  0x00007ffff677c23b in 
> klassItable::initialize_itable_for_interface(int, KlassHandle, bool, 
> Thread*) (this=0x7ffed4030ce0, method_table_offset=824, interf_h=..., 
> checkconstraints=true, __the_thread__=0x7ffe4a512000) at 
> /home/yumin.qi/ws/openjdk/8/jdk8u/hotspot/src/share/vm/oops/klassVtable.cpp:1271
> #1  0x00007ffff677b867 in klassItable::initialize_itable(bool, 
> Thread*) (this=0x7ffed4030ce0, checkconstraints=true, 
> __the_thread__=0x7ffe4a512000)
>     at 
> /home/yumin.qi/ws/openjdk/8/jdk8u/hotspot/src/share/vm/oops/klassVtable.cpp:1130
> #2  0x00007ffff65e5019 in 
> InstanceKlass::link_class_impl(instanceKlassHandle, bool, Thread*) 
> (this_oop=..., throw_verifyerror=true, __the_thread__=0x7ffe4a512000)
>     at 
> /home/yumin.qi/ws/openjdk/8/jdk8u/hotspot/src/share/vm/oops/instanceKlass.cpp:755
> #3  0x00007ffff65e4afe in 
> InstanceKlass::link_class_impl(instanceKlassHandle, bool, Thread*) 
> (this_oop=..., throw_verifyerror=true, __the_thread__=0x7ffe4a512000)
>     at 
> /home/yumin.qi/ws/openjdk/8/jdk8u/hotspot/src/share/vm/oops/instanceKlass.cpp:676
> #4  0x00007ffff65e46fd in InstanceKlass::link_class(Thread*) 
> (this=0x809177b38, __the_thread__=0x7ffe4a512000) at 
> /home/yumin.qi/ws/openjdk/8/jdk8u/hotspot/src/share/vm/oops/instanceKlass.cpp:617
> #5  0x00007ffff65e56e3 in 
> InstanceKlass::initialize_impl(instanceKlassHandle, Thread*) 
> (this_oop=..., __the_thread__=0x7ffe4a512000) at 
> /home/yumin.qi/ws/openjdk/8/jdk8u/hotspot/src/share/vm/oops/instanceKlass.cpp:845
> #6  0x00007ffff65e4464 in InstanceKlass::initialize(Thread*) 
> (this=0x809177b38, __the_thread__=0x7ffe4a512000) at 
> /home/yumin.qi/ws/openjdk/8/jdk8u/hotspot/src/share/vm/oops/instanceKlass.cpp:572
> #7  0x00007ffff663f672 in InterpreterRuntime::_new(JavaThread*, 
> ConstantPool*, int) (thread=0x7ffe4a512000, pool=0x80915e480, index=4)
>     at 
> /home/yumin.qi/ws/openjdk/8/jdk8u/hotspot/src/share/vm/interpreter/interpreterRuntime.cpp:158
>
> from 0 is a break point inside throwing exception:
>
>       if (checkconstraints) {
>         Handle method_holder_loader (THREAD, 
> target->method_holder()->class_loader());
>         if (method_holder_loader() != interface_loader()) {
>           ResourceMark rm(THREAD);
>           Symbol* failed_type_symbol =
> SystemDictionary::check_signature_loaders(m->signature(),
>   method_holder_loader,
>   interface_loader,
>   true, CHECK);
>           if (failed_type_symbol != NULL) {
>             const char* msg = "loader constraint violation in interface "
>               "itable initialization: when resolving method \"%s\" the 
> class"
>               " loader (instance of %s) of the current class, %s, "
>               "and the class loader (instance of %s) for interface "
>               "%s have different Class objects for the type %s "
>               "used in the signature";
>             char* sig = target()->name_and_sig_as_C_string(); 
>  ////////////////////    <======== break here
>             const char* loader1 = 
> SystemDictionary::loader_name(method_holder_loader());
>             char* current = _klass->name()->as_C_string();
>             const char* loader2 = 
> SystemDictionary::loader_name(interface_loader());
>             char* iface = 
> InstanceKlass::cast(interf_h())->name()->as_C_string();
>             char* failed_type_name = failed_type_symbol->as_C_string();
>             size_t buflen = strlen(msg) + strlen(sig) + strlen(loader1) +
>               strlen(current) + strlen(loader2) + strlen(iface) +
>               strlen(failed_type_name);
>             char* buf = NEW_RESOURCE_ARRAY_IN_THREAD(THREAD, char, 
> buflen);
>             jio_snprintf(buf, buflen, msg, sig, loader1, current, loader2,
>                          iface, failed_type_name);
> THROW_MSG(vmSymbols::java_lang_LinkageError(), buf);
>           }
>         }
>       }
>
> The structure of the classes are:  (the numbers are dumped class id in 
> class list file)
> Log4jLogger:8302
>       extends  LoggerSupport:8299
>       implements Logger:8271
>
> LoggerSupport:8299
>       extends Object:1
>       implements Logger:8271
>
> Logger:8271
>       extends ActivateOption:8268
>
> ActivateOption:8268
>      is an interface.
>
> Continue from the break point will almost immediately crash at 
> class_loader_data  is 0x0 for the interface function of ActivateOption.
> The runtime resolved method (target)'s holder's loader is different 
> from the interface loader.
> method holder is LoggerSupport and interface is ActivateOption.
>
> Any idea what is wrong here?
>
> Thanks
> Yumin
>
>
>
>
>
>
>
>
>
>
>
>
> On Mon, Jan 7, 2019 at 3:11 PM yumin qi <yumin.qi at gmail.com 
> <mailto:yumin.qi at gmail.com>> wrote:
>
>     Hi, Ioi
>
>        Thanks for fixing the custom loader issue in CDS. It looks good! Only a few minor suggestions.
>
>     1) classListParser.cpp:
>
>           } else if (parse_int_option("super:", &_super)) {
>     - check_already_loaded("Super class", _super);
>     - continue;
>     +
>           }
>
>     According to current design, DumpLoadedClassList will output
>     java/lang/Object id : 1
>     ....
>     So except for Object, every other class will have "super : <id>"
>     There is no checking for return false. If format damages leads to
>     fail to parse the value, should we do something here?
>
>     2) systemDictionaryShared.cpp:
>
>     bool SystemDictionaryShared::check_hierarchy(...):
>
>     Klass* k = resolve_super_or_fail(ik->name(),
>     expected_intf->name(), class_loader, protection_domain, false,
>     CHECK_NULL);
>
>     Since the function return bool, I think here should be
>     CHECK_false, that is, with Exception, it returns false.
>
>     3) TestCommon.java:
>
>     + static byte[] asciibytes(String s) {
>     + byte b[] = new byte[s.length()];
>     + for (int i=0; i<b.length; i++) {
>     + b[i] = (byte)s.charAt(i);
>     + }
>     + return b;
>     + }
>
>     Maybe just use String.getBytes:
>
>     return s.getBytes(StandardCharsets.US_ASCII);
>
>     4) For many loops (C/C++ part), space should be added before and after "=", "<". I don't know if it is still recommended coding in that style.
>
>     Thanks
>
>     Yumin
>
>
>     On Fri, Dec 21, 2018 at 9:19 PM Ioi Lam <ioi.lam at oracle.com
>     <mailto:ioi.lam at oracle.com>> wrote:
>
>         http://cr.openjdk.java.net/~iklam/jdk13/8192921-cds-cust-loader-improve.v01/
>         https://bugs.openjdk.java.net/browse/JDK-8192921
>
>
>         ISSUE:
>
>         In JDK 12, there's experimental support for custom loader in CDS.
>         However, there are limitations:
>
>         1. if two or more custom loaders want to load a class named
>         "A", only
>             one of those loaders can actually succeed in loading an
>         archived
>             class.
>
>         2. the classlist needs to be hand-crafted (or by a script that
>         parses
>             the output of -Xlog:class+load=debug).
>
>         FIX:
>
>         1. Now you can archive multiple classes of the same name. At run
>             time, we select an archived class that
>             (a) matches the fingerprint of the class file, and
>             (b) matches the class hierarchy of the current loader.
>
>             (b) is the tricky part. See
>         SystemDictionaryShared::has_same_hierarchy_for_unregistered_class
>
>             For a test case, see DuplicatedClassesTest.java
>
>         2. -XX:DumpLoadedClassList now automatically includes classes
>         loaded
>             by custom loaders. The generated class list includes all the
>             necessary attributes for custom loader support, such as
>         "super:",
>             "source:" and "interface:".
>
>         I have tested with Eclipse IDE, which loads most of its
>         classes with
>         custom loaders. I am planning to add more test cases.
>
>         Thanks
>         - Ioi
>
>


More information about the hotspot-runtime-dev mailing list