--- a/src/share/vm/gc_implementation/g1/heapRegionSets.cpp	Wed Nov 02 08:04:23 2011 +0100
+++ b/src/share/vm/gc_implementation/g1/heapRegionSets.cpp	Mon Nov 07 22:11:12 2011 -0500
@@ -26,6 +26,17 @@
 #include "gc_implementation/g1/heapRegionRemSet.hpp"
 #include "gc_implementation/g1/heapRegionSets.hpp"
 
+// Note on the check_mt_safety() methods below:
+//
+// Verification of the "master" heap region sets / lists that are
+// maintained by G1CollectedHeap is always done during a STW pause and
+// by the VM thread at the start / end of the pause. The standard
+// verification methods all assert check_mt_safety(). This is
+// important as it ensures that verification is done without
+// concurrent updates taking place at the same time. It follows, that,
+// for the "master" heap region sets / lists, the check_mt_safety()
+// method should include the VM thread / STW case.
+
 //////////////////// FreeRegionList ////////////////////
 
 const char* FreeRegionList::verify_region_extra(HeapRegion* hr) {
@@ -33,7 +44,7 @@
     return "the region should not be young";
   }
   // The superclass will check that the region is empty and
-  // not-humongous.
+  // not humongous.
   return HeapRegionLinkedList::verify_region_extra(hr);
 }
 
@@ -58,12 +69,16 @@
   // (b) If we're not at a safepoint, operations on the master free
   // list should be invoked while holding the Heap_lock.
 
-  guarantee((SafepointSynchronize::is_at_safepoint() &&
-               (Thread::current()->is_VM_thread() ||
-                                            FreeList_lock->owned_by_self())) ||
-            (!SafepointSynchronize::is_at_safepoint() &&
-                                                Heap_lock->owned_by_self()),
-            hrs_ext_msg(this, "master free list MT safety protocol"));
+  if (SafepointSynchronize::is_at_safepoint()) {
+    guarantee(Thread::current()->is_VM_thread() ||
+              FreeList_lock->owned_by_self(),
+              hrs_ext_msg(this, "master free list MT safety protocol "
+                                "at a safepoint"));
+  } else {
+    guarantee(Heap_lock->owned_by_self(),
+              hrs_ext_msg(this, "master free list MT safety protocol "
+                                "outside a safepoint"));
+  }
 
   return FreeRegionList::check_mt_safety();
 }
@@ -81,6 +96,48 @@
   return FreeRegionList::check_mt_safety();
 }
 
+//////////////////// OldRegionSet ////////////////////
+
+const char* OldRegionSet::verify_region_extra(HeapRegion* hr) {
+  if (hr->is_young()) {
+    return "the region should not be young";
+  }
+  // The superclass will check that the region is not empty and not
+  // humongous.
+  return HeapRegionSet::verify_region_extra(hr);
+}
+
+//////////////////// MasterOldRegionSet ////////////////////
+
+bool MasterOldRegionSet::check_mt_safety() {
+  // Master Old Set MT safety protocol:
+  // (a) If we're at a safepoint, operations on the master old set
+  // should be invoked:
+  // - by the VM thread (which will serialize them), or
+  // - by the GC workers while holding the FreeList_lock, if we're
+  //   at a safepoint for an evacuation pause (this lock is taken
+  //   anyway when an GC alloc region is retired so that a new one
+  //   is allocated from the free list), or
+  // - by the GC workers while holding the OldSets_lock, if we're at a
+  //   safepoint for a cleanup pause.
+  // (b) If we're not at a safepoint, operations on the master old set
+  // should be invoked while holding the Heap_lock.
+
+  if (SafepointSynchronize::is_at_safepoint()) {
+    guarantee(Thread::current()->is_VM_thread() ||
+              _phase == HRSPhaseEvacuation && FreeList_lock->owned_by_self() ||
+              _phase == HRSPhaseCleanup && OldSets_lock->owned_by_self(),
+              hrs_ext_msg(this, "master old set MT safety protocol "
+                                "at a safepoint"));
+  } else {
+    guarantee(Heap_lock->owned_by_self(),
+              hrs_ext_msg(this, "master old set MT safety protocol "
+                                "outside a safepoint"));
+  }
+
+  return OldRegionSet::check_mt_safety();
+}
+
 //////////////////// HumongousRegionSet ////////////////////
 
 const char* HumongousRegionSet::verify_region_extra(HeapRegion* hr) {
@@ -103,11 +160,16 @@
   // (b) If we're not at a safepoint, operations on the master
   // humongous set should be invoked while holding the Heap_lock.
 
-  guarantee((SafepointSynchronize::is_at_safepoint() &&
-               (Thread::current()->is_VM_thread() ||
-                                             OldSets_lock->owned_by_self())) ||
-            (!SafepointSynchronize::is_at_safepoint() &&
-                                                 Heap_lock->owned_by_self()),
-            hrs_ext_msg(this, "master humongous set MT safety protocol"));
+  if (SafepointSynchronize::is_at_safepoint()) {
+    guarantee(Thread::current()->is_VM_thread() ||
+              OldSets_lock->owned_by_self(),
+              hrs_ext_msg(this, "master humongous set MT safety protocol "
+                                "at a safepoint"));
+  } else {
+    guarantee(Heap_lock->owned_by_self(),
+              hrs_ext_msg(this, "master humongous set MT safety protocol "
+                                "outside a safepoint"));
+  }
+
   return HumongousRegionSet::check_mt_safety();
 }