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

Emanuel Peter epeter at openjdk.org
Wed Jan 3 09:11:54 UTC 2024


> As explained in a [comment below](https://github.com/openjdk/jdk/pull/16840#issuecomment-1833529561), we have to ensure that reading/writing/cleaning the extra data all needs to be guarded by the `extra_data_lock`, and that no safepoint should happen while holding that lock, so that the lock is not broken.
> 
> I introduced `check_extra_data_locked`, where I check that we hold the lock, and if we are a java thread (only those ever safepoint), that we currently are in a `NoSafepointVerifier` scope, hence we verify that no safepoint will be taken.
> 
> I placed `check_extra_data_locked` in all the places where we access the extra data, and then placed locks and no-safepoint-verifiers at the call-site of those places.
> 
> I also needed to change the rank of `extra_data_lock` to `nosafepoint` and set the `Mutex::_no_safepoint_check_flag` when taking the lock. Otherwise I could not take the lock from a VM thread.
> 
> **Complications with ttyl**
> There were a few places in printing code, where did `ttyLocker ttyl;`, and then in that scope we would access the extra data. Now that I introduced locking with `extra_data_lock`, this ran into asserts which check the lock ranks: `ttyl` has a very low rank, and `extra_data_lock` a rather high lock. Hence, we cannot lock `extra_data_lock` inside a `ttyl` scope.
> 
> If we were to simply remove the `ttyl` locking, then the many print lines inside that scope might be interrupted and another thread can insert other printing in between. To avoid that, I now first buffer all lines in a `stringStream`, and then print that buffered stream to `tty` all at once, which means no other printing can be injected in between.
> 
> **Testing**
> Testing: tier1-3 and stress.

Emanuel Peter has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 12 commits:

 - Merge branch 'master' into JDK-8306767
 - removed some ttyl cases, which collided with the extra_data_lock
 - remove more locking
 - fix conflicts with tty lock
 - move a lock to earlier, to have order right with tty lock
 - missed a case where I need to lock
 - make lock not safepointing
 - manual merge with master after JDK-8267532
 - more locking, still fails tho - WIP
 - adding more verification and more locking, WIP
 - ... and 2 more: https://git.openjdk.org/jdk/compare/cbe329b9...2bda9d7c

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

Changes: https://git.openjdk.org/jdk/pull/16840/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16840&range=10
  Stats: 222 lines in 22 files changed: 141 ins; 14 del; 67 mod
  Patch: https://git.openjdk.org/jdk/pull/16840.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16840/head:pull/16840

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


More information about the hotspot-dev mailing list