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

[Emanuel Peter]

On Wed, 29 Nov 2023 07:30:48 GMT, Tom Rodriguez <never at openjdk.org> wrote:

>> And there are 2 uses of `query_update_method_data`. One does not use the return `pdata`. The other uses it and in some cases updates it. Do you think it is safe to just re-fetch it, or would that potentially cut some connection between the two that should not be cut?
>> The alternative is just to already get the lock before calling `query_update_method_data`.
>
> I think that anything that can return data from the extra data section is a potential danger.  bci_to_data calls bci_to_extra_data at the end so it seems potentially unsafe which seems like a huge problem since that's used all over the place.  Whether the callers are actually getting or expecting record from extra data is unclear.  I would suspect that most places where it's used there should already be a preallocated record.  The concurrent repacking really makes it hard to ensure the accesses are safe.  I think the API would need to make a stronger split between preallocated records and records which might come from the extra data section.  I'm honestly not sure how to make this truly safe.

Ok. So this issue is much bigger than `query_update_method_data` and `allocate_bci_to_data`, is what you are saying. Sounds like I need to study this much deeper. Maybe we need to refactor the while way we access the records? Maybe any access to the records needs to be guarded with a lock, just to be safe?
If there are concurrent updates, which are guarded by lock, then should not all reads also be guarded? In the end maybe we just need to make all accesses mutually exclusive, right?

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

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