Re: RFR: 8275731: CDS archived enums objects are recreated at runtime [v4]
On Thu, 17 Feb 2022 23:20:41 GMT, Calvin Cheung wrote: >> Ioi Lam has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Use InstanceKlass::do_local_static_fields for some field iterations > > Looks good. Minor comment below. > Also, several files with copyright year 2021 need updating. Thanks @calvinccheung and @coleenp for the review. Passed tiers 1-5. - PR: https://git.openjdk.java.net/jdk/pull/6653
Re: RFR: 8275731: CDS archived enums objects are recreated at runtime [v4]
On Wed, 19 Jan 2022 05:47:57 GMT, Ioi Lam wrote:
>> **Background:**
>>
>> In the Java Language, Enums can be tested for equality, so the constants in
>> an Enum type must be unique. Javac compiles an enum declaration like this:
>>
>>
>> public enum Day { SUNDAY, MONDAY ... }
>>
>>
>> to
>>
>>
>> public class Day extends java.lang.Enum {
>> public static final SUNDAY = new Day("SUNDAY");
>> public static final MONDAY = new Day("MONDAY"); ...
>> }
>>
>>
>> With CDS archived heap objects, `Day::` is executed twice: once
>> during `java -Xshare:dump`, and once during normal JVM execution. If the
>> archived heap objects references one of the Enum constants created at dump
>> time, we will violate the uniqueness requirements of the Enum constants at
>> runtime. See the test case in the description of
>> [JDK-8275731](https://bugs.openjdk.java.net/browse/JDK-8275731)
>>
>> **Fix:**
>>
>> During -Xshare:dump, if we discovered that an Enum constant of type X is
>> archived, we archive all constants of type X. At Runtime, type X will skip
>> the normal execution of `X::`. Instead, we run
>> `HeapShared::initialize_enum_klass()` to retrieve all the constants of X
>> that were saved at dump time.
>>
>> This is safe as we know that `X::` has no observable side effect --
>> it only creates the constants of type X, as well as the synthetic value
>> `X::$VALUES`, which cannot be observed until X is fully initialized.
>>
>> **Verification:**
>>
>> To avoid future problems, I added a new tool, CDSHeapVerifier, to look for
>> similar problems where the archived heap objects reference a static field
>> that may be recreated at runtime. There are some manual steps involved, but
>> I analyzed the potential problems found by the tool are they are all safe
>> (after the current bug is fixed). See cdsHeapVerifier.cpp for gory details.
>> An example trace of this tool can be found at
>> https://bugs.openjdk.java.net/secure/attachment/97242/enum_warning.txt
>>
>> **Testing:**
>>
>> Passed Oracle CI tiers 1-4. WIll run tier 5 as well.
>
> Ioi Lam has updated the pull request incrementally with one additional commit
> since the last revision:
>
> Use InstanceKlass::do_local_static_fields for some field iterations
Looks good. Minor comment below.
Also, several files with copyright year 2021 need updating.
src/hotspot/share/cds/cdsHeapVerifier.cpp line 63:
> 61: // class Bar {
> 62: // // this field is initialized in both CDS dump time and runtime.
> 63: // static final Bar bar = new Bar;
`new Bar` should be `new Bar()`?
-
Marked as reviewed by ccheung (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/6653
Re: RFR: 8275731: CDS archived enums objects are recreated at runtime [v4]
> **Background:**
>
> In the Java Language, Enums can be tested for equality, so the constants in
> an Enum type must be unique. Javac compiles an enum declaration like this:
>
>
> public enum Day { SUNDAY, MONDAY ... }
>
>
> to
>
>
> public class Day extends java.lang.Enum {
> public static final SUNDAY = new Day("SUNDAY");
> public static final MONDAY = new Day("MONDAY"); ...
> }
>
>
> With CDS archived heap objects, `Day::` is executed twice: once
> during `java -Xshare:dump`, and once during normal JVM execution. If the
> archived heap objects references one of the Enum constants created at dump
> time, we will violate the uniqueness requirements of the Enum constants at
> runtime. See the test case in the description of
> [JDK-8275731](https://bugs.openjdk.java.net/browse/JDK-8275731)
>
> **Fix:**
>
> During -Xshare:dump, if we discovered that an Enum constant of type X is
> archived, we archive all constants of type X. At Runtime, type X will skip
> the normal execution of `X::`. Instead, we run
> `HeapShared::initialize_enum_klass()` to retrieve all the constants of X that
> were saved at dump time.
>
> This is safe as we know that `X::` has no observable side effect --
> it only creates the constants of type X, as well as the synthetic value
> `X::$VALUES`, which cannot be observed until X is fully initialized.
>
> **Verification:**
>
> To avoid future problems, I added a new tool, CDSHeapVerifier, to look for
> similar problems where the archived heap objects reference a static field
> that may be recreated at runtime. There are some manual steps involved, but I
> analyzed the potential problems found by the tool are they are all safe
> (after the current bug is fixed). See cdsHeapVerifier.cpp for gory details.
> An example trace of this tool can be found at
> https://bugs.openjdk.java.net/secure/attachment/97242/enum_warning.txt
>
> **Testing:**
>
> Passed Oracle CI tiers 1-4. WIll run tier 5 as well.
Ioi Lam has updated the pull request incrementally with one additional commit
since the last revision:
Use InstanceKlass::do_local_static_fields for some field iterations
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/6653/files
- new: https://git.openjdk.java.net/jdk/pull/6653/files/6e160057..e27d3523
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=6653=03
- incr: https://webrevs.openjdk.java.net/?repo=jdk=6653=02-03
Stats: 150 lines in 2 files changed: 82 ins; 59 del; 9 mod
Patch: https://git.openjdk.java.net/jdk/pull/6653.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/6653/head:pull/6653
PR: https://git.openjdk.java.net/jdk/pull/6653
