[jdk20] RFR: 8296955: Kitchensink.java failed with "double free or corruption (!prev): <addr>"

[Daniel D. Daugherty]

On Fri, 9 Dec 2022 12:27:42 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> Thanks to Kim for the diagnosis of this problem.  G1 cleans up nmethods in parallel threads during a safepoint and may post or queue unload events to be posted.  The event posting creates jmethodIDs.  The jmethodID code assumes that it's thread safe in a safepoint, which it is not.
> I removed this optimization, which isn't much of an optimization (did run some sanity performance tests) and is unsafe in the modern code.
> Tested with tier1-7, where tier5 found the original bug.

Thumbs up. The 'load_data' -> 'cld' changes were distracting, but then I saw
that the functions had this:

void Method::foo_method(ClassLoaderData* loader_data, ...
  ClassLoaderData* cld = loader_data;

And I can't remember why we did that so very long ago...

Thanks for including the testing info!!

This pattern:

    if (Threads::number_of_threads() == 0 ||
        SafepointSynchronize::is_at_safepoint()) {

is common in JVM/TI code as a way to determine if it is "safe"
to do something without a lock. We probably need to audit the
uses of that pattern and see if there's anything else that has to
be updated in the face of more parallelism during safepoints.

src/hotspot/share/oops/method.cpp line 2185:

> 2183:   // Also have to add the method to the list safely, which the lock
> 2184:   // protects as well.
> 2185:   assert(JmethodIdCreation_lock->owned_by_self(), "sanity check");

I jeep going back and forth about changing this from `assert` to
`guarantee` for at least a few complete test cycles. However, we
are post code fork so it would be difficult to justify switching it
back in 3 or so builds. Switching to a `guarantee` would also
mean another round of perf testing and I don't want you to have
to do that either.

I'll keep my eyes open on the two CIs...

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

Marked as reviewed by dcubed (Reviewer).

PR: https://git.openjdk.org/jdk20/pull/10