8010151: nsk/regression/b6653214 fails "assert(snapshot != NULL) failed:...

8010151: nsk/regression/b6653214 fails "assert(snapshot != NULL) failed: Worker should not be started"
Summary: Fixed a racing condition when shutting down NMT while worker thread is being started, also fixed a few mis-declared volatile pointers.
Reviewed-by: dholmes, dlong

src/share/vm/runtime/thread.hpp

@@ -1056,11 +1056,11 @@ class JavaThread: public Thread {
 #if INCLUDE_NMT
   // native memory tracking
   inline MemRecorder* get_recorder() const          { return (MemRecorder*)_recorder; }
-  inline void         set_recorder(MemRecorder* rc) { _recorder = (volatile MemRecorder*)rc; }
+  inline void         set_recorder(MemRecorder* rc) { _recorder = rc; }
 
  private:
   // per-thread memory recorder
-  volatile MemRecorder* _recorder;
+  MemRecorder* volatile _recorder;
 #endif // INCLUDE_NMT
 
   // Suspend/resume support for JavaThread

src/share/vm/services/memTrackWorker.cpp

@@ -39,7 +39,7 @@ void GenerationData::reset() {
   }
 }
 
-MemTrackWorker::MemTrackWorker() {
+MemTrackWorker::MemTrackWorker(MemSnapshot* snapshot): _snapshot(snapshot) {
   // create thread uses cgc thread type for now. We should revisit
   // the option, or create new thread type.
   _has_error = !os::create_thread(this, os::cgc_thread);
@@ -88,8 +88,7 @@ void MemTrackWorker::run() {
   assert(MemTracker::is_on(), "native memory tracking is off");
   this->initialize_thread_local_storage();
   this->record_stack_base_and_size();
-  MemSnapshot* snapshot = MemTracker::get_snapshot();
-  assert(snapshot != NULL, "Worker should not be started");
+  assert(_snapshot != NULL, "Worker should not be started");
   MemRecorder* rec;
   unsigned long processing_generation = 0;
   bool          worker_idle = false;
@@ -109,7 +108,7 @@ void MemTrackWorker::run() {
       }
 
       // merge the recorder into staging area
-      if (!snapshot->merge(rec)) {
+      if (!_snapshot->merge(rec)) {
         MemTracker::shutdown(MemTracker::NMT_out_of_memory);
       } else {
         NOT_PRODUCT(_merge_count ++;)
@@ -132,7 +131,7 @@ void MemTrackWorker::run() {
           _head = (_head + 1) % MAX_GENERATIONS;
         }
         // promote this generation data to snapshot
-        if (!snapshot->promote(number_of_classes)) {
+        if (!_snapshot->promote(number_of_classes)) {
           // failed to promote, means out of memory
           MemTracker::shutdown(MemTracker::NMT_out_of_memory);
         }
@@ -140,7 +139,7 @@ void MemTrackWorker::run() {
         // worker thread is idle
         worker_idle = true;
         MemTracker::report_worker_idle();
-        snapshot->wait(1000);
+        _snapshot->wait(1000);
         ThreadCritical tc;
         // check if more data arrived
         if (!_gen[_head].has_more_recorder()) {

src/share/vm/services/memTrackWorker.hpp

@@ -85,8 +85,10 @@ class MemTrackWorker : public NamedThread {
 
   bool            _has_error;
 
+  MemSnapshot*    _snapshot;
+
  public:
-  MemTrackWorker();
+  MemTrackWorker(MemSnapshot* snapshot);
   ~MemTrackWorker();
   _NOINLINE_ void* operator new(size_t size);
   _NOINLINE_ void* operator new(size_t size, const std::nothrow_t& nothrow_constant);

src/share/vm/services/memTracker.cpp

@@ -53,12 +53,12 @@ void SyncThreadRecorderClosure::do_thread(Thread* thread) {
 }
 
 
-MemRecorder*                    MemTracker::_global_recorder = NULL;
+MemRecorder* volatile           MemTracker::_global_recorder = NULL;
 MemSnapshot*                    MemTracker::_snapshot = NULL;
 MemBaseline                     MemTracker::_baseline;
 Mutex*                          MemTracker::_query_lock = NULL;
-volatile MemRecorder*           MemTracker::_merge_pending_queue = NULL;
-volatile MemRecorder*           MemTracker::_pooled_recorders = NULL;
+MemRecorder* volatile           MemTracker::_merge_pending_queue = NULL;
+MemRecorder* volatile           MemTracker::_pooled_recorders = NULL;
 MemTrackWorker*                 MemTracker::_worker_thread = NULL;
 int                             MemTracker::_sync_point_skip_count = 0;
 MemTracker::NMTLevel            MemTracker::_tracking_level = MemTracker::NMT_off;
@@ -128,7 +128,7 @@ void MemTracker::start() {
 
   _snapshot = new (std::nothrow)MemSnapshot();
   if (_snapshot != NULL) {
-    if (!_snapshot->out_of_memory() && start_worker()) {
+    if (!_snapshot->out_of_memory() && start_worker(_snapshot)) {
       _state = NMT_started;
       NMT_track_callsite = (_tracking_level == NMT_detail && can_walk_stack());
       return;
@@ -209,7 +209,7 @@ void MemTracker::final_shutdown() {
 // delete all pooled recorders
 void MemTracker::delete_all_pooled_recorders() {
   // free all pooled recorders
-  volatile MemRecorder* cur_head = _pooled_recorders;
+  MemRecorder* volatile cur_head = _pooled_recorders;
   if (cur_head != NULL) {
     MemRecorder* null_ptr = NULL;
     while (cur_head != NULL && (void*)cur_head != Atomic::cmpxchg_ptr((void*)null_ptr,
@@ -543,14 +543,14 @@ void MemTracker::sync() {
 /*
  * Start worker thread.
  */
-bool MemTracker::start_worker() {
-  assert(_worker_thread == NULL, "Just Check");
-  _worker_thread = new (std::nothrow) MemTrackWorker();
-  if (_worker_thread == NULL || _worker_thread->has_error()) {
-    if (_worker_thread != NULL) {
-      delete _worker_thread;
-      _worker_thread = NULL;
-    }
+bool MemTracker::start_worker(MemSnapshot* snapshot) {
+  assert(_worker_thread == NULL && _snapshot != NULL, "Just Check");
+  _worker_thread = new (std::nothrow) MemTrackWorker(snapshot);
+  if (_worker_thread == NULL) {
+    return false;
+  } else if (_worker_thread->has_error()) {
+    delete _worker_thread;
+    _worker_thread = NULL;
     return false;
   }
   _worker_thread->start();

src/share/vm/services/memTracker.hpp

@@ -421,7 +421,7 @@ class MemTracker : AllStatic {
 
  private:
   // start native memory tracking worker thread
-  static bool start_worker();
+  static bool start_worker(MemSnapshot* snapshot);
 
   // called by worker thread to complete shutdown process
   static void final_shutdown();
@@ -475,18 +475,18 @@ class MemTracker : AllStatic {
   // a thread can start to allocate memory before it is attached
   // to VM 'Thread', those memory activities are recorded here.
   // ThreadCritical is required to guard this global recorder.
-  static MemRecorder*     _global_recorder;
+  static MemRecorder* volatile _global_recorder;
 
   // main thread id
   debug_only(static intx   _main_thread_tid;)
 
   // pending recorders to be merged
-  static volatile MemRecorder*      _merge_pending_queue;
+  static MemRecorder* volatile     _merge_pending_queue;
 
   NOT_PRODUCT(static volatile jint   _pending_recorder_count;)
 
   // pooled memory recorders
-  static volatile MemRecorder*      _pooled_recorders;
+  static MemRecorder* volatile     _pooled_recorders;
 
   // memory recorder pool management, uses following
   // counter to determine if a released memory recorder