On Tue, 12 Jul 2022 17:02:54 GMT, Zhengyu Gu <z...@openjdk.org> wrote:
>> Currently, jdi only check and process class unloading event when it detects >> a new GC cycle. >> >> After [JDK-8212879](https://bugs.openjdk.org/browse/JDK-8212879), posting >> class events can overlap with GC finish event, that results, sometimes, it >> only captures partial or even empty unloaded class list. The pending list >> usually can be flushed out at next GC cycle. But for the classes unloaded >> during the last GC cycle, the class unloading events may lost forever. >> >> This patch checks and processes class unloading events unconditionally, >> suggested by @kbarrett, the last pending unloaded class list can be flushed >> by other events, such as `VM_DEATH`. >> >> It also performs `commonRef_compact()` only when there are classes unloaded. >> >> New test failed about 20% without patch, none with patch. >> >> **Update** >> There are significant changes from early patch. >> >> The new approach: >> No longer removing dead objects and post events on VM thread. I believe it >> was implemented this way to workaround the following issues: >> - JDI event handler uses JVMTI raw monitor, it requires thread in >> `_in_native` state >> - The thread can not hold lock, which is needed to protect `JvmtiTagMap` >> while walking, when transition to `_in_native` state >> >> The new solution breaks up into two steps: >> - Collect all dead object tags with lock >> - Transition to `_in_native` state and post object free events in one batch >> >> This way, JDI event handler can process object free events upon arrivals >> without delay. >> >> **Update 2** >> There is a comment for ` JvmtiTagMap::check_hashmap()` that states >> `ObjectFree` events are posted before heap walks. >> >> // This checks for posting and rehashing before operations that >> // this tagmap table. The calls from a JavaThread only rehash, posting is >> // only done before heap walks. >> void JvmtiTagMap::check_hashmap(bool post_events) { >> >> Now, the events are actually posted after heap walks, but I don't think it >> makes any material material difference. >> Even the events are posted earlier in old code, but they are only processed >> after next GC cycle. > > Zhengyu Gu has updated the pull request incrementally with one additional > commit since the last revision: > > Fix race I haven't had a chance to review your change or to understand the sync issues, but I did test them out and it seems to have gotten rid of the issue. However, there is new assert that turned up once in 75 runs: # Internal Error (/<snip>/workspace/open/src/hotspot/share/runtime/interfaceSupport.inline.hpp:182), pid=3626508, tid=3626546 # assert(!thread->owns_locks()) failed: must release all locks when leaving VM V [libjvm.so+0x138a807] JvmtiJavaThreadEventTransition::JvmtiJavaThreadEventTransition(JavaThread*)+0x157 V [libjvm.so+0x137eee5] JvmtiExport::post_object_free(JvmtiEnv*, GrowableArray<long>*)+0xa5 V [libjvm.so+0x13b571d] JvmtiTagMap::post_dead_objects(GrowableArray<long>*)+0x5d V [libjvm.so+0x13b59dd] JvmtiTagMap::iterate_over_heap(jvmtiHeapObjectFilter, Klass*, jvmtiIterationControl (*)(long, long, long*, void*), void const*)+0x23d V [libjvm.so+0x134d721] JvmtiEnv::IterateOverInstancesOfClass(oop, jvmtiHeapObjectFilter, jvmtiIterationControl (*)(long, long, long*, void*), void const*)+0xd1 V [libjvm.so+0x12dd823] jvmti_IterateOverInstancesOfClass+0x1d3 C [libap04t001.so+0xd1c6] Java_nsk_jvmti_scenarios_allocation_AP04_ap04t001_runIterateOverInstancesOfClass+0xa6 j nsk.jvmti.scenarios.allocation.AP04.ap04t001.runIterateOverInstancesOfClass()V+0 j nsk.jvmti.scenarios.allocation.AP04.ap04t001ClassIterator.runIteration()V+0 j nsk.jvmti.scenarios.allocation.AP04.ap04t001Thread.run()V+30 v ~StubRoutines::call_stub 0x00007f55a0528d47 V [libjvm.so+0x1036784] JavaCalls::call_helper(JavaValue*, methodHandle const&, JavaCallArguments*, JavaThread*)+0x514 V [libjvm.so+0x1037024] JavaCalls::call_virtual(JavaValue*, Klass*, Symbol*, Symbol*, JavaCallArguments*, JavaThread*)+0x4b4 V [libjvm.so+0x1037497] JavaCalls::call_virtual(JavaValue*, Handle, Klass*, Symbol*, Symbol*, JavaThread*)+0x77 V [libjvm.so+0x11c731b] thread_entry(JavaThread*, JavaThread*)+0x12b V [libjvm.so+0x106d948] JavaThread::thread_main_inner()+0x238 V [libjvm.so+0x1a8cbd0] Thread::call_run()+0x100 V [libjvm.so+0x174fea4] thread_native_entry(Thread*)+0x104 ------------- PR: https://git.openjdk.org/jdk/pull/9168
