RFR: 8344032: InterpreterRuntime::verify_mdp() missing lock while printing MethodData on failure [v2]

[Coleen Phillimore]

On Wed, 13 Nov 2024 18:36:01 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Use ConditionalMutexLocker instead.
>
> src/hotspot/share/oops/methodData.cpp line 1561:
> 
>> 1559: 
>> 1560: void MethodData::print_data_on(outputStream* st) const {
>> 1561:   MutexLocker ml(extra_data_lock(), Mutex::_no_safepoint_check_flag);
> 
> Are we sure nothing calls this method when lock is already held? Safer to use `ConditionalMutexLocker` with self-locked check?

I don't really like ConditionalMutexLocker because it feels like a hack when we don't understand the control flow, but that said, there was a case I found with ciMethodData printing that already has the lock.  It would have failed if any tests hit it.

> src/hotspot/share/oops/methodData.hpp line 2514:
> 
>> 2512:   void clean_method_data(bool always_clean);
>> 2513:   void clean_weak_method_links();
>> 2514:   Mutex* extra_data_lock() const { return const_cast<Mutex*>(&_extra_data_lock); }
> 
> Not sure what this const-ness gains us :)

It gets us to compile line 1561 above in the print function which is const.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22082#discussion_r1841071283
PR Review Comment: https://git.openjdk.org/jdk/pull/22082#discussion_r1841071839