summaryrefslogtreecommitdiff
path: root/cpp
diff options
context:
space:
mode:
Diffstat (limited to 'cpp')
-rw-r--r--cpp/CHANGES3
-rwxr-xr-xcpp/include/Ice/GC.h2
-rwxr-xr-xcpp/include/Ice/GCShared.h11
-rwxr-xr-xcpp/src/Ice/GC.cpp27
-rwxr-xr-xcpp/src/Ice/GCShared.cpp11
-rw-r--r--cpp/src/slice2cpp/Gen.cpp34
-rw-r--r--cpp/test/Ice/gc/Client.cpp25
7 files changed, 82 insertions, 31 deletions
diff --git a/cpp/CHANGES b/cpp/CHANGES
index 6b85e1e7619..40506d62d70 100644
--- a/cpp/CHANGES
+++ b/cpp/CHANGES
@@ -1,6 +1,9 @@
Changes since version 3.0.0
---------------------------
+- Fixed a bug in the class garbage collector that could result in double
+ deallocation of class instances.
+
- IceUtil::Time::toString() has been deprecated and should no longer
be used. Instead, use IceUtil::Time::toDateTime() to get a date
and time string. (toString() will continue to work as a synonym for
diff --git a/cpp/include/Ice/GC.h b/cpp/include/Ice/GC.h
index 88bad7e11b9..80a617f0dc8 100755
--- a/cpp/include/Ice/GC.h
+++ b/cpp/include/Ice/GC.h
@@ -13,7 +13,7 @@
#include <IceUtil/Config.h>
#include <IceUtil/Thread.h>
#include <IceUtil/Monitor.h>
-#include <IceUtil/Mutex.h>
+#include <IceUtil/Mutex.h>
#include <Ice/Config.h>
namespace IceInternal
diff --git a/cpp/include/Ice/GCShared.h b/cpp/include/Ice/GCShared.h
index 047caffac8d..5090ddd12e2 100755
--- a/cpp/include/Ice/GCShared.h
+++ b/cpp/include/Ice/GCShared.h
@@ -40,9 +40,15 @@ public:
virtual void __incRef(); // First derived class with class data members overrides this.
virtual void __decRef(); // Ditto.
+ virtual void __addObject(GCObjectMultiSet&) {} // Ditto.
+ virtual bool __usesClasses() { return false; } // Ditto.
+
virtual int __getRef() const;
virtual void __setNoDelete(bool);
+ virtual void __gcReachable(GCObjectMultiSet&) const = 0;
+ virtual void __gcClear() = 0;
+
int __getRefUnsafe() const
{
return _ref;
@@ -53,13 +59,8 @@ public:
--_ref;
}
- virtual void __gcReachable(GCObjectMultiSet&) const = 0;
- virtual void __gcClear() = 0;
-
protected:
- static void __addObject(GCObjectMultiSet&, GCShared*);
-
int _ref;
bool _noDelete;
diff --git a/cpp/src/Ice/GC.cpp b/cpp/src/Ice/GC.cpp
index 938ee6fab56..463eff13fd4 100755
--- a/cpp/src/Ice/GC.cpp
+++ b/cpp/src/Ice/GC.cpp
@@ -163,10 +163,11 @@ IceInternal::GC::collectGarbage()
//
// gcObjects contains the set of class instances that have at least one member of class type,
// that is, gcObjects contains all those instances that can point at other instances.
- // Call this the the candiate set.
+ // Call this the candidate set.
// Build a multiset of instances that are immediately (not recursively) reachable from instances
- // in the candidate set. This adds leaf nodes (class instances that are pointed at, but cannot
- // point at anything themselves) to the multiset.
+ // in the candidate set. This adds adds all immediately reachable nodes to the reachable set, but
+ // does not add nodes that cannot participate in a cycle (class instances that are pointed at, but cannot
+ // point at anything themselves because they have no class members) to the multiset.
//
GCObjectMultiSet reachable;
{
@@ -177,7 +178,7 @@ IceInternal::GC::collectGarbage()
}
//
- // Create a map of reference counts.
+ // Create a map of reference counts of all objects in the reachable set.
//
typedef map<GCShared*, int> ObjectCounts;
ObjectCounts counts;
@@ -188,7 +189,7 @@ IceInternal::GC::collectGarbage()
pos = counts.find(*i);
//
- // If this instance is not in the counts set yet, insert it with its reference count - 1;
+ // If this instance is not in the counts map yet, insert it with its reference count - 1;
// otherwise, decrement its reference count.
//
if(pos == counts.end())
@@ -203,8 +204,8 @@ IceInternal::GC::collectGarbage()
}
//
- // Any instances with a ref count > 0 are referenced from outside the set of class instances (and therefore
- // reachable from the program, for example, via Ptr variable on the stack). Remove these instances
+ // Any instances with a ref count > 0 are referenced from outside the reachable class instances (and
+ // therefore reachable from the program, for example, via Ptr variable on the stack). Remove these instances
// (and all instances reachable from them) from the overall set of objects.
//
{
@@ -230,7 +231,15 @@ IceInternal::GC::collectGarbage()
ObjectCounts::const_iterator i;
for(i = counts.begin(); i != counts.end(); ++i)
{
- i->first->__gcClear(); // Decrement ref count of objects pointed at by this object.
+ //
+ // For classes with members that point at potentially-cyclic instances, __gcClear()
+ // decrements the reference count of those instances and clears the
+ // corrsponding Ptr members.
+ // For classes that cannot be part of a cycle (because they do not contain class members)
+ // and are therefore true leaves, __gcClear() assigns 0 to the corresponding class member,
+ // which either decrements the ref count or, if it reaches zero, deletes the instance as usual.
+ //
+ i->first->__gcClear();
}
for(i = counts.begin(); i != counts.end(); ++i)
{
@@ -255,7 +264,7 @@ IceInternal::GC::collectGarbage()
counts.clear();
{
- Monitor<Mutex>::Lock sync2(*this);
+ Monitor<Mutex>::Lock sync(*this);
_collecting = false;
}
diff --git a/cpp/src/Ice/GCShared.cpp b/cpp/src/Ice/GCShared.cpp
index 7dc7b05c7a9..b5421f89407 100755
--- a/cpp/src/Ice/GCShared.cpp
+++ b/cpp/src/Ice/GCShared.cpp
@@ -73,14 +73,3 @@ IceInternal::GCShared::__setNoDelete(bool b)
_noDelete = b;
gcRecMutex._m->unlock();
}
-
-void
-IceInternal::GCShared::__addObject(GCObjectMultiSet& c, GCShared* p)
-{
- gcRecMutex._m->lock();
- if(p)
- {
- c.insert(p);
- }
- gcRecMutex._m->unlock();
-}
diff --git a/cpp/src/slice2cpp/Gen.cpp b/cpp/src/slice2cpp/Gen.cpp
index a80c8b3946b..813f04fa9e4 100644
--- a/cpp/src/slice2cpp/Gen.cpp
+++ b/cpp/src/slice2cpp/Gen.cpp
@@ -3158,10 +3158,10 @@ Slice::Gen::ObjectVisitor::emitGCFunctions(const ClassDefPtr& p)
//
// A class can potentially be part of a cycle if it (recursively) contains class
- // members. If so, we override __incRef() and __decRef() and, hence, consider instances
- // of the class as candidates for collection by the garbage collector.
- // We override __incRef() and __decRef() only once, in the basemost potentially cyclic class
- // in an inheritance hierarchy.
+ // members. If so, we override __incRef(), __decRef(), and __addObject() and, hence, consider
+ // instances of the class as candidates for collection by the garbage collector.
+ // We override __incRef(), __decRef(), and __addObject() only once, in the basemost potentially
+ // cyclic class in an inheritance hierarchy.
//
bool hasBaseClass = !bases.empty() && !bases.front()->isInterface();
bool canBeCyclic = p->canBeCyclic();
@@ -3224,6 +3224,20 @@ Slice::Gen::ObjectVisitor::emitGCFunctions(const ClassDefPtr& p)
C << nl << "delete this;";
C << eb;
C << eb;
+
+ H << nl << "virtual void __addObject(::IceInternal::GCObjectMultiSet&);";
+
+ C << sp << nl << "void" << nl << scoped.substr(2) << "::__addObject(::IceInternal::GCObjectMultiSet& _c)";
+ C << sb;
+ C << nl << "_c.insert(this);";
+ C << eb;
+
+ H << nl << "virtual bool __usesClasses();";
+
+ C << sp << nl << "bool" << nl << scoped.substr(2) << "::__usesClasses()";
+ C << sb;
+ C << nl << "return true;";
+ C << eb;
}
//
@@ -3279,7 +3293,10 @@ Slice::Gen::ObjectVisitor::emitGCInsertCode(const TypePtr& p, const string& pref
if((BuiltinPtr::dynamicCast(p) && BuiltinPtr::dynamicCast(p)->kind() == Builtin::KindObject)
|| ClassDeclPtr::dynamicCast(p))
{
- C << nl << "__addObject(_c, " << prefix << name << ".get());";
+ C << nl << "if(" << prefix << name << ')';
+ C << sb;
+ C << nl << prefix << name << "->__addObject(_c);";
+ C << eb;
}
else if(StructPtr s = StructPtr::dynamicCast(p))
{
@@ -3330,9 +3347,16 @@ Slice::Gen::ObjectVisitor::emitGCClearCode(const TypePtr& p, const string& prefi
{
C << nl << "if(" << prefix << name << ")";
C << sb;
+ C << nl << "if(" << prefix << name << "->__usesClasses())";
+ C << sb;
C << nl << prefix << name << "->__decRefUnsafe();";
C << nl << prefix << name << ".__clearHandleUnsafe();";
C << eb;
+ C << nl << "else";
+ C << sb;
+ C << nl << prefix << name << " = 0;";
+ C << eb;
+ C << eb;
}
else if(StructPtr s = StructPtr::dynamicCast(p))
{
diff --git a/cpp/test/Ice/gc/Client.cpp b/cpp/test/Ice/gc/Client.cpp
index cf4aa79fb4b..edddb6b4bc2 100644
--- a/cpp/test/Ice/gc/Client.cpp
+++ b/cpp/test/Ice/gc/Client.cpp
@@ -528,6 +528,31 @@ MyApplication::run(int argc, char* argv[])
cout << "testing leaf nodes... " << flush;
{
+ NNPtr nn = new NN;
+ nn->l = new NL;
+ test(getNum() == 2);
+ Ice::collectGarbage();
+ test(getNum() == 2);
+ }
+ Ice::collectGarbage();
+ test(getNum() == 0);
+
+ {
+ NLPtr p;
+ {
+ NNPtr nn = new NN;
+ p = new NL;
+ nn->l = p;
+ test(getNum() == 2);
+ Ice::collectGarbage();
+ test(getNum() == 2);
+ }
+ Ice::collectGarbage();
+ test(getNum() == 1);
+ }
+ test(getNum() == 0);
+
+ {
NNPtr nn = new NN;
NLPtr nl = new NL;
nn->l = nl;
generated by cgit v1.2.3 (git 2.46.0) at 2025-08-05 22:34:40 +0000