Hello,
Squid calls cbdataFree() for objects that have non-POD data members
such as refcounted pointers. Since cbdataFree() does not call the object
destructor when freeing object memory, those data members are not
properly destroyed, leading to leaks (at best). Some code tries to reset
those pointers before calling cbdataFree(), but, naturally, there are
quite a few places were we fail to cleanup. The other places are just
ticking bombs until somebody adds a sensitive non-POD data member. See
bug #3133 for an example.
The attached untested patch tries to fix the bugs I could identify, but
I wonder whether it would be better to remove cbdataAlloc/Free and call
new/delete instead (to properly execute constructors and destructors)?
Such a rewrite would require adding CBDATA_CLASS2 macros to some
structs, but it will be much safer than trying to find all such bugs and
to monitor that new ones do not crop up (especially where a non-POD
member is added indirectly -- to a type used by a previously POD class
member; for example String added to Ip::Address used by many
cbdataFree()d types).
I think this transition can be done more-or-less safely by first
adjusting cbdataAlloc/Free() macros so that they fail to compile if the
corresponding class does not have a special member added by
CBDATA_CLASS2 macros. This way, we will not forget to add CBDATA_CLASS2
to any class/struct that uses cbdataFree(). cbdataFree() also sets the
pointer to NULL so we would still have to manually check that it is not
needed if we want to remove that macro.
Any better ideas?
Thank you,
Alex.
P.S. I am also concerned what happens when cbdataReferenceDone() reaches
zero lock counter for a non-POD object. Will need to get some sleep
before researching that :-).
This archive was generated by hypermail 2.2.0 : Sat Jul 07 2012 - 12:00:02 MDT