RFR(M) 8192921 Improve CDS support for custom loaders
yumin qi
yumin.qi at gmail.com
Thu Feb 14 06:12:35 UTC 2019
Hi, Ioi
Without CDS, it never hits the break point.
With CDS, turn on -XX:+TraceLoaderConstraints, I got following output:
[Adding new constraint for name: com/taobao/middleware/logger/Level,
loader[0]: com/taobao/pandora/service/loader/ModuleClassLoader, loader[1]:
com/taobao/pandora/service/loader/ModuleClassLoader ]
[Setting class object in existing constraint for name:
com/taobao/middleware/logger/Level and loader
com/taobao/pandora/service/loader/ModuleClassLoader ]
[Failed to add constraint for name: com/taobao/middleware/logger/Logger,
loader[0]: com/taobao/pandora/service/loader/ModuleClassLoader, loader[1]:
com/taobao/pandora/service/loader/ModuleClassLoader, Reason: the class
objects presented by loader[0] and loader[1] are different ]
Trace next is throwing exception at break point, then crashed at
_class_loader_data is 0x0.
I tried to hard watch the change of _class_loader_data by its address
*(int*)&(the problematic klass)->_class_loader_data, it did not catch the
change.
some time the reason is 3:
switch(failure_code) {
case 1: reason = "the class objects presented by loader[0] and
loader[1]"
" are different"; break;
case 2: reason = "the class object presented by loader[0] does not
match"
" the stored class object in the constraint"; break;
case 3: reason = "the class object presented by loader[1] does not
match"
" the stored class object in the constraint"; break;
default: reason = "unknown reason code";
}
It looks after the exception, the _class_loader_data of the interface is
changed into 0x0.
Note the interf_h()->class_loader() in at beginning of the function:
void klassItable::initialize_itable_for_interface(int method_table_offset,
KlassHandle interf_h, bool checkconstraints, T
RAPS) {
Array<Method*>* methods = InstanceKlass::cast(interf_h())->methods();
int nof_methods = methods->length();
HandleMark hm;
Handle interface_loader (THREAD,
InstanceKlass::cast(interf_h())->class_loader());
.....
The check constrain is bellow so it could from different thread result.
When check_hierarchy, replace the super type with same hirerachy version,
for itable:
template <class ITERATOR>
inline void InstanceKlass::itable_pointers_do(ITERATOR* iter) {
if (itable_length() > 0) {
itableOffsetEntry* ioe = (itableOffsetEntry*)start_of_itable();
int method_table_offset_in_words = ioe->offset()/wordSize;
int nof_interfaces = (method_table_offset_in_words -
itable_offset_in_words())
/ itableOffsetEntry::size();
for (int i = 0; i < nof_interfaces; i ++, ioe ++) {
if (ioe->interface_klass() != NULL) {
iter->push(ioe->interface_klass_addr()); //// <------ if found
interface replace
itableMethodEntry* ime = ioe->first_method_entry(this);
int n =
klassItable::method_count_for_interface(ioe->interface_klass());
for (int index = 0; index < n; index ++) {
iter->push(ime[index].method_addr()); //// <----- here we find
a replace holder, do we make sure the replaced holder is same as above
replaced interface?
}
}
}
}
}
Thanks
Yumin
On Wed, Feb 13, 2019 at 9:53 PM Ioi Lam <ioi.lam at oracle.com> wrote:
> 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> 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> 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