RFR: 8257140: Crash in JvmtiTagMap::flush_object_free_events() [v2]

[Coleen Phillimore]

Including serviceability-dev.

On 12/1/20 9:03 PM, Coleen Phillimore wrote:
>
>
> On 12/1/20 7:12 PM, Serguei Spitsyn wrote:
>> On Tue, 1 Dec 2020 14:19:15 GMT, Coleen Phillimore 
>> <coleenp at 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()
>> Hi Coleen,
>>
>> The JVMTI environment can be disposed with DisposeEnvironment:
>> https://docs.oracle.com/en/java/javase/15/docs/specs/jvmti.html#DisposeEnvironment
>> It seems to me that this operation is better to wait until posting is 
>> finished. Otherwise, the information is going to be lost in the 
>> report. Of course, we could blame the agent to call the 
>> DisposeEnvironment too early but then the agent needs a way to check 
>> if posting is finished.
>
> When the event is disposed, it calls set_event_callbacks(), which 
> calls flush_object_free_events, which will flush the remaining object 
> free events for this environment.  So the events aren't lost.  The bug 
> was that after that, the JvmtiEnv is marked as DISPOSED but it's still 
> on the list of JvmtiEnvIterator events that we walk until the next 
> safepoint.  When the JvmtiEnv is disposed, it was deleting the tag_map 
> but leaving it on the list, so later calls to flush_object_free_events 
> was getting a bad tag_map pointer in the JvmtiEnv.
>
> Coleen
>
>>
>> Thanks,
>> Serguei
>>
>> -------------
>>
>> PR: https://git.openjdk.java.net/jdk/pull/1539
>