RFR: 8277486: NMT: Cleanup ThreadStackTracker

[Zhengyu Gu]

On Wed, 24 Nov 2021 08:16:51 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/memTracker.hpp line 270:
> 
>> 268:     if (tracking_level() < NMT_summary) return;
>> 269:     if (addr != NULL) {
>> 270:       ThreadCritical tc;
> 
> Would this not require, to prevent the assert in (new|delete)_thread_stack, to repeat the tracking level check inside the ThreadCritical scope? Since the tracking level could have changed between lines 268 and 270?

With/without ThreadCritical lock, the assertion in (new|delete)_thread_stack is unreliable, because tracking level is undergraded without ThreadCritical lock, therefore, there is no synchronize-with relationship. So to check tracking level inside (new|delete)_thread_stack, it can only rely on 

Early return at line #268 avoids very expensive ThreadCritcal lock, given NMT is off by default and also tracking level can only be downgraded monotonically.

Anyway, I believe current shutdown logic for virtual memory tracking is racy, filed JDK-8277788.

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

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