Bit by void*

Recently I got bit by void* again because of another C++ quirk I didn't think through.  I had a class which wrapped a void* which could be one of many different structs.  The structs were POD and didn't have any shared functionality hence I didn't bother creating an inheritance hierarchy.  Unfortunately I defined the structs like so

 class C1 {
  struct S1 {
    int field1;
    float field2;
  };
  struct S2 {
    char field1;
  };
  ~C1() {
    delete m_pData;
  }
  void* m_pData; // Can be S1,S2,etc ...
}

Unfortunately this appeared to work fine for quite some time.  Then after a couple of days of bug fixes I ended up with a memory leak which I quickly tracked down to a leaked COM object.  Although C1 was at fault I didn't suspect any changes to this class because after all it was working fine for some time and all I did was add a new field to one of the structs.  If the structs were being successfully free'd before a new field shouldn't change anything.

The field I added was of type CComPtr<T> which exposed a greater problem in my code.  Even though I properly delete the pointer in C1::~C1() I wasn't running the destructor on the pointed at data and instead I was just freeing the memory.  Until I added a field which had a non-trivial destructor this wasn't a problem (still a bug though). 

Why did this happen?  By deleting a void* and expecting a destructor to run what I'm really doing is asking C++ to behave polymorphicly.  C++ as a rule won't behave this way unless it is specifically asked to with inheritance and virtual.   In the case of void*, it just won't.  The fix is to actually implement an inheritance hierarchy which supports polymorphism.

It's just another rule that I need to remember when coding C++. 

Deleting void* is dangerous, period.

Unfortunately C++ has too many of these rules and not enough enforcement.

Comments

• Anonymous
  June 16, 2008
  PingBack from http://blog.a-foton.ru/2008/06/16/bit-by-void/

• Anonymous
  June 16, 2008
  Boost's ptr_* containers are based on things like void* pointers. They pay great attention to the detail of managing void*, and can be a good set as to how to proceed. I am wondering whether you let implicit casting get in the way, and in that case, not having defined a proper inheritance hierarchy or virtual destructors, this could pose a problem. I wonder what was the flow leading to your leak there.

• Anonymous
  June 16, 2008
  The basic flow for the problem was both a lack of virtual destructors and an inheritance hierarchy.   I haven't had time to play with the boost ptr's yet but I'm interested in how they approach void*.  I was under the impression most of them were templated so they could avoid void.  

• Anonymous
  June 16, 2008
  Yeah the are using void* inside a std::vector for example, even increasing void* deletion complexity further. Pretty much what happens here: template<class T, class CloneAllocator = heap_clone_allocator,class Allocator= std::allocator<void*> > class ptr_vector : public ptr_sequence_adapter<T, std::vector<void*,Allocator>,                              CloneAllocator> { /.../ } They carefully derive from the stl containers as well, which is adding further on the points to look after. I found it in their documentation here: http://www.boost.org/doc/libs/1_35_0/libs/ptr_container/doc/ptr_vector.html void* pointer use is yes, one of the evil points. By the way, I like your blog a lot! Keep up the good work :)

• Anonymous
  June 16, 2008
  oops delete the derivation comment, slightly more twisted than that :P

• Anonymous
  June 17, 2008
  I'll need to dive into that implemantation a bit.  It looks interesting.   Glad you enjoy the blog.