RFR: 8267347: CDS record_linking_constraint asserts with unregistered class [v2]

Ioi Lam iklam at openjdk.java.net
Mon May 24 21:57:53 UTC 2021


On Fri, 21 May 2021 21:40:32 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> Ioi Lam has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
>> 
>>  - Merge branch 'master' into 8267347-record_linking_constraint-asserts
>>  - PR comments by @yminqi and @calvinccheung
>>  - fixed windows testing
>>  - 8267347: CDS record_linking_constraint asserts with unregistered class
>
> Looks good. Just one question and one nit.

Thanks to @calvinccheung and @yminqi for the review.

> test/hotspot/jtreg/runtime/cds/appcds/loaderConstraints/LoaderConstraintsTest.java line 52:
> 
>> 50:         httpHandlerClass,
>> 51:         httpExchangeClass,
>> 52:         Asserts.class.getName(),
> 
> Why other class names don't need to do `replace(".", "/")`?

The problem is that the classlist requires the slash separator. I.e., `com/sun/net/httpserver/HttpHandler` must be used. In contrast, `ClassFileInstaller.writeJar()` allows both `jdk.test.lib.Asserts` and `jdk/test/lib/Asserts`. Since the `Asserts` class is used only by `ClassFileInstaller`, I left it as is.

To make it easier to write test cases, I have filed https://bugs.openjdk.java.net/browse/JDK-8267624 "CDS classlist should allow both . and / in class names"

> test/hotspot/jtreg/runtime/cds/appcds/test-classes/CustomAppLoader.java line 32:
> 
>> 30: 
>> 31: // This is a handy class for running an application inside a custom class loader. This
>> 32: // is used for testing CDS handling of unregistered classes (i.e., archved classes loaded
> 
> Typo: archved -> archived

Fixed.

-------------

PR: https://git.openjdk.java.net/jdk/pull/4147


More information about the hotspot-runtime-dev mailing list