RFR: 8297389: resexhausted003 fails with assert(!thread->owns_locks()) failed: must release all locks when leaving VM [v2]

Tobias Hartmann thartmann at openjdk.org
Mon Nov 28 12:08:47 UTC 2022


On Mon, 28 Nov 2022 12:01:43 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> `Method::build_profiling_method_data` acquires the `MethodData_lock` when initializing `Method::_method_data` to prevent multiple allocations by different threads. The problem is that when metaspace allocation fails and `JvmtiExport::should_post_resource_exhausted()` is set, we assert during the `ThreadToNativeFromVM` transition in JVMTI code.
>> 
>> Since concurrent initialization is a rare event, I suggest to get rid of the lock and perform the initialization with a `cmpxchg`, similar to how method counters are initialized:
>> https://github.com/openjdk/jdk/blob/f4b5065c37e86f4b2ca26da6ce678febe4a52950/src/hotspot/share/oops/method.cpp#L644-L646
>> 
>> Since [current code](https://github.com/openjdk/jdk/blob/f4b5065c37e86f4b2ca26da6ce678febe4a52950/src/hotspot/share/oops/method.inline.hpp#L41-L46) in `Method::set_method_data` uses a `Atomic::release_store`, I added a `OrderAccess::release()`.
>> 
>> Thanks,
>> Tobias
>
> Tobias Hartmann has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Removed OrderAccess::release, added assert and adjusted ciReplay code

Thanks for the review, David!

I added an assert to `MethodData::allocate` and fixed the ciReplay code which only ever gets executed by a single thread by removing the lock. I also removed the `OrderAccess::release()`.

Regarding the overhead of going lock-free: I temporarily added code that counts the number of times that multiple threads attempt initialization and asserts if it's more that 50x. This triggers in 25 out of 203.772 runs (tests from tier1 to tier3). The average size of these allocations is around 88 words (704 bytes). I therefore think it's fine to avoid a lock in this case. Also, we already use a lock-free mechanism for `method::build_method_counters`.

What do you think?

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

PR: https://git.openjdk.org/jdk/pull/11316


More information about the hotspot-dev mailing list