> In the rewrites made for: > [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) `VM_ThreadDump > asserts in interleaved ObjectMonitor::deflate_monitor calls` > > I removed the filtering of *owned ObjectMonitors with dead objects*. The > reasoning was that you should never have an owned ObjectMonitor with a dead > object. I added an assert to check this assumption. It turns out that the > assumption was wrong *if* you use JNI to call MonitorEnter and then remove > all references to the locked object. > > The provided tests provoke this assert form: > * the JNI thread detach code > * thread dumping with locked monitors, and > * the JVMTI GetOwnedMonitorInfo API. > > While investigating this we've found that the thread detach code becomes more > correct when this filter was removed. Previously, the locked monitors never > got unlocked because the ObjectMonitor iterator never exposed these monitors > to the JNI detach code that unlocks the thread's monitors. That bug caused an > ObjectMonitor leak. So, for this case I'm leaving these ObjectMonitors > unfiltered so that we don't reintroduce the leak. > > The thread dumping case doesn't tolerate ObjectMonitor with dead objects, so > I'm filtering those in the closure that collects ObjectMonitor. Side note: We > have discussions about ways to completely rewrite this by letting each thread > have thread-local information about JNI held locks. If we have this we could > probably throw away the entire ObjectMonitorDump hashtable, and its walk of > the `_in_use_list.`. > > For GetOwnedMonitorInfo it is unclear if we should expose these weird > ObjectMonitor. If we do, then the users can detect that a thread holds a lock > with a dead object, and the code will return NULL as one of the "owned > monitors" returned. I don't think that's a good idea, so I'm filtering out > these ObjectMonitor for those calls. > > Test: the written tests with and without the fix. Tier1-Tier3, so far.
Stefan Karlsson has updated the pull request incrementally with two additional commits since the last revision: - Tweaked comment in test - Rewrite tests ------------- Changes: - all: https://git.openjdk.org/jdk/pull/16783/files - new: https://git.openjdk.org/jdk/pull/16783/files/b1dd4cf8..4b0976a8 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16783&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16783&range=00-01 Stats: 605 lines in 9 files changed: 388 ins; 214 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/16783.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16783/head:pull/16783 PR: https://git.openjdk.org/jdk/pull/16783
