Browse Source

Fix GC when local objects transfer ownership

Greg 14 years ago
parent
commit
692531a3c8

+ 4 - 1
gmsrc/doc/ChangeLog.txt

@@ -381,10 +381,13 @@ o Changed #define in MSVC platform file to _M_X64 which is correct 64bit build m
 o Add 64bit MSVC build configurations for GME
 
 2/10/11
-o Fixed GC root scan.
+o Fixed GC root scan. (PS. Failed attempt)
 o Increased GMTHREAD_MAXBYTESIZE to allow samples to run in 64bit
 o Fixed gmTable 64bit gmVariable equality test.
 o Fixed byte code generator. 64bit was mixing 32 and 64 bit patched addresses.
 
 3/10/11
 o Fixed GC root scan to ignore persistent objects.
+
+4/10/11
+o Fixed GC tricolor invariance wasn't preserved with local object transfer of ownership.

+ 0 - 69
gmsrc/src/gm/gmIncGC.cpp

@@ -233,75 +233,6 @@ void gmGCColorSet::GrayThisObject(gmGCObjBase* a_obj)
 }
 
 
-void gmGCColorSet::GrayThisRootObject(gmGCObjBase* a_obj)
-{
-  // Normally, a object will never go from black to gray, only black to white or white to gray/black.
-  // We need to enforce that roots are treated as gray, and in this language and GC implementation
-  // it is convenient to force roots to gray from whatever logical state they are in.
-  // This is due to GC design choices including: atomic root snapshot, write barrier and allocate black.
-
-#if  GM_GC_KEEP_PERSISTANT_SEPARATE
-  if(a_obj->GetPersist()) // Don't do anything with persistant objects
-  {
-    return;
-  }
-#endif //GM_GC_KEEP_PERSISTANT_SEPARATE
-
-  gmGCObjBase* objPrev = a_obj->GetPrev();
-  gmGCObjBase* objNext = a_obj->GetNext();
-
-#if GM_GC_DEBUG
-  GM_ASSERT(a_obj->m_curPosColor != GM_GC_DEBUG_COL_FREE);
-  GM_ASSERT(a_obj->m_curPosColor != GM_GC_DEBUG_COL_INVALID);
-  a_obj->m_curPosColor = GM_GC_DEBUG_COL_GRAY;
-#endif //GM_GC_DEBUG
-
-  // Set object`s color to shaded first.
-  a_obj->SetColor(m_gc->GetCurShadeColor());
-
-  // The object must be shaded
-  GM_ASSERT(m_gc->IsShaded(a_obj));
-
-  // Splice the object out of the list
-  objPrev->SetNext(objNext);
-  objNext->SetPrev(objPrev);
-
-  // Fix sentinels
-  if( m_scan == a_obj )
-  {
-    m_scan = m_scan->GetNext(); // Not Prev as scan should be the start of black inclusive
-  }
-  if( m_free == a_obj )
-  {
-    m_free = m_free->GetNext();
-  }
-  
-  // We are scanning roots, not tracing, check that this is so
-  GM_ASSERT( m_gc->GetTraceState().m_done );
-
-  // Put the object into the correct place in the gray list
-
-#if DEPTH_FIRST
-  // Put the gray object at the head of the gray list.
-  a_obj->SetPrev(m_scan->GetPrev());
-  a_obj->SetNext(m_scan);
-  m_scan->GetPrev()->SetNext(a_obj);
-  m_scan->SetPrev(a_obj);
-#else // BREADTH_FIRST
-  // Put the gray object at the tail of the gray list.
-  a_obj->SetPrev(m_gray);
-  a_obj->SetNext(m_gray->GetNext());
-  m_gray->GetNext()->SetPrev(a_obj);
-  m_gray->SetNext(a_obj);
-#endif // BREADTH_FIRST
-
-#if GM_GC_DEBUG
-  // Slow, paranoid check
-  VerifyIntegrity();
-#endif //GM_GC_DEBUG
-}
-
-
 void gmGCColorSet::Revive(gmGCObjBase* a_obj)
 {
   // NOTE: Once objects are in the free list, we can't trust the color mark, 

+ 0 - 6
gmsrc/src/gm/gmIncGC.h

@@ -162,9 +162,6 @@ public:
   /// \brief Gray this object.
   void GrayThisObject(gmGCObjBase* a_obj);
 
-  /// \brief Gray this root object (Called to force roots to gray, which they logically always are)
-  void GrayThisRootObject(gmGCObjBase* a_obj);
-  
   /// \brief Called on a new object being allocated.
   void Allocate(gmGCObjBase* a_obj);
 
@@ -290,9 +287,6 @@ public:
   /// \brief Do a full collect and don't return until done.
   void FullCollect();
 
-  /// \brief Called during trace by machine ONLY on ROOT objects (eg. stacks)
-  void GetNextRootObject(gmGCObjBase* a_obj)      {m_colorSet.GrayThisRootObject(a_obj);}
-
   /// \brief Called during trace by client code, and by scan roots callback.  
   /// This grays a white object.
   inline void GetNextObject(gmGCObjBase* a_obj)   {m_colorSet.GrayAWhite(a_obj);}

+ 4 - 4
gmsrc/src/gm/gmMachine.cpp

@@ -157,7 +157,7 @@ void GM_CDECL gmMachine::ScanRootsCallBack(gmMachine* a_machine, gmGarbageCollec
   for(cgmoIt = a_machine->m_cppOwnedGMObjs.First(); cgmoIt; ++cgmoIt)
   {
     gmObject* curObj = cgmoIt->m_obj;
-    a_gc->GetNextRootObject(curObj);
+    a_gc->GetNextObject(curObj);
   }
 
   // iterate over all threads and mark the stacks.
@@ -169,20 +169,20 @@ void GM_CDECL gmMachine::ScanRootsCallBack(gmMachine* a_machine, gmGarbageCollec
   // iterate over global variables and mark
   if(a_machine->m_global)
   {
-    a_gc->GetNextRootObject(a_machine->m_global);
+    a_gc->GetNextObject(a_machine->m_global);
   }
   // iterate over type variables and mark
   gmuint i;
   for(i = 0; i < a_machine->m_types.Count(); ++i)
   {
-    a_gc->GetNextRootObject(a_machine->m_types[i].m_variables);
+    a_gc->GetNextObject(a_machine->m_types[i].m_variables);
   }
 
 #if !GM_GC_KEEP_PERSISTANT_SEPARATE
   //NOTE This needs to be spread over time perhaps.
   for(i=0; i<(gmuint)a_machine->m_numPermanantStrings; ++i)
   {
-    a_gc->GetNextRootObject(a_machine->m_permanantStrings[i]);
+    a_gc->GetNextObject(a_machine->m_permanantStrings[i]);
   }
 #endif //!GM_GC_KEEP_PERSISTANT_SEPARATE
 }

+ 16 - 3
gmsrc/src/gm/gmThread.cpp

@@ -102,7 +102,7 @@ void gmThread::GCScanRoots(gmMachine* a_machine, gmGarbageCollector* a_gc)
     if(m_stack[i].IsReference())
     {
       gmObject * object = GM_MOBJECT(m_machine, m_stack[i].m_value.m_ref);
-      a_gc->GetNextRootObject(object);
+      a_gc->GetNextObject(object);
     }
   }
 
@@ -113,7 +113,7 @@ void gmThread::GCScanRoots(gmMachine* a_machine, gmGarbageCollector* a_gc)
     if(signal->m_signal.IsReference())
     {
       gmObject * object = GM_MOBJECT(m_machine, signal->m_signal.m_value.m_ref);
-      a_gc->GetNextRootObject(object);
+      a_gc->GetNextObject(object);
     }
     signal = signal->m_nextSignal;
   }
@@ -125,7 +125,7 @@ void gmThread::GCScanRoots(gmMachine* a_machine, gmGarbageCollector* a_gc)
     if(block->m_block.IsReference())
     {
       gmObject * object = GM_MOBJECT(m_machine, block->m_block.m_value.m_ref);
-      a_gc->GetNextRootObject(object);
+      a_gc->GetNextObject(object);
     }
     block = block->m_nextBlock;
   }
@@ -1181,6 +1181,19 @@ gmThread::State gmThread::Sys_PopStackFrame(const gmuint8 * &a_ip, const gmuint8
     return SYS_EXCEPTION;
   }
 
+  // Write barrier old local objects
+  {
+    gmGarbageCollector* gc = m_machine->GetGC();
+    for(int index = m_base; index < m_top; ++index) // NOTE: Should this be from m_base - 2 ?
+    {
+      if(m_stack[index].IsReference())
+      {
+        gmObject * object = GM_MOBJECT(m_machine, m_stack[index].m_value.m_ref);
+        gc->WriteBarrier(object);
+      }
+    }
+  }
+
   gmStackFrame * frame = m_frame->m_prev;
   if( frame == NULL ) // Final frame, we will exit now
   {