[smart_ptr] Problem with circular reference detection
I am trying to detect circular references using the find_unreachable_objects function in sp_collector.cpp. However, it reports false positives in the simple case of one object being owned by another. To illustrate: #define BOOST_SP_ENABLE_DEBUG_HOOKS #include <boost/shared_ptr.hpp> // sp_collector.cpp exported functions std::size_t find_unreachable_objects(bool report); struct X {}; struct Y { boost::shared_ptr<X> pX; Y() : pX(new X) {} }; int main() { boost::shared_ptr<Y> y(new Y); find_unreachable_objects(true); return 0; } The output is as follows: ... 2 objects in m. ... 1 objects in m2. ... 1 objects in open. Unreachable object at 0036BA60, 12 bytes long. Clearly this is incorrect as there are no circular references. Looking at the code, there is what appears to be a typo at line 124: std::cout << "... " << m2.size() << " objects in open.\n"; should be: std::cout << "... " << open.size() << " objects in open.\n"; But the real problem seems to be at line 121: if(p->use_count() != i->second) open.push_back(p); I'm not entirely sure what's going on in this section of code, but it ends up determining (incorrectly) that the "X" object is unreachable. Is there any way to get this working properly? Thanks, - Jim
Jim Barry:
I am trying to detect circular references using the find_unreachable_objects function in sp_collector.cpp. However, it reports false positives in the simple case of one object being owned by another. To illustrate:
#define BOOST_SP_ENABLE_DEBUG_HOOKS #include <boost/shared_ptr.hpp>
// sp_collector.cpp exported functions std::size_t find_unreachable_objects(bool report);
struct X {};
struct Y { boost::shared_ptr<X> pX;
Y() : pX(new X) {} };
int main() { boost::shared_ptr<Y> y(new Y); find_unreachable_objects(true); return 0; }
The output is as follows:
... 2 objects in m. ... 1 objects in m2. ... 1 objects in open. Unreachable object at 0036BA60, 12 bytes long.
Clearly this is incorrect as there are no circular references. Looking at the code, there is what appears to be a typo at line 124:
std::cout << "... " << m2.size() << " objects in open.\n";
should be:
std::cout << "... " << open.size() << " objects in open.\n";
But the real problem seems to be at line 121:
if(p->use_count() != i->second) open.push_back(p);
I'm not entirely sure what's going on in this section of code, but it ends up determining (incorrectly) that the "X" object is unreachable. Is there any way to get this working properly?
I admit that I don't remember what I was thinking when I wrote sp_collector.cpp in 2002-2003, but the following patch does seem to fix the problem with your example. Can you please confirm that it also works for more complicated cases? Index: sp_collector.cpp =================================================================== --- sp_collector.cpp (revision 48405) +++ sp_collector.cpp (working copy) @@ -104,6 +104,8 @@ BOOST_ASSERT(p->use_count() != 0); // there should be no inactive counts in the map + m2[ i->first ]; + scan_and_count(i->second.first, i->second.second, m, m2); } @@ -121,7 +123,7 @@ if(p->use_count() != i->second) open.push_back(p); } - std::cout << "... " << m2.size() << " objects in open.\n"; + std::cout << "... " << open.size() << " objects in open.\n"; for(open_type::iterator j = open.begin(); j != open.end(); ++j) {
Peter Dimov wrote:
I admit that I don't remember what I was thinking when I wrote sp_collector.cpp in 2002-2003, but the following patch does seem to fix the problem with your example. Can you please confirm that it also works for more complicated cases?
Hi Peter, thanks for the swift reply. Your patch seems to have done the trick. And I think I've just about figured out what the code does now (though a few more comments would have helped me along the way). I'll let you know if I find any more problems, but for now it is looking good :-) Cheers, - Jim
What kind of design needs circular smart pointers? I cannot think of anything I would design this way. I usually use normal pointers somewhere to break circular references. These normal pointers get removed on destruction of the object, e.g.: static std::map<std::string, CClass *> s_sNameToObject; The constructor of the CClass would create the entry in the map and the destructor would remove it. Also I use Reference count forwarding together with normal pointers, in case of an entire tree must be kept alive in case of somebody holds a reference to a child object. Also I would try to avoid using smart_ptr, since it must allocate another object (to store the reference count) using operator new. Use the intrusive smart pointer instead. -----Original Message----- From: boost-users-bounces@lists.boost.org [mailto:boost-users-bounces@lists.boost.org] On Behalf Of Jim Barry Sent: Tuesday, September 16, 2008 09:17 To: boost-users@lists.boost.org Subject: [Boost-users] [smart_ptr] Problem with circular reference detection I am trying to detect circular references using the find_unreachable_objects function in sp_collector.cpp. However, it reports false positives in the simple case of one object being owned by another. To illustrate: #define BOOST_SP_ENABLE_DEBUG_HOOKS #include <boost/shared_ptr.hpp> // sp_collector.cpp exported functions std::size_t find_unreachable_objects(bool report); struct X {}; struct Y { boost::shared_ptr<X> pX; Y() : pX(new X) {} }; int main() { boost::shared_ptr<Y> y(new Y); find_unreachable_objects(true); return 0; } The output is as follows: ... 2 objects in m. ... 1 objects in m2. ... 1 objects in open. Unreachable object at 0036BA60, 12 bytes long. Clearly this is incorrect as there are no circular references. Looking at the code, there is what appears to be a typo at line 124: std::cout << "... " << m2.size() << " objects in open.\n"; should be: std::cout << "... " << open.size() << " objects in open.\n"; But the real problem seems to be at line 121: if(p->use_count() != i->second) open.push_back(p); I'm not entirely sure what's going on in this section of code, but it ends up determining (incorrectly) that the "X" object is unreachable. Is there any way to get this working properly? Thanks, - Jim _______________________________________________ Boost-users mailing list Boost-users@lists.boost.org http://lists.boost.org/mailman/listinfo.cgi/boost-users
peter_foelsche@agilent.com wrote:
What kind of design needs circular smart pointers?
I don't want any circular references - that's the point!
I cannot think of anything I would design this way. I usually use normal pointers somewhere to break circular references.
That's what weak_ptr is for. It's superior to a raw pointer because it knows when the pointee has gone away.
Also I would try to avoid using smart_ptr, since it must allocate another object (to store the reference count) using operator new. Use the intrusive smart pointer instead.
I use shared_ptr because I also need weak_ptr. I can live with the overhead. - Jim
participants (3)
-
Jim Barry -
Peter Dimov -
peter_foelsche＠agilent.com