6994628: G1: Test gc/gctests/FinalizeTest05 fails (one live object is finalized)
authorjohnc
Tue, 07 Dec 2010 16:18:45 -0800
changeset 7417 0fa6a0a76d54
parent 7400 39b4f2314833
child 7418 ca00dd5fcae0
6994628: G1: Test gc/gctests/FinalizeTest05 fails (one live object is finalized) Summary: The Solaris Studio 12 update 1 C++ compiler was incorrectly re-ordering the reads of an object's mark word in oopDesc::forward_to_atomic(). This opened a small window where one thread could execute the successful CAS path even though another thread had already successfully forwarded the object. This could result in an object being copied twice. The code in oopDesc::forward_to_atomic() was changed to read the mark word once. Reviewed-by: ysr, tonyp
hotspot/src/share/vm/oops/oop.pcgc.inline.hpp
--- a/hotspot/src/share/vm/oops/oop.pcgc.inline.hpp	Thu Dec 02 13:20:39 2010 -0500
+++ b/hotspot/src/share/vm/oops/oop.pcgc.inline.hpp	Tue Dec 07 16:18:45 2010 -0800
@@ -118,12 +118,15 @@
   assert(forwardPtrMark->decode_pointer() == p, "encoding must be reversable");
   assert(sizeof(markOop) == sizeof(intptr_t), "CAS below requires this.");
 
-  while (!is_forwarded()) {
+  while (!oldMark->is_marked()) {
     curMark = (markOop)Atomic::cmpxchg_ptr(forwardPtrMark, &_mark, oldMark);
+    assert(is_forwarded(), "object should have been forwarded");
     if (curMark == oldMark) {
-      assert(is_forwarded(), "the CAS should have succeeded.");
       return NULL;
     }
+    // If the CAS was unsuccessful then curMark->is_marked()
+    // should return true as another thread has CAS'd in another
+    // forwarding pointer.
     oldMark = curMark;
   }
   return forwardee();