On Tue, 1 Dec 2020 14:19:15 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> The JvmtiTagMap can be deleted while it is posting. The JvmtiEnv is still >> on the list but the env is "disposed" of and the tag_map is deleted to save >> memory until the JvmtiEnv can be reclaimed at a safepoint and taken off the >> list. >> >> There's a check JvmtiEnvBase::check_for_periodic_clean_up() that determines >> whether the JvmtiEnv can be taken off the list and that check looks for >> whether threads are in the middle of JvmtiEnvIterator which sets >> Thread::jvmti_env_iteration_count. So this part is safe for the JvmtiTagMap >> changes. >> >> The fix is to clear the JvmtiTagMapTable of the entries inside the table >> lock to save memory but leave JvmtiTagMap intact so that we can load a valid >> pointer when iterating through JvmtiEnvIterator. We could also not do this >> at all and let the JvmtiTagMap destructor delete the table when the JvmtiEnv >> is also deleted, but we don't know what customer situation led to this code >> in the first place. > > Coleen Phillimore has updated the pull request with a new target base due to > a merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Merge branch 'master' into more-jvmti-table > - Rename JvmtiTagMap::clear() and fix JvmtiTagMapTable::clear() to null out > deleted entries. > - 8257140: Crash in JvmtiTagMap::flush_object_free_events() This is reasonable. Thank you for the explanation! The fix looks good to me. > When the event is disposed, it calls set_event_callbacks()... You, probably meant "environment" is disposed, not "event". This correction is for other readers. :) ------------- Marked as reviewed by sspitsyn (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/1539
