> This is a redo of [JDK-8304824](https://bugs.openjdk.org/browse/JDK-8304824) > which was backed out by > [JDK-8343726](https://bugs.openjdk.org/browse/JDK-8343726) due to problems > documented in [JDK-8343244](https://bugs.openjdk.org/browse/JDK-8343244). > > The problem was that `NmtVirtualMemoryLocker` was not locking when the > current thread is not attached by checking `Thread::current_or_null_safe() != > nullptr`. This is necessary during VM init, but should not be allowed > afterward. NMT may be used in `attach_current_thread` before the current > thread is set. The lock was not being acquired in that case, which > intermittently caused NMT accounting to become corrupted, triggering various > assertions when future NMT operations are done. To fix this, I've adopted > [Thomas' > suggestion](https://github.com/openjdk/jdk/pull/21928#issuecomment-2460238057) > to reverse the order of > > > thread->register_thread_stack_with_NMT(); > thread->initialize_thread_current(); > > > in `attach_current_thread`. This allows `NmtVirtualMemoryLocker` to be > locked after current thread is set. > > To allow for `NmtVirtualMemoryLocker` to be used during VM init, I've > replaced the `ConditionalMutexLocker` check `Thread::current_or_null_safe() > != nullptr` with a new flag: `_done_bootstrap`. This flag prevents the lock > from being used during VM init, at which point we are single threaded anyway. > This avoids errors due to Hotspot mutexes and current thread not yet being > ready. > > I also added new asserts in `virtualMemoryTracker.cpp` to catch future bugs > like this where the lock is not held when it should be. I updated the > appropriate VMT tests to also lock (there were a few cases where locking was > being bypassed) so they can pass the new asserts. > > I also removed the unused `_query_lock` variable in `MemTracker`. > > Testing: > > - On Linux amd64, I was able to consistently reproduce the errors described > in [JDK-8343244](https://bugs.openjdk.org/browse/JDK-8343244) by increasing > the number of test threads in > `java/lang/Thread/jni/AttachCurrentThread/AttachTest.java`. The test > consistently passes with the new changes in this PR. > - hotspot_nmt , gtest:VirtualSpace, tier1
Robert Toyonaga has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits: - Merge master - move MemTracker::set_done_bootstrap() - Update src/hotspot/share/utilities/vmError.cpp Co-authored-by: David Holmes <62092539+dholmes-...@users.noreply.github.com> - updates tests and remove old class - use ConditionalMutexLocker directly - Fix concurrency issue. Add assertions. Update tests. - Revert "8343726: [BACKOUT] NMT should not use ThreadCritical" This reverts commit c3df050b88ecef123199a4e96f6d9884d064ae45. ------------- Changes: https://git.openjdk.org/jdk/pull/22745/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=22745&range=04 Stats: 93 lines in 19 files changed: 45 ins; 24 del; 24 mod Patch: https://git.openjdk.org/jdk/pull/22745.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/22745/head:pull/22745 PR: https://git.openjdk.org/jdk/pull/22745
