Acquire and release Heap_lock on VM_MMTkSTWOperation (#316)

This PR changes how we acquire `Heap_lock` for GC. With the changes, we
acquire the lock in `VM_MMTkSTWOperation::doit_prologue()`, and release
the lock in epilogue. We hold the lock during the entire STW. This is
the same behavior as OpenJDK's GCs.

We used to acquire the lock during reference enqueueing before
`swap_reference_pending_list()` (which requires the lock to be locked),
and acquire the lock before notifying the reference handler. This causes
deadlock as seen in #315 .
The `has_reference_pending_list` call used for an assertion in reference
enqueueing is removed, as it requires the current thread to hold the
lock which cannot be satisfied.

This PR should fix #315.

Author: qinsoon

openjdk/mmtkUpcalls.cpp

@@ -87,15 +87,6 @@ static void mmtk_resume_mutators(void *tls) {
     MutexLockerEx locker(MMTkHeap::heap()->gc_lock(), Mutex::_no_safepoint_check_flag);
     MMTkHeap::heap()->gc_lock()->notify_all();
   }
-
-  log_debug(gc)("Notifying mutators blocking on Heap_lock for reference pending list...");
-  // Note: That's the ReferenceHandler thread.
-  {
-    MutexLockerEx x(Heap_lock, Mutex::_no_safepoint_check_flag);
-    if (Universe::has_reference_pending_list()) {
-      Heap_lock->notify_all();
-    }
-  }
 }
 
 static const int GC_THREAD_KIND_WORKER = 1;
@@ -302,8 +293,6 @@ static void mmtk_enqueue_references(void** objects, size_t len) {
     return;
   }
 
-  MutexLocker x(Heap_lock);
-
   oop first = (oop) objects[0]; // This points to the first node of the linked list.
   oop last = first; // This points to the last node of the linked list.
 
@@ -324,7 +313,6 @@ static void mmtk_enqueue_references(void** objects, size_t len) {
 
   oop old_first = Universe::swap_reference_pending_list(first);
   HeapAccess<AS_NO_KEEPALIVE>::oop_store_at(last, java_lang_ref_Reference::discovered_offset, old_first);
-  assert(Universe::has_reference_pending_list(), "Reference pending list is empty after swap");
 }
 
 OpenJDK_Upcalls mmtk_upcalls = {

openjdk/mmtkVMOperation.cpp

@@ -32,8 +32,21 @@ VM_MMTkSTWOperation::VM_MMTkSTWOperation(MMTkVMCompanionThread *companion_thread
     _companion_thread(companion_thread) {
 }
 
+bool VM_MMTkSTWOperation::doit_prologue() {
+    Heap_lock->lock();
+    return true;
+}
+
 void VM_MMTkSTWOperation::doit() {
     log_trace(vmthread)("Entered VM_MMTkSTWOperation::doit().");
     _companion_thread->do_mmtk_stw_operation();
     log_trace(vmthread)("Leaving VM_MMTkSTWOperation::doit()");
 }
+
+void VM_MMTkSTWOperation::doit_epilogue() {
+    // Notify the reference processing thread
+    if (Universe::has_reference_pending_list()) {
+        Heap_lock->notify_all();
+    }
+    Heap_lock->unlock();
+}

openjdk/mmtkVMOperation.hpp

@@ -39,7 +39,9 @@ class VM_MMTkSTWOperation : public VM_MMTkOperation {
 
 public:
   VM_MMTkSTWOperation(MMTkVMCompanionThread *companion_thread);
+  virtual bool doit_prologue() override;
   virtual void doit() override;
+  virtual void doit_epilogue() override;
 };
 
 #endif // MMTK_OPENJDK_MMTK_VM_OPERATION_HPP