http://bugzilla/bugzilla/show_bug.cgi?id=4120 GC depends on order of global object destruction.

1 files changed, 30 insertions, 25 deletions

diff --git a/cpp/src/Ice/GC.cpp b/cpp/src/Ice/GC.cpp
index 9e7f475b6c1..1b49eb6c4db 100644
--- a/cpp/src/Ice/GC.cpp
+++ b/cpp/src/Ice/GC.cpp
@@ -8,7 +8,6 @@
 // **********************************************************************
 
 #include <IceUtil/Time.h>
-#include <IceUtil/Mutex.h>
 #include <IceUtil/MutexPtrLock.h>
 #include <IceUtil/RecMutex.h>
 #include <Ice/GC.h>
@@ -17,15 +16,14 @@
 
 using namespace IceUtil;
 
-namespace
+namespace 
 {
 
-Mutex* numCollectorsMutex = 0;
-int numCollectors = 0;
+static Mutex* numCollectorsMutex = 0;
+static int numCollectors = 0;
 typedef std::set<IceInternal::GCShared*> GCObjectSet;
-GCObjectSet gcObjects; // Set of pointers to all existing classes with class data members.
-
-RecMutex gcRecMutex;
+static GCObjectSet* gcObjects = 0; // Set of pointers to all existing classes with class data members.
+static RecMutex* gcRecMutex = 0;
 
 class Init
 {
@@ -34,12 +32,18 @@ public:
     Init()
     {
         numCollectorsMutex = new IceUtil::Mutex;
+        gcObjects = new GCObjectSet();
+        gcRecMutex = new RecMutex();
     }
 
     ~Init()
     {
         delete numCollectorsMutex;
         numCollectorsMutex = 0;
+        delete gcRecMutex;
+        gcRecMutex = 0;
+        delete gcObjects;
+        gcObjects = 0;
     }
 };
 
@@ -78,7 +82,7 @@ using namespace IceInternal;
 void
 IceInternal::GCShared::__incRef()
 {
-    RecMutex::Lock lock(gcRecMutex);
+    IceUtilInternal::MutexPtrLock<IceUtil::RecMutex> lock(gcRecMutex);
     assert(_ref >= 0);
     ++_ref;
 }
@@ -86,7 +90,7 @@ IceInternal::GCShared::__incRef()
 void
 IceInternal::GCShared::__decRef()
 {
-    RecMutex::Lock lock(gcRecMutex);
+    IceUtilInternal::MutexPtrLock<IceUtil::RecMutex> lock(gcRecMutex);
     bool doDelete = false;
     assert(_ref > 0);
     if(--_ref == 0)
@@ -104,28 +108,28 @@ IceInternal::GCShared::__decRef()
 int
 IceInternal::GCShared::__getRef() const
 {
-    RecMutex::Lock lock(gcRecMutex);
+    IceUtilInternal::MutexPtrLock<IceUtil::RecMutex> lock(gcRecMutex);
     return _ref;
 }
 
 void
 IceInternal::GCShared::__setNoDelete(bool b)
 {
-    RecMutex::Lock lock(gcRecMutex);
+    IceUtilInternal::MutexPtrLock<IceUtil::RecMutex> lock(gcRecMutex);
     _noDelete = b;
 }
 
 void
 IceInternal::GCShared::__gcIncRef()
 {
-    RecMutex::Lock lock(gcRecMutex);
+    IceUtilInternal::MutexPtrLock<IceUtil::RecMutex> lock(gcRecMutex);
     assert(_ref >= 0);
-    if(_ref == 0)
+    if(_ref == 0 && gcObjects != 0)
     {
 #ifdef NDEBUG // To avoid annoying warnings about variables that are not used...
-        gcObjects.insert(this);
+        gcObjects->insert(this);
 #else
-        std::pair<GCObjectSet::iterator, bool> rc = gcObjects.insert(this);
+        std::pair<GCObjectSet::iterator, bool> rc = gcObjects->insert(this);
         assert(rc.second);
 #endif
     }
@@ -135,17 +139,17 @@ IceInternal::GCShared::__gcIncRef()
 void
 IceInternal::GCShared::__gcDecRef()
 {
-    RecMutex::Lock lock(gcRecMutex);
+    IceUtilInternal::MutexPtrLock<IceUtil::RecMutex> lock(gcRecMutex);
     bool doDelete = false;
     assert(_ref > 0);
-    if(--_ref == 0)
+    if(--_ref == 0 && gcObjects != 0)
     {
         doDelete = !_noDelete;
         _noDelete = true;
 #ifdef NDEBUG // To avoid annoying warnings about variables that are not used...
-        gcObjects.erase(this);
+        gcObjects->erase(this);
 #else
-        GCObjectSet::size_type num = gcObjects.erase(this);
+        GCObjectSet::size_type num = gcObjects->erase(this);
         assert(num == 1);
 #endif
     }
@@ -162,8 +166,7 @@ IceInternal::GCShared::__gcDecRef()
 //
 
 
-IceInternal::GC::GC(int interval, StatsCallback cb) :
-    Thread("Ice garbage collector thread")
+IceInternal::GC::GC(int interval, StatsCallback cb)
 {
     IceUtilInternal::MutexPtrLock<IceUtil::Mutex> sync(numCollectorsMutex);
     if(numCollectors++ > 0)
@@ -269,9 +272,11 @@ IceInternal::GC::collectGarbage()
             return;
         }
         _collecting = true;
+        assert(gcObjects != 0);
     }
 
-    RecMutex::Lock sync(gcRecMutex); // Prevent any further class reference count activity.
+    // Prevent any further class reference count activity.
+    IceUtilInternal::MutexPtrLock<IceUtil::RecMutex> lock(gcRecMutex);
 
     Time t;
     GCStats stats;
@@ -279,7 +284,7 @@ IceInternal::GC::collectGarbage()
     if(_statsCallback)
     {
         t = Time::now(IceUtil::Time::Monotonic);
-        stats.examined = static_cast<int>(gcObjects.size());
+        stats.examined = static_cast<int>(gcObjects->size());
     }
 
     GCCountMap counts;
@@ -296,7 +301,7 @@ IceInternal::GC::collectGarbage()
         //
         GCCountMap reachable;
         {
-            for(GCObjectSet::const_iterator i = gcObjects.begin(); i != gcObjects.end(); ++i)
+            for(GCObjectSet::const_iterator i = gcObjects->begin(); i != gcObjects->end(); ++i)
             {
                 counts.insert(GCCountMap::value_type(*i, (*i)->__getRefUnsafe()));
                 (*i)->__gcReachable(reachable);
@@ -370,7 +375,7 @@ IceInternal::GC::collectGarbage()
         }
         for(i = counts.begin(); i != counts.end(); ++i)
         {
-            gcObjects.erase(i->first); // Remove this object from candidate set.
+            gcObjects->erase(i->first); // Remove this object from candidate set.
             delete i->first; // Delete this object.
         }
     }