RFR: 8277486: NMT: Cleanup ThreadStackTracker

[Zhengyu Gu]

On Wed, 24 Nov 2021 08:47:35 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> There are several issues with `ThreadStackTracker`.
>> 
>> 1) Following assertion:
>>   `assert(MemTracker::tracking_level() >= NMT_summary, "Must be");`
>> in `ThreadStackTracker::record/release_thread_stack()` is unreliable. The full fence after downgrading tracking level is *not* sufficient to avoid the racy.
>> 
>> 2) NMT tracking level is downgraded without `ThreadCritical` lock held. But, it does require `ThreadCritical` lock to be held when it actually downgrade internal tracking data structure, so checking internal state is reliable to determine current tracking state.
>> Add assertion to ensure correct tracking state
>> 
>> 3) `_thread_counter` is updated with `ThreadCritical` lock, but is read without the lock. Change to atomic update to ensure reader will not read stale value.
>> 
>> 4) NMT events are relative rare. So far, I have yet seen (1) assertion failure but add new test may help to spot such problem.
>
> src/hotspot/share/services/virtualMemoryTracker.cpp line 673:
> 
>> 671:     return true;
>> 672:   } else {
>> 673:     assert(MemTracker::tracking_level() < NMT_summary, "Must be");
> 
> Is this safe? We don't guard downgrading the tracking level with ThreadCritical.

I think it is safe. There is a full fence before downgrading transition. Therefore, it has a inter-thread happen-before relationship. So, if a thread sees transition (e.g. reset internal structure) with `ThreadCritical` lock held, it should see the downgraded tracking level.

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

PR: https://git.openjdk.java.net/jdk/pull/6504