8013651: NMT: reserve/release sequence id's in incorrect order due to race

Summary: Fixed NMT race condition for realloc, uncommit and release
Reviewed-by: coleenp, ccheung

src/os/bsd/vm/os_bsd.cpp

@@ -2354,21 +2354,20 @@ char* os::reserve_memory_special(size_t bytes, char* req_addr, bool exec) {
   }
 
   // The memory is committed
-  address pc = CALLER_PC;
-  MemTracker::record_virtual_memory_reserve((address)addr, bytes, pc);
-  MemTracker::record_virtual_memory_commit((address)addr, bytes, pc);
+  MemTracker::record_virtual_memory_reserve_and_commit((address)addr, bytes, mtNone, CALLER_PC);
 
   return addr;
 }
 
 bool os::release_memory_special(char* base, size_t bytes) {
+  MemTracker::Tracker tkr = MemTracker::get_virtual_memory_release_tracker();
   // detaching the SHM segment will also delete it, see reserve_memory_special()
   int rslt = shmdt(base);
   if (rslt == 0) {
-    MemTracker::record_virtual_memory_uncommit((address)base, bytes);
-    MemTracker::record_virtual_memory_release((address)base, bytes);
+    tkr.record((address)base, bytes);
     return true;
   } else {
+    tkr.discard();
     return false;
   }
 

src/os/bsd/vm/perfMemory_bsd.cpp

@@ -753,8 +753,7 @@ static char* mmap_create_shared(size_t size) {
   (void)::memset((void*) mapAddress, 0, size);
 
   // it does not go through os api, the operation has to record from here
-  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, CURRENT_PC);
-  MemTracker::record_virtual_memory_type((address)mapAddress, mtInternal);
+  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, mtInternal, CURRENT_PC);
 
   return mapAddress;
 }
@@ -919,8 +918,7 @@ static void mmap_attach_shared(const char* user, int vmid, PerfMemory::PerfMemor
   }
 
   // it does not go through os api, the operation has to record from here
-  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, CURRENT_PC);
-  MemTracker::record_virtual_memory_type((address)mapAddress, mtInternal);
+  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, mtInternal, CURRENT_PC);
 
   *addr = mapAddress;
   *sizep = size;

src/os/linux/vm/os_linux.cpp

@@ -3363,22 +3363,21 @@ char* os::reserve_memory_special(size_t bytes, char* req_addr, bool exec) {
   }
 
   // The memory is committed
-  address pc = CALLER_PC;
-  MemTracker::record_virtual_memory_reserve((address)addr, bytes, pc);
-  MemTracker::record_virtual_memory_commit((address)addr, bytes, pc);
+  MemTracker::record_virtual_memory_reserve_and_commit((address)addr, bytes, mtNone, CALLER_PC);
 
   return addr;
 }
 
 bool os::release_memory_special(char* base, size_t bytes) {
+  MemTracker::Tracker tkr = MemTracker::get_virtual_memory_release_tracker();
   // detaching the SHM segment will also delete it, see reserve_memory_special()
   int rslt = shmdt(base);
   if (rslt == 0) {
-    MemTracker::record_virtual_memory_uncommit((address)base, bytes);
-    MemTracker::record_virtual_memory_release((address)base, bytes);
+    tkr.record((address)base, bytes);
     return true;
   } else {
-   return false;
+    tkr.discard();
+    return false;
   }
 }
 

src/os/linux/vm/perfMemory_linux.cpp

@@ -753,8 +753,7 @@ static char* mmap_create_shared(size_t size) {
   (void)::memset((void*) mapAddress, 0, size);
 
   // it does not go through os api, the operation has to record from here
-  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, CURRENT_PC);
-  MemTracker::record_virtual_memory_type((address)mapAddress, mtInternal);
+  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, mtInternal, CURRENT_PC);
 
   return mapAddress;
 }
@@ -917,8 +916,7 @@ static void mmap_attach_shared(const char* user, int vmid, PerfMemory::PerfMemor
   }
 
   // it does not go through os api, the operation has to record from here
-  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, CURRENT_PC);
-  MemTracker::record_virtual_memory_type((address)mapAddress, mtInternal);
+  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, mtInternal, CURRENT_PC);
 
   *addr = mapAddress;
   *sizep = size;

src/os/solaris/vm/os_solaris.cpp

@@ -3532,22 +3532,21 @@ char* os::reserve_memory_special(size_t size, char* addr, bool exec) {
   }
 
   // The memory is committed
-  address pc = CALLER_PC;
-  MemTracker::record_virtual_memory_reserve((address)retAddr, size, pc);
-  MemTracker::record_virtual_memory_commit((address)retAddr, size, pc);
+  MemTracker::record_virtual_memory_reserve_and_commit((address)retAddr, size, mtNone, CURRENT_PC);
 
   return retAddr;
 }
 
 bool os::release_memory_special(char* base, size_t bytes) {
+  MemTracker::Tracker tkr = MemTracker::get_virtual_memory_release_tracker();
   // detaching the SHM segment will also delete it, see reserve_memory_special()
   int rslt = shmdt(base);
   if (rslt == 0) {
-    MemTracker::record_virtual_memory_uncommit((address)base, bytes);
-    MemTracker::record_virtual_memory_release((address)base, bytes);
+    tkr.record((address)base, bytes);
     return true;
   } else {
-   return false;
+    tkr.discard();
+    return false;
   }
 }
 

src/os/solaris/vm/perfMemory_solaris.cpp

@@ -768,8 +768,7 @@ static char* mmap_create_shared(size_t size) {
   (void)::memset((void*) mapAddress, 0, size);
 
   // it does not go through os api, the operation has to record from here
-  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, CURRENT_PC);
-  MemTracker::record_virtual_memory_type((address)mapAddress, mtInternal);
+  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, mtInternal, CURRENT_PC);
 
   return mapAddress;
 }
@@ -932,8 +931,7 @@ static void mmap_attach_shared(const char* user, int vmid, PerfMemory::PerfMemor
   }
 
   // it does not go through os api, the operation has to record from here
-  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, CURRENT_PC);
-  MemTracker::record_virtual_memory_type((address)mapAddress, mtInternal);
+  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, mtInternal, CURRENT_PC);
 
   *addr = mapAddress;
   *sizep = size;

src/os/windows/vm/os_windows.cpp

@@ -2875,7 +2875,7 @@ static char* allocate_pages_individually(size_t bytes, char* addr, DWORD flags,
                                 PAGE_READWRITE);
   // If reservation failed, return NULL
   if (p_buf == NULL) return NULL;
-  MemTracker::record_virtual_memory_reserve((address)p_buf, size_of_reserve, CALLER_PC);
+  MemTracker::record_virtual_memory_reserve((address)p_buf, size_of_reserve, mtNone, CALLER_PC);
   os::release_memory(p_buf, bytes + chunk_size);
 
   // we still need to round up to a page boundary (in case we are using large pages)
@@ -2941,7 +2941,7 @@ static char* allocate_pages_individually(size_t bytes, char* addr, DWORD flags,
         // need to create a dummy 'reserve' record to match
         // the release.
         MemTracker::record_virtual_memory_reserve((address)p_buf,
-          bytes_to_release, CALLER_PC);
+          bytes_to_release, mtNone, CALLER_PC);
         os::release_memory(p_buf, bytes_to_release);
       }
 #ifdef ASSERT
@@ -2961,9 +2961,10 @@ static char* allocate_pages_individually(size_t bytes, char* addr, DWORD flags,
   // Although the memory is allocated individually, it is returned as one.
   // NMT records it as one block.
   address pc = CALLER_PC;
-  MemTracker::record_virtual_memory_reserve((address)p_buf, bytes, pc);
   if ((flags & MEM_COMMIT) != 0) {
-    MemTracker::record_virtual_memory_commit((address)p_buf, bytes, pc);
+    MemTracker::record_virtual_memory_reserve_and_commit((address)p_buf, bytes, mtNone, pc);
+  } else {
+    MemTracker::record_virtual_memory_reserve((address)p_buf, bytes, mtNone, pc);
   }
 
   // made it this far, success
@@ -3154,8 +3155,7 @@ char* os::reserve_memory_special(size_t bytes, char* addr, bool exec) {
     char * res = (char *)VirtualAlloc(NULL, bytes, flag, prot);
     if (res != NULL) {
       address pc = CALLER_PC;
-      MemTracker::record_virtual_memory_reserve((address)res, bytes, pc);
-      MemTracker::record_virtual_memory_commit((address)res, bytes, pc);
+      MemTracker::record_virtual_memory_reserve_and_commit((address)res, bytes, mtNone, pc);
     }
 
     return res;
@@ -3164,8 +3164,6 @@ char* os::reserve_memory_special(size_t bytes, char* addr, bool exec) {
 
 bool os::release_memory_special(char* base, size_t bytes) {
   assert(base != NULL, "Sanity check");
-  // Memory allocated via reserve_memory_special() is committed
-  MemTracker::record_virtual_memory_uncommit((address)base, bytes);
   return release_memory(base, bytes);
 }
 

src/os/windows/vm/perfMemory_windows.cpp

@@ -1498,8 +1498,7 @@ static char* mapping_create_shared(size_t size) {
   (void)memset(mapAddress, '\0', size);
 
   // it does not go through os api, the operation has to record from here
-  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, CURRENT_PC);
-  MemTracker::record_virtual_memory_type((address)mapAddress, mtInternal);
+  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, mtInternal, CURRENT_PC);
 
   return (char*) mapAddress;
 }
@@ -1681,8 +1680,7 @@ static void open_file_mapping(const char* user, int vmid,
   }
 
   // it does not go through os api, the operation has to record from here
-  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, CURRENT_PC);
-  MemTracker::record_virtual_memory_type((address)mapAddress, mtInternal);
+  MemTracker::record_virtual_memory_reserve((address)mapAddress, size, mtInternal, CURRENT_PC);
 
 
   *addrp = (char*)mapAddress;
@@ -1836,9 +1834,10 @@ void PerfMemory::detach(char* addr, size_t bytes, TRAPS) {
     return;
   }
 
+  MemTracker::Tracker tkr = MemTracker::get_virtual_memory_release_tracker();
   remove_file_mapping(addr);
   // it does not go through os api, the operation has to record from here
-  MemTracker::record_virtual_memory_release((address)addr, bytes);
+  tkr.record((address)addr, bytes);
 }
 
 char* PerfMemory::backing_store_filename() {

src/share/vm/runtime/os.cpp

@@ -647,10 +647,13 @@ void* os::realloc(void *memblock, size_t size, MEMFLAGS memflags, address caller
 #ifndef ASSERT
   NOT_PRODUCT(inc_stat_counter(&num_mallocs, 1));
   NOT_PRODUCT(inc_stat_counter(&alloc_bytes, size));
+  MemTracker::Tracker tkr = MemTracker::get_realloc_tracker();
   void* ptr = ::realloc(memblock, size);
   if (ptr != NULL) {
-    MemTracker::record_realloc((address)memblock, (address)ptr, size, memflags,
+    tkr.record((address)memblock, (address)ptr, size, memflags,
      caller == 0 ? CALLER_PC : caller);
+  } else {
+    tkr.discard();
   }
   return ptr;
 #else
@@ -1456,7 +1459,7 @@ bool os::create_stack_guard_pages(char* addr, size_t bytes) {
 char* os::reserve_memory(size_t bytes, char* addr, size_t alignment_hint) {
   char* result = pd_reserve_memory(bytes, addr, alignment_hint);
   if (result != NULL) {
-    MemTracker::record_virtual_memory_reserve((address)result, bytes, CALLER_PC);
+    MemTracker::record_virtual_memory_reserve((address)result, bytes, mtNone, CALLER_PC);
   }
 
   return result;
@@ -1466,7 +1469,7 @@ char* os::reserve_memory(size_t bytes, char* addr, size_t alignment_hint,
    MEMFLAGS flags) {
   char* result = pd_reserve_memory(bytes, addr, alignment_hint);
   if (result != NULL) {
-    MemTracker::record_virtual_memory_reserve((address)result, bytes, CALLER_PC);
+    MemTracker::record_virtual_memory_reserve((address)result, bytes, mtNone, CALLER_PC);
     MemTracker::record_virtual_memory_type((address)result, flags);
   }
 
@@ -1476,7 +1479,7 @@ char* os::reserve_memory(size_t bytes, char* addr, size_t alignment_hint,
 char* os::attempt_reserve_memory_at(size_t bytes, char* addr) {
   char* result = pd_attempt_reserve_memory_at(bytes, addr);
   if (result != NULL) {
-    MemTracker::record_virtual_memory_reserve((address)result, bytes, CALLER_PC);
+    MemTracker::record_virtual_memory_reserve((address)result, bytes, mtNone, CALLER_PC);
   }
   return result;
 }
@@ -1516,17 +1519,23 @@ void os::commit_memory_or_exit(char* addr, size_t size, size_t alignment_hint,
 }
 
 bool os::uncommit_memory(char* addr, size_t bytes) {
+  MemTracker::Tracker tkr = MemTracker::get_virtual_memory_uncommit_tracker();
   bool res = pd_uncommit_memory(addr, bytes);
   if (res) {
-    MemTracker::record_virtual_memory_uncommit((address)addr, bytes);
+    tkr.record((address)addr, bytes);
+  } else {
+    tkr.discard();
   }
   return res;
 }
 
 bool os::release_memory(char* addr, size_t bytes) {
+  MemTracker::Tracker tkr = MemTracker::get_virtual_memory_release_tracker();
   bool res = pd_release_memory(addr, bytes);
   if (res) {
-    MemTracker::record_virtual_memory_release((address)addr, bytes);
+    tkr.record((address)addr, bytes);
+  } else {
+    tkr.discard();
   }
   return res;
 }
@@ -1537,8 +1546,7 @@ char* os::map_memory(int fd, const char* file_name, size_t file_offset,
                            bool allow_exec) {
   char* result = pd_map_memory(fd, file_name, file_offset, addr, bytes, read_only, allow_exec);
   if (result != NULL) {
-    MemTracker::record_virtual_memory_reserve((address)result, bytes, CALLER_PC);
-    MemTracker::record_virtual_memory_commit((address)result, bytes, CALLER_PC);
+    MemTracker::record_virtual_memory_reserve_and_commit((address)result, bytes, mtNone, CALLER_PC);
   }
   return result;
 }
@@ -1551,10 +1559,12 @@ char* os::remap_memory(int fd, const char* file_name, size_t file_offset,
 }
 
 bool os::unmap_memory(char *addr, size_t bytes) {
+  MemTracker::Tracker tkr = MemTracker::get_virtual_memory_release_tracker();
   bool result = pd_unmap_memory(addr, bytes);
   if (result) {
-    MemTracker::record_virtual_memory_uncommit((address)addr, bytes);
-    MemTracker::record_virtual_memory_release((address)addr, bytes);
+    tkr.record((address)addr, bytes);
+  } else {
+    tkr.discard();
   }
   return result;
 }

src/share/vm/services/memPtr.hpp

 /*
- * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -457,9 +457,8 @@ class SeqMemPointerRecord : public MemPointerRecord {
  public:
   SeqMemPointerRecord(): _seq(0){ }
 
-  SeqMemPointerRecord(address addr, MEMFLAGS flags, size_t size)
-    : MemPointerRecord(addr, flags, size) {
-    _seq = SequenceGenerator::next();
+  SeqMemPointerRecord(address addr, MEMFLAGS flags, size_t size, jint seq)
+    : MemPointerRecord(addr, flags, size), _seq(seq)  {
   }
 
   SeqMemPointerRecord(const SeqMemPointerRecord& copy_from)
@@ -488,8 +487,8 @@ class SeqMemPointerRecordEx : public MemPointerRecordEx {
   SeqMemPointerRecordEx(): _seq(0) { }
 
   SeqMemPointerRecordEx(address addr, MEMFLAGS flags, size_t size,
-    address pc): MemPointerRecordEx(addr, flags, size, pc) {
-    _seq = SequenceGenerator::next();
+    jint seq, address pc):
+    MemPointerRecordEx(addr, flags, size, pc), _seq(seq)  {
   }
 
   SeqMemPointerRecordEx(const SeqMemPointerRecordEx& copy_from)

src/share/vm/services/memRecorder.cpp

 /*
- * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -69,10 +69,11 @@ MemRecorder::MemRecorder() {
 
   if (_pointer_records != NULL) {
     // recode itself
+    address pc = CURRENT_PC;
     record((address)this, (MemPointerRecord::malloc_tag()|mtNMT|otNMTRecorder),
-        sizeof(MemRecorder), CALLER_PC);
+        sizeof(MemRecorder), SequenceGenerator::next(), pc);
     record((address)_pointer_records, (MemPointerRecord::malloc_tag()|mtNMT|otNMTRecorder),
-        _pointer_records->instance_size(),CURRENT_PC);
+        _pointer_records->instance_size(), SequenceGenerator::next(), pc);
   }
 }
 
@@ -116,7 +117,8 @@ int MemRecorder::sort_record_fn(const void* e1, const void* e2) {
   }
 }
 
-bool MemRecorder::record(address p, MEMFLAGS flags, size_t size, address pc) {
+bool MemRecorder::record(address p, MEMFLAGS flags, size_t size, jint seq, address pc) {
+  assert(seq > 0, "No sequence number");
 #ifdef ASSERT
   if (MemPointerRecord::is_virtual_memory_record(flags)) {
     assert((flags & MemPointerRecord::tag_masks) != 0, "bad virtual memory record");
@@ -133,11 +135,11 @@ bool MemRecorder::record(address p, MEMFLAGS flags, size_t size, address pc) {
 #endif
 
   if (MemTracker::track_callsite()) {
-    SeqMemPointerRecordEx ap(p, flags, size, pc);
+    SeqMemPointerRecordEx ap(p, flags, size, seq, pc);
     debug_only(check_dup_seq(ap.seq());)
     return _pointer_records->append(&ap);
   } else {
-    SeqMemPointerRecord ap(p, flags, size);
+    SeqMemPointerRecord ap(p, flags, size, seq);
     debug_only(check_dup_seq(ap.seq());)
     return _pointer_records->append(&ap);
   }

src/share/vm/services/memRecorder.hpp

 /*
- * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -220,7 +220,7 @@ class MemRecorder : public CHeapObj<mtNMT|otNMTRecorder> {
   ~MemRecorder();
 
   // record a memory operation
-  bool record(address addr, MEMFLAGS flags, size_t size, address caller_pc = 0);
+  bool record(address addr, MEMFLAGS flags, size_t size, jint seq, address caller_pc = 0);
 
   // linked list support
   inline void set_next(MemRecorder* rec) {

src/share/vm/services/memTracker.cpp

@@ -69,6 +69,7 @@ int                             MemTracker::_thread_count = 255;
 volatile jint                   MemTracker::_pooled_recorder_count = 0;
 volatile unsigned long          MemTracker::_processing_generation = 0;
 volatile bool                   MemTracker::_worker_thread_idle = false;
+volatile jint                   MemTracker::_pending_op_count = 0;
 volatile bool                   MemTracker::_slowdown_calling_thread = false;
 debug_only(intx                 MemTracker::_main_thread_tid = 0;)
 NOT_PRODUCT(volatile jint       MemTracker::_pending_recorder_count = 0;)
@@ -337,92 +338,14 @@ void MemTracker::release_thread_recorder(MemRecorder* rec) {
   Atomic::inc(&_pooled_recorder_count);
 }
 
-/*
- * This is the most important method in whole nmt implementation.
- *
- * Create a memory record.
- * 1. When nmt is in single-threaded bootstrapping mode, no lock is needed as VM
- *    still in single thread mode.
- * 2. For all threads other than JavaThread, ThreadCritical is needed
- *    to write to recorders to global recorder.
- * 3. For JavaThreads that are not longer visible by safepoint, also
- *    need to take ThreadCritical and records are written to global
- *    recorders, since these threads are NOT walked by Threads.do_thread().
- * 4. JavaThreads that are running in native state, have to transition
- *    to VM state before writing to per-thread recorders.
- * 5. JavaThreads that are running in VM state do not need any lock and
- *    records are written to per-thread recorders.
- * 6. For a thread has yet to attach VM 'Thread', they need to take
- *    ThreadCritical to write to global recorder.
- *
- *    Important note:
- *    NO LOCK should be taken inside ThreadCritical lock !!!
- */
-void MemTracker::create_memory_record(address addr, MEMFLAGS flags,
-    size_t size, address pc, Thread* thread) {
-  assert(addr != NULL, "Sanity check");
-  if (!shutdown_in_progress()) {
-    // single thread, we just write records direct to global recorder,'
-    // with any lock
-    if (_state == NMT_bootstrapping_single_thread) {
-      assert(_main_thread_tid == os::current_thread_id(), "wrong thread");
-      thread = NULL;
-    } else {
-      if (thread == NULL) {
-          // don't use Thread::current(), since it is possible that
-          // the calling thread has yet to attach to VM 'Thread',
-          // which will result assertion failure
-          thread = ThreadLocalStorage::thread();
-      }
-    }
-
-    if (thread != NULL) {
-      // slow down all calling threads except NMT worker thread, so it
-      // can catch up.
-      if (_slowdown_calling_thread && thread != _worker_thread) {
-        os::yield_all();
-      }
-
-      if (thread->is_Java_thread() && ((JavaThread*)thread)->is_safepoint_visible()) {
-        JavaThread*      java_thread = (JavaThread*)thread;
-        JavaThreadState  state = java_thread->thread_state();
-        if (SafepointSynchronize::safepoint_safe(java_thread, state)) {
-          // JavaThreads that are safepoint safe, can run through safepoint,
-          // so ThreadCritical is needed to ensure no threads at safepoint create
-          // new records while the records are being gathered and the sequence number is changing
-          ThreadCritical tc;
-          create_record_in_recorder(addr, flags, size, pc, java_thread);
-        } else {
-          create_record_in_recorder(addr, flags, size, pc, java_thread);
-        }
-      } else {
-        // other threads, such as worker and watcher threads, etc. need to
-        // take ThreadCritical to write to global recorder
-        ThreadCritical tc;
-        create_record_in_recorder(addr, flags, size, pc, NULL);
-      }
-    } else {
-      if (_state == NMT_bootstrapping_single_thread) {
-        // single thread, no lock needed
-        create_record_in_recorder(addr, flags, size, pc, NULL);
-      } else {
-        // for thread has yet to attach VM 'Thread', we can not use VM mutex.
-        // use native thread critical instead
-        ThreadCritical tc;
-        create_record_in_recorder(addr, flags, size, pc, NULL);
-      }
-    }
-  }
-}
-
 // write a record to proper recorder. No lock can be taken from this method
 // down.
-void MemTracker::create_record_in_recorder(address addr, MEMFLAGS flags,
-    size_t size, address pc, JavaThread* thread) {
+void MemTracker::write_tracking_record(address addr, MEMFLAGS flags,
+    size_t size, jint seq, address pc, JavaThread* thread) {
 
     MemRecorder* rc = get_thread_recorder(thread);
     if (rc != NULL) {
-      rc->record(addr, flags, size, pc);
+      rc->record(addr, flags, size, seq, pc);
     }
 }
 
@@ -487,39 +410,43 @@ void MemTracker::sync() {
         return;
       }
     }
-    _sync_point_skip_count = 0;
     {
       // This method is running at safepoint, with ThreadCritical lock,
       // it should guarantee that NMT is fully sync-ed.
       ThreadCritical tc;
 
-      SequenceGenerator::reset();
+      // We can NOT execute NMT sync-point if there are pending tracking ops.
+      if (_pending_op_count == 0) {
+        SequenceGenerator::reset();
+        _sync_point_skip_count = 0;
 
-      // walk all JavaThreads to collect recorders
-      SyncThreadRecorderClosure stc;
-      Threads::threads_do(&stc);
+        // walk all JavaThreads to collect recorders
+        SyncThreadRecorderClosure stc;
+        Threads::threads_do(&stc);
 
-      _thread_count = stc.get_thread_count();
-      MemRecorder* pending_recorders = get_pending_recorders();
+        _thread_count = stc.get_thread_count();
+        MemRecorder* pending_recorders = get_pending_recorders();
 
-      if (_global_recorder != NULL) {
-        _global_recorder->set_next(pending_recorders);
-        pending_recorders = _global_recorder;
-        _global_recorder = NULL;
-      }
+        if (_global_recorder != NULL) {
+          _global_recorder->set_next(pending_recorders);
+          pending_recorders = _global_recorder;
+          _global_recorder = NULL;
+        }
 
-      // see if NMT has too many outstanding recorder instances, it usually
-      // means that worker thread is lagging behind in processing them.
-      if (!AutoShutdownNMT) {
-        _slowdown_calling_thread = (MemRecorder::_instance_count > MAX_RECORDER_THREAD_RATIO * _thread_count);
-      }
+        // see if NMT has too many outstanding recorder instances, it usually
+        // means that worker thread is lagging behind in processing them.
+        if (!AutoShutdownNMT) {
+          _slowdown_calling_thread = (MemRecorder::_instance_count > MAX_RECORDER_THREAD_RATIO * _thread_count);
+        }
 
-      // check _worker_thread with lock to avoid racing condition
-      if (_worker_thread != NULL) {
-        _worker_thread->at_sync_point(pending_recorders, InstanceKlass::number_of_instance_classes());
+        // check _worker_thread with lock to avoid racing condition
+        if (_worker_thread != NULL) {
+          _worker_thread->at_sync_point(pending_recorders, InstanceKlass::number_of_instance_classes());
+        }
+        assert(SequenceGenerator::peek() == 1, "Should not have memory activities during sync-point");
+      } else {
+        _sync_point_skip_count ++;
       }
-
-      assert(SequenceGenerator::peek() == 1, "Should not have memory activities during sync-point");
     }
   }
 
@@ -708,3 +635,243 @@ void MemTracker::print_tracker_stats(outputStream* st) {
 }
 #endif
 
+
+// Tracker Implementation
+
+/*
+ * Create a tracker.
+ * This is a fairly complicated constructor, as it has to make two important decisions:
+ *   1) Does it need to take ThreadCritical lock to write tracking record
+ *   2) Does it need to pre-reserve a sequence number for the tracking record
+ *
+ * The rules to determine if ThreadCritical is needed:
+ *   1. When nmt is in single-threaded bootstrapping mode, no lock is needed as VM
+ *      still in single thread mode.
+ *   2. For all threads other than JavaThread, ThreadCritical is needed
+ *      to write to recorders to global recorder.
+ *   3. For JavaThreads that are no longer visible by safepoint, also
+ *      need to take ThreadCritical and records are written to global
+ *      recorders, since these threads are NOT walked by Threads.do_thread().
+ *   4. JavaThreads that are running in safepoint-safe states do not stop
+ *      for safepoints, ThreadCritical lock should be taken to write
+ *      memory records.
+ *   5. JavaThreads that are running in VM state do not need any lock and
+ *      records are written to per-thread recorders.
+ *   6. For a thread has yet to attach VM 'Thread', they need to take
+ *      ThreadCritical to write to global recorder.
+ *
+ *  The memory operations that need pre-reserve sequence numbers:
+ *    The memory operations that "release" memory blocks and the
+ *    operations can fail, need to pre-reserve sequence number. They
+ *    are realloc, uncommit and release.
+ *
+ *  The reason for pre-reserve sequence number, is to prevent race condition:
+ *    Thread 1                      Thread 2
+ *    <release>
+ *                                  <allocate>
+ *                                  <write allocate record>
+ *   <write release record>
+ *   if Thread 2 happens to obtain the memory address Thread 1 just released,
+ *   then NMT can mistakenly report the memory is free.
+ *
+ *  Noticeably, free() does not need pre-reserve sequence number, because the call
+ *  does not fail, so we can alway write "release" record before the memory is actaully
+ *  freed.
+ *
+ *  For realloc, uncommit and release, following coding pattern should be used:
+ *
+ *     MemTracker::Tracker tkr = MemTracker::get_realloc_tracker();
+ *     ptr = ::realloc(...);
+ *     if (ptr == NULL) {
+ *       tkr.record(...)
+ *     } else {
+ *       tkr.discard();
+ *     }
+ *
+ *     MemTracker::Tracker tkr = MemTracker::get_virtual_memory_uncommit_tracker();
+ *     if (uncommit(...)) {
+ *       tkr.record(...);
+ *     } else {
+ *       tkr.discard();
+ *     }
+ *
+ *     MemTracker::Tracker tkr = MemTracker::get_virtual_memory_release_tracker();
+ *     if (release(...)) {
+ *       tkr.record(...);
+ *     } else {
+ *       tkr.discard();
+ *     }
+ *
+ * Since pre-reserved sequence number is only good for the generation that it is acquired,
+ * when there is pending Tracker that reserved sequence number, NMT sync-point has
+ * to be skipped to prevent from advancing generation. This is done by inc and dec
+ * MemTracker::_pending_op_count, when MemTracker::_pending_op_count > 0, NMT sync-point is skipped.
+ * Not all pre-reservation of sequence number will increment pending op count. For JavaThreads
+ * that honor safepoints, safepoint can not occur during the memory operations, so the
+ * pre-reserved sequence number won't cross the generation boundry.
+ */
+MemTracker::Tracker::Tracker(MemoryOperation op, Thread* thr) {
+  _op = NoOp;
+  _seq = 0;
+  if (MemTracker::is_on()) {
+    _java_thread = NULL;
+    _op = op;
+
+    // figure out if ThreadCritical lock is needed to write this operation
+    // to MemTracker
+    if (MemTracker::is_single_threaded_bootstrap()) {
+      thr = NULL;
+    } else if (thr == NULL) {
+      // don't use Thread::current(), since it is possible that
+      // the calling thread has yet to attach to VM 'Thread',
+      // which will result assertion failure
+      thr = ThreadLocalStorage::thread();
+    }
+
+    if (thr != NULL) {
+      // Check NMT load
+      MemTracker::check_NMT_load(thr);
+
+      if (thr->is_Java_thread() && ((JavaThread*)thr)->is_safepoint_visible()) {
+        _java_thread = (JavaThread*)thr;
+        JavaThreadState  state = _java_thread->thread_state();
+        // JavaThreads that are safepoint safe, can run through safepoint,
+        // so ThreadCritical is needed to ensure no threads at safepoint create
+        // new records while the records are being gathered and the sequence number is changing
+        _need_thread_critical_lock =
+          SafepointSynchronize::safepoint_safe(_java_thread, state);
+      } else {
+        _need_thread_critical_lock = true;
+      }
+    } else {
+       _need_thread_critical_lock
+         = !MemTracker::is_single_threaded_bootstrap();
+    }
+
+    // see if we need to pre-reserve sequence number for this operation
+    if (_op == Realloc || _op == Uncommit || _op == Release) {
+      if (_need_thread_critical_lock) {
+        ThreadCritical tc;
+        MemTracker::inc_pending_op_count();
+        _seq = SequenceGenerator::next();
+      } else {
+        // for the threads that honor safepoints, no safepoint can occur
+        // during the lifespan of tracker, so we don't need to increase
+        // pending op count.
+        _seq = SequenceGenerator::next();
+      }
+    }
+  }
+}
+
+void MemTracker::Tracker::discard() {
+  if (MemTracker::is_on() && _seq != 0) {
+    if (_need_thread_critical_lock) {
+      ThreadCritical tc;
+      MemTracker::dec_pending_op_count();
+    }
+    _seq = 0;
+  }
+}
+
+
+void MemTracker::Tracker::record(address old_addr, address new_addr, size_t size,
+  MEMFLAGS flags, address pc) {
+  assert(old_addr != NULL && new_addr != NULL, "Sanity check");
+  assert(_op == Realloc || _op == NoOp, "Wrong call");
+  if (MemTracker::is_on() && NMT_CAN_TRACK(flags) && _op != NoOp) {
+    assert(_seq > 0, "Need pre-reserve sequence number");
+    if (_need_thread_critical_lock) {
+      ThreadCritical tc;
+      // free old address, use pre-reserved sequence number
+      MemTracker::write_tracking_record(old_addr, MemPointerRecord::free_tag(),
+        0, _seq, pc, _java_thread);
+      MemTracker::write_tracking_record(new_addr, flags | MemPointerRecord::malloc_tag(),
+        size, SequenceGenerator::next(), pc, _java_thread);
+      // decrement MemTracker pending_op_count
+      MemTracker::dec_pending_op_count();
+    } else {
+      // free old address, use pre-reserved sequence number
+      MemTracker::write_tracking_record(old_addr, MemPointerRecord::free_tag(),
+        0, _seq, pc, _java_thread);
+      MemTracker::write_tracking_record(new_addr, flags | MemPointerRecord::malloc_tag(),
+        size, SequenceGenerator::next(), pc, _java_thread);
+    }
+    _seq = 0;
+  }
+}
+
+void MemTracker::Tracker::record(address addr, size_t size, MEMFLAGS flags, address pc) {
+  // OOM already?
+  if (addr == NULL) return;
+
+  if (MemTracker::is_on() && NMT_CAN_TRACK(flags) && _op != NoOp) {
+    bool pre_reserved_seq = (_seq != 0);
+    address  pc = CALLER_CALLER_PC;
+    MEMFLAGS orig_flags = flags;
+
+    // or the tagging flags
+    switch(_op) {
+      case Malloc:
+        flags |= MemPointerRecord::malloc_tag();
+        break;
+      case Free:
+        flags = MemPointerRecord::free_tag();
+        break;
+      case Realloc:
+        fatal("Use the other Tracker::record()");
+        break;
+      case Reserve:
+      case ReserveAndCommit:
+        flags |= MemPointerRecord::virtual_memory_reserve_tag();
+        break;
+      case Commit:
+        flags = MemPointerRecord::virtual_memory_commit_tag();
+        break;
+      case Type:
+        flags |= MemPointerRecord::virtual_memory_type_tag();
+        break;
+      case Uncommit:
+        assert(pre_reserved_seq, "Need pre-reserve sequence number");
+        flags = MemPointerRecord::virtual_memory_uncommit_tag();
+        break;
+      case Release:
+        assert(pre_reserved_seq, "Need pre-reserve sequence number");
+        flags = MemPointerRecord::virtual_memory_release_tag();
+        break;
+      case ArenaSize:
+        // a bit of hack here, add a small postive offset to arena
+        // address for its size record, so the size record is sorted
+        // right after arena record.
+        flags = MemPointerRecord::arena_size_tag();
+        addr += sizeof(void*);
+        break;
+      case StackRelease:
+        flags = MemPointerRecord::virtual_memory_release_tag();
+        break;
+      default:
+        ShouldNotReachHere();
+    }
+
+    // write memory tracking record
+    if (_need_thread_critical_lock) {
+      ThreadCritical tc;
+      if (_seq == 0) _seq = SequenceGenerator::next();
+      MemTracker::write_tracking_record(addr, flags, size, _seq, pc, _java_thread);
+      if (_op == ReserveAndCommit) {
+        MemTracker::write_tracking_record(addr, orig_flags | MemPointerRecord::virtual_memory_commit_tag(),
+          size, SequenceGenerator::next(), pc, _java_thread);
+      }
+      if (pre_reserved_seq) MemTracker::dec_pending_op_count();
+    } else {
+      if (_seq == 0) _seq = SequenceGenerator::next();
+      MemTracker::write_tracking_record(addr, flags, size, _seq, pc, _java_thread);
+      if (_op == ReserveAndCommit) {
+        MemTracker::write_tracking_record(addr, orig_flags | MemPointerRecord::virtual_memory_commit_tag(),
+          size, SequenceGenerator::next(), pc, _java_thread);
+      }
+    }
+    _seq = 0;
+  }
+}
+

src/share/vm/services/memTracker.hpp

 /*
- * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -54,6 +54,18 @@ class MemTracker : AllStatic {
       NMT_sequence_overflow  // overflow the sequence number
    };
 
+  class Tracker {
+   public:
+    void discard() { }
+
+    void record(address addr, size_t size = 0, MEMFLAGS flags = mtNone, address pc = NULL) { }
+    void record(address old_addr, address new_addr, size_t size,
+      MEMFLAGS flags, address pc = NULL) { }
+  };
+
+  private:
+   static Tracker  _tkr;
+
 
   public:
    static inline void init_tracking_options(const char* option_line) { }
@@ -68,19 +80,18 @@ class MemTracker : AllStatic {
    static inline void record_malloc(address addr, size_t size, MEMFLAGS flags,
         address pc = 0, Thread* thread = NULL) { }
    static inline void record_free(address addr, MEMFLAGS flags, Thread* thread = NULL) { }
-   static inline void record_realloc(address old_addr, address new_addr, size_t size,
-        MEMFLAGS flags, address pc = 0, Thread* thread = NULL) { }
    static inline void record_arena_size(address addr, size_t size) { }
    static inline void record_virtual_memory_reserve(address addr, size_t size,
-        address pc = 0, Thread* thread = NULL) { }
+        MEMFLAGS flags, address pc = 0, Thread* thread = NULL) { }
+   static inline void record_virtual_memory_reserve_and_commit(address addr, size_t size,
+        MEMFLAGS flags, address pc = 0, Thread* thread = NULL) { }
    static inline void record_virtual_memory_commit(address addr, size_t size,
         address pc = 0, Thread* thread = NULL) { }
-   static inline void record_virtual_memory_uncommit(address addr, size_t size,
-        Thread* thread = NULL) { }
-   static inline void record_virtual_memory_release(address addr, size_t size,
-        Thread* thread = NULL) { }
    static inline void record_virtual_memory_type(address base, MEMFLAGS flags,
         Thread* thread = NULL) { }
+   static inline Tracker get_realloc_tracker() { return _tkr; }
+   static inline Tracker get_virtual_memory_uncommit_tracker() { return _tkr; }
+   static inline Tracker get_virtual_memory_release_tracker()  { return _tkr; }
    static inline bool baseline() { return false; }
    static inline bool has_baseline() { return false; }
 
@@ -164,6 +175,45 @@ class MemTracker : AllStatic {
     NMT_shutdown                         // shutdown
   };
 
+ public:
+  class Tracker : public StackObj {
+    friend class MemTracker;
+   public:
+    enum MemoryOperation {
+      NoOp,                   // no op
+      Malloc,                 // malloc
+      Realloc,                // realloc
+      Free,                   // free
+      Reserve,                // virtual memory reserve
+      Commit,                 // virtual memory commit
+      ReserveAndCommit,       // virtual memory reserve and commit
+      StackAlloc = ReserveAndCommit, // allocate thread stack
+      Type,                   // assign virtual memory type
+      Uncommit,               // virtual memory uncommit
+      Release,                // virtual memory release
+      ArenaSize,              // set arena size
+      StackRelease            // release thread stack
+    };
+
+
+   protected:
+    Tracker(MemoryOperation op, Thread* thr = NULL);
+
+   public:
+    void discard();
+
+    void record(address addr, size_t size = 0, MEMFLAGS flags = mtNone, address pc = NULL);
+    void record(address old_addr, address new_addr, size_t size,
+      MEMFLAGS flags, address pc = NULL);
+
+   private:
+    bool            _need_thread_critical_lock;
+    JavaThread*     _java_thread;
+    MemoryOperation _op;          // memory operation
+    jint            _seq;         // reserved sequence number
+  };
+
+
  public:
   // native memory tracking level
   enum NMTLevel {
@@ -276,109 +326,74 @@ class MemTracker : AllStatic {
   // record a 'malloc' call
   static inline void record_malloc(address addr, size_t size, MEMFLAGS flags,
                             address pc = 0, Thread* thread = NULL) {
-    if (is_on() && NMT_CAN_TRACK(flags)) {
-      assert(size > 0, "Sanity check");
-      create_memory_record(addr, (flags|MemPointerRecord::malloc_tag()), size, pc, thread);
-    }
+    Tracker tkr(Tracker::Malloc, thread);
+    tkr.record(addr, size, flags, pc);
   }
   // record a 'free' call
   static inline void record_free(address addr, MEMFLAGS flags, Thread* thread = NULL) {
-    if (is_on() && NMT_CAN_TRACK(flags)) {
-      create_memory_record(addr, MemPointerRecord::free_tag(), 0, 0, thread);
-    }
-  }
-  // record a 'realloc' call
-  static inline void record_realloc(address old_addr, address new_addr, size_t size,
-       MEMFLAGS flags, address pc = 0, Thread* thread = NULL) {
-    if (is_on() && NMT_CAN_TRACK(flags)) {
-      assert(size > 0, "Sanity check");
-      record_free(old_addr, flags, thread);
-      record_malloc(new_addr, size, flags, pc, thread);
-    }
+    Tracker tkr(Tracker::Free, thread);
+    tkr.record(addr, 0, flags, DEBUG_CALLER_PC);
   }
 
-  // record arena memory size
   static inline void record_arena_size(address addr, size_t size) {
-    // we add a positive offset to arena address, so we can have arena memory record
-    // sorted after arena record
-    if (is_on() && !UseMallocOnly) {
-      assert(addr != NULL, "Sanity check");
-      create_memory_record((addr + sizeof(void*)), MemPointerRecord::arena_size_tag(), size,
-        DEBUG_CALLER_PC, NULL);
-    }
+    Tracker tkr(Tracker::ArenaSize);
+    tkr.record(addr, size);
   }
 
   // record a virtual memory 'reserve' call
   static inline void record_virtual_memory_reserve(address addr, size_t size,
-                            address pc = 0, Thread* thread = NULL) {
-    if (is_on()) {
-      assert(size > 0, "Sanity check");
-      create_memory_record(addr, MemPointerRecord::virtual_memory_reserve_tag(),
-                           size, pc, thread);
-    }
+                     MEMFLAGS flags, address pc = 0, Thread* thread = NULL) {
+    assert(size > 0, "Sanity check");
+    Tracker tkr(Tracker::Reserve, thread);
+    tkr.record(addr, size, flags, pc);
   }
 
   static inline void record_thread_stack(address addr, size_t size, Thread* thr,
                            address pc = 0) {
-    if (is_on()) {
-      assert(size > 0 && thr != NULL, "Sanity check");
-      create_memory_record(addr, MemPointerRecord::virtual_memory_reserve_tag() | mtThreadStack,
-                          size, pc, thr);
-      create_memory_record(addr, MemPointerRecord::virtual_memory_commit_tag() | mtThreadStack,
-                          size, pc, thr);
-    }
+    Tracker tkr(Tracker::StackAlloc, thr);
+    tkr.record(addr, size, mtThreadStack, pc);
   }
 
   static inline void release_thread_stack(address addr, size_t size, Thread* thr) {
-    if (is_on()) {
-      assert(size > 0 && thr != NULL, "Sanity check");
-      assert(!thr->is_Java_thread(), "too early");
-      create_memory_record(addr, MemPointerRecord::virtual_memory_uncommit_tag() | mtThreadStack,
-                          size, DEBUG_CALLER_PC, thr);
-      create_memory_record(addr, MemPointerRecord::virtual_memory_release_tag() | mtThreadStack,
-                          size, DEBUG_CALLER_PC, thr);
-    }
+    Tracker tkr(Tracker::StackRelease, thr);
+    tkr.record(addr, size, mtThreadStack, DEBUG_CALLER_PC);
   }
 
   // record a virtual memory 'commit' call
   static inline void record_virtual_memory_commit(address addr, size_t size,
                             address pc, Thread* thread = NULL) {
-    if (is_on()) {
-      assert(size > 0, "Sanity check");
-      create_memory_record(addr, MemPointerRecord::virtual_memory_commit_tag(),
-                           size, pc, thread);
-    }
+    Tracker tkr(Tracker::Commit, thread);
+    tkr.record(addr, size, mtNone, pc);
   }
 
-  // record a virtual memory 'uncommit' call
-  static inline void record_virtual_memory_uncommit(address addr, size_t size,
-                            Thread* thread = NULL) {
-    if (is_on()) {
-      assert(size > 0, "Sanity check");
-      create_memory_record(addr, MemPointerRecord::virtual_memory_uncommit_tag(),
-                           size, DEBUG_CALLER_PC, thread);
-    }
+  static inline void record_virtual_memory_reserve_and_commit(address addr, size_t size,
+    MEMFLAGS flags, address pc, Thread* thread = NULL) {
+    Tracker tkr(Tracker::ReserveAndCommit, thread);
+    tkr.record(addr, size, flags, pc);
   }
 
-  // record a virtual memory 'release' call
-  static inline void record_virtual_memory_release(address addr, size_t size,
-                            Thread* thread = NULL) {
-    if (is_on()) {
-      assert(size > 0, "Sanity check");
-      create_memory_record(addr, MemPointerRecord::virtual_memory_release_tag(),
-                           size, DEBUG_CALLER_PC, thread);
-    }
-  }
 
   // record memory type on virtual memory base address
   static inline void record_virtual_memory_type(address base, MEMFLAGS flags,
                             Thread* thread = NULL) {
-    if (is_on()) {
-      assert(base > 0, "wrong base address");
-      assert((flags & (~mt_masks)) == 0, "memory type only");
-      create_memory_record(base, (flags | MemPointerRecord::virtual_memory_type_tag()),
-                           0, DEBUG_CALLER_PC, thread);
-    }
+    Tracker tkr(Tracker::Type);
+    tkr.record(base, 0, flags);
+  }
+
+  // Get memory trackers for memory operations that can result race conditions.
+  // The memory tracker has to be obtained before realloc, virtual memory uncommit
+  // and virtual memory release, and call tracker.record() method if operation
+  // succeeded, or tracker.discard() to abort the tracking.
+  static inline Tracker get_realloc_tracker() {
+    return Tracker(Tracker::Realloc);
+  }
+
+  static inline Tracker get_virtual_memory_uncommit_tracker() {
+    return Tracker(Tracker::Uncommit);
+  }
+
+  static inline Tracker get_virtual_memory_release_tracker() {
+    return Tracker(Tracker::Release);
   }
 
 
@@ -444,6 +459,31 @@ class MemTracker : AllStatic {
   static MemRecorder* get_pending_recorders();
   static void delete_all_pending_recorders();
 
+  // write a memory tracking record in recorder
+  static void write_tracking_record(address addr, MEMFLAGS type,
+    size_t size, jint seq, address pc, JavaThread* thread);
+
+  static bool is_single_threaded_bootstrap() {
+    return _state == NMT_bootstrapping_single_thread;
+  }
+
+  static void check_NMT_load(Thread* thr) {
+    assert(thr != NULL, "Sanity check");
+    if (_slowdown_calling_thread && thr != _worker_thread) {
+      os::yield_all();
+    }
+  }
+
+  static void inc_pending_op_count() {
+    Atomic::inc(&_pending_op_count);
+  }
+
+  static void dec_pending_op_count() {
+    Atomic::dec(&_pending_op_count);
+    assert(_pending_op_count >= 0, "Sanity check");
+  }
+
+
  private:
   // retrieve a pooled memory record or create new one if there is not
   // one available
@@ -522,6 +562,12 @@ class MemTracker : AllStatic {
   // if NMT should slow down calling thread to allow
   // worker thread to catch up
   static volatile bool             _slowdown_calling_thread;
+
+  // pending memory op count.
+  // Certain memory ops need to pre-reserve sequence number
+  // before memory operation can happen to avoid race condition.
+  // See MemTracker::Tracker for detail
+  static volatile jint             _pending_op_count;
 };
 
 #endif // !INCLUDE_NMT