RFR: 8358680: AOT cache creation fails: no strings should have been added

[Coleen Phillimore]

On Mon, 16 Jun 2025 03:30:40 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> Background: when writing the string table in the AOT cache, we do this:
> 
> 1. Find out the number of strings in the interned string table
> 2. Allocate Java object arrays that are large enough to store these strings. These arrays are used by `StringTable::lookup_shared()` in the production run.
> 3. Enter safepoint
> 4. Copy the strings into the arrays
> 
> This bug happened because:
> 
> - Step 1 is not thread safe, so it may be reading a stale version of `_items_count`
> - JIT compiler threads may create more interned strings after step 1
> 
> This PR attempts to fix both issues.

I have a couple of comments. Overall, the approach seems good.

src/hotspot/share/classfile/stringTable.cpp line 351:

> 349: }
> 350: 
> 351: size_t StringTable::items_count() {

I think there's a convention to make accessor functions that use acquire semantics to be named items_count_acquire().

src/hotspot/share/classfile/stringTable.cpp line 970:

> 968:   // This flag will be cleared after intern table dumping has completed, so we can run the
> 969:   // compiler again (for future AOT method compilation, etc).
> 970:   DEBUG_ONLY(Atomic::release_store(&_disable_interning_during_cds_dump, 1));

I think atomics work with bool or is this a refcount ?

src/hotspot/share/compiler/compileTask.cpp line 93:

> 91: void CompileTask::wait_for_no_active_tasks() {
> 92:   MonitorLocker locker(CompileTaskAlloc_lock);
> 93:   while (_active_tasks > 0) {

Doesn't this have to have an Atomic::load() to make it re-read in the loop? Even though it's after we reacquire the lock.

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

Changes requested by coleenp (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/25816#pullrequestreview-2936227969
PR Review Comment: https://git.openjdk.org/jdk/pull/25816#discussion_r2152761032
PR Review Comment: https://git.openjdk.org/jdk/pull/25816#discussion_r2152619475
PR Review Comment: https://git.openjdk.org/jdk/pull/25816#discussion_r2152626132