RFR: 8306767: Concurrent repacking of extra data in MethodData is potentially unsafe [v16]

[Emanuel Peter]

On Thu, 11 Jan 2024 13:43:25 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fixed typo
>
> src/hotspot/share/ci/ciMethodData.cpp line 96:
> 
>> 94:     // a safepoint. We temporarily release the lock and allow
>> 95:     // safepoints, and revert that at the end of the scope:
>> 96:     MutexUnlocker mu(_mdo->extra_data_lock(), Mutex::_no_safepoint_check_flag);
> 
> Why is this safe?

@rwestrel I still believe this is safe. But maybe also ugly.

I looked into making the locking more fine-grained, so that we could avoid unlocking the lock temporarily.
The biggest problem is in `ciMethodData::load_remaining_extra_data`. Here we first (iteratively) clean, and then assume that we still hold the lock when we copy it for the `ciMethodData`. Hence, it seems the lock has to be held at this outer scope, but then temporarily unlocked to allow calls to `get_method` in `PrepareExtraDataClosure::finish`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16840#discussion_r1452084643