RFR(M) 8192921 Improve CDS support for custom loaders
yumin qi
yumin.qi at gmail.com
Thu Feb 14 10:52:09 UTC 2019
I print out detailed info:
expected: <name> <addr> -> actual: <name> <addr> for data inserting
into the growable array, and others is for replacement:
expected: com.taobao.middleware.logger.option.ActivateOption 0x8091529b8 ->
actual: com.taobao.middleware.logger.option.ActivateOption 0x809153260
0x8091529b8 com.taobao.middleware.logger.option.ActivateOption
-> 0x809153260 com.taobao.middleware.logger.option.ActivateOption
0x8091529b8 com.taobao.middleware.logger.option.ActivateOption
-> 0x809153260 com.taobao.middleware.logger.option.ActivateOption
0x8091529b8 com.taobao.middleware.logger.option.ActivateOption
-> 0x809153260 com.taobao.middleware.logger.option.ActivateOption
expected: com.taobao.middleware.logger.option.ActivateOption 0x809153260 ->
actual: com.taobao.middleware.logger.option.ActivateOption 0x8091529b8
0x809153260 com.taobao.middleware.logger.option.ActivateOption
-> 0x8091529b8 com.taobao.middleware.logger.option.ActivateOption
0x809153260 com.taobao.middleware.logger.option.ActivateOption
-> 0x8091529b8 com.taobao.middleware.logger.option.ActivateOption
0x809153260 com.taobao.middleware.logger.option.ActivateOption
-> 0x8091529b8 com.taobao.middleware.logger.option.ActivateOption
[Extending constraint for name java/lang/Class by adding loader[19]:
com/taobao/pandora/service/loader/ModuleClassLoader ]
expected: com.taobao.middleware.logger.option.ActivateOption 0x8091529b8 ->
actual: com.taobao.middleware.logger.option.ActivateOption 0x8091576f0
expected: com.taobao.middleware.logger.Logger 0x809159ba0 -> actual:
com.taobao.middleware.logger.Logger 0x8091554f8
[Extending constraint for name java/lang/reflect/Method by adding
loader[5]: com/taobao/pandora/service/loader/ModuleClassLoader ]
It looks a circle here: the replace reversed the former replacement?
Thanks
Yumin
On Thu, Feb 14, 2019 at 2:12 PM yumin qi <yumin.qi at gmail.com> wrote:
> 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