Gotcha: CComAutoCriticalSection and copy constructors

While investigating a crash during a suite run I found the stack walk included the destructor for a CComAutoCriticalSection.  This is a fairly reliable class so I immediately suspected my code.  I did a couple of quick checks for a double free and didn’t find any.  Then I looked a little closer at CComAutoCriticalSection and spotted that it doesn’t redefine the standard copy and operator= constructor.

This is a red flag for any class that implements RAII.  C++ will automatically define a copy/operator= for your clasess which will amount to a memcpy.  This means copies of two objects will be managing the same resource.  Once the second one is destroyed it will result in a double free and hopefully a crash. 

I walked the hierarchy and discovered that none of the base classes redefined these methods either.  After that it only took a few minutes to discover the bug.  I attempt to pass the object by reference but instead ended up passing it by value. 

To ensure I didn’t miss any other cases of this, I added the following definition to my code base and moved all instances of CComAutoCriticalSection to point to this version. 

class SafeCriticalSection : public CComAutoCriticalSection
    SafeCriticalSection() {}
    SafeCriticalSection(const SafeCriticalSection&);
    SafeCriticalSection& operator=(const SafeCriticalSection&);

It should be standard practice to define all 3 of the above methods on any class which has RAII semantics.  This is not necessary if all of the members are copy safe (CComPtr<> for example).

Comments (0)