On Wed, 30 Apr 2025 08:47:59 GMT, Axel Boldt-Christmas <abold...@openjdk.org>
wrote:
>> Ioi Lam has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Renamed the internal field ReferenceQueue.NULL to NULL_QUEUE to avoid
>> failing hotspot/jtreg/sources/TestNoNULL.java
>
> src/hotspot/share/cds/aotArtifactFinder.cpp line 77:
>
>> 75: // into the AOT cache -- that will be decided by the code below.
>> 76: SystemDictionaryShared::finish_exclusion_checks();
>> 77: AOTReferenceObjSupport::init_keep_alive_objs_table();
>
> This should be guarded with `INCLUDE_CDS_JAVA_HEAP` somehow. I guess the
> cleanest way would be to use `NOT_CDS_JAVA_HEAP_RETURN` in
> `aotReferenceObjSupport.hpp`
I added `NOT_CDS_JAVA_HEAP_RETURN` to the declaration of
`init_keep_alive_objs_table`. All the other functions are called only inside
`INCLUDE_CDS_JAVA_HEAP` blocks, so I didn't change them to avoid cluttering the
code.
> src/hotspot/share/cds/aotReferenceObjSupport.cpp line 95:
>
>> 93: #if INCLUDE_CDS_JAVA_HEAP
>> 94:
>> 95:
>
> Suggestion:
Fixed.
> test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/WeakReferenceTest.java
> line 233:
>
>> 231: if (ref2.get() == null) {
>> 232: throw new RuntimeException("ref2.get() should not be null");
>> 233: }
>
> The referent of ref2 is never strong. If a GC occurs between makeRef2() and
> this check, we could hit this Exception. The test should either keep it
> strongly reachable until after this check or just not test wether it is
> reachable before the call to System.gc().
I removed the check that was before the GC.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24757#discussion_r2069169854
PR Review Comment: https://git.openjdk.org/jdk/pull/24757#discussion_r2069169794
PR Review Comment: https://git.openjdk.org/jdk/pull/24757#discussion_r2069169743
