Re: [RFC] cbdataFree does not call destructor

lör 2012-07-07 klockan 00:04 -0600 skrev Alex Rousskov:

> 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).

I had the impression that the CBDATA_CLASS controlled classes should
have their destructors called on free?

Are you talking about the old C struct based ones? Those should go.

> 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)?

The cbdata interface do not fit entirely to new/delete. The difference
is that cbdataFree only marks objects as "to be freed". There may still
be reference counts keeping it alive.

But this said cbdata really SHOULD get replaced by type hiding refcount
+ invalidation. Moving to CBDATA_CLASS macros is a good step towards
that.

> 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?

As said above.. refcount instead of explicit counts.

> 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 :-).

Hmm... yes it's odd.

Looking at CBDATA_CLASS2 it does NOT provide the original cbdata
interface where the object is destructed when last reference is gone
AFTER free. Here the object is destructed on delete but not freed until
last cbdata reference is gone.

The interface provided by CBDATA_CLASS actually requires that any
references are anonymous not knowing their target type, making sure that
they do not attempt to access the object.

Regards
Henrik
Received on Sat Jul 07 2012 - 10:30:49 MDT