Reference counting is hard.

Raymond Chen

Raymond

One of the big advantages of managed code is that you don’t have
to worry about managing object lifetimes. Here’s an example of
some unmanaged code that tries to manage reference counts and
doesn’t quite get it right.
Even a seemingly-simple function has a reference-counting bug.

template <class T>
T *SetObject(T **ppt, T *ptNew)
{
 if (*ppt) (*ppt)->Release(); // Out with the old
 *ppt = ptNew; // In with the new
 if (ptNew) (ptNew)->AddRef();
 return ptNew;
}

The point of this function is to take a (pointer to)
a variable that points to one object and replace it with
a pointer to another object. This is a function that sits
at the bottom of many “smart pointer” classes. Here’s
an example use:

template <class T>
class SmartPointer {
public:
 SmartPointer(T* p = NULL)
   : m_p(NULL) { *this = p; }
 ~SmartPointer() { *this = NULL; }
 T* operator=(T* p)
   { return SetObject(&m_p, p); }
 operator T*() { return m_p; }
 T** operator&() { return &m_p; }
private:
 T* m_p;
};
void Sample(IStream *pstm)
{
  SmartPointer<IStream> spstm(pstm);
  SmartPointer<IStream> spstmT;
  if (SUCCEEDED(GetBetterStream(&spstmT))) {
   spstm = spstmT;
  }
  ...
}

Oh why am I explaining this? You know how smart pointers work.

Okay, so the question is, what’s the bug here?

Stop reading here and don’t read ahead until you’ve figured it out
or you’re stumped or you’re just too lazy to think about it.


The bug is that the old object is Release()d before the new object
is AddRef()’d. Consider:

  SmartPointer<IStream> spstm;
  CreateStream(&spstm);
  spstm = spstm;

This assignment statement looks harmless (albeit wasteful).
But is it?

The “smart pointer” is constructed with NULL,
then the CreateStream creates a stream and assigns it to
the “smart pointer”. The stream’s reference count is now one.
Now the assignment statement is executed, which turns into

 SetObject(&spstm.m_p, spstm.m_p);

Inside the SetObject function, ppt points tp spstm.m_p, and
pptNew equals the original value of spstm.m_p.

The first thing that SetObject does is release the old pointer,
which now drops the reference count of the stream to zero.
This destroys the stream object.
Then the ptNew parameter (which now points to a freed object)
is assigned to spstm.m_p, and finally the ptNew pointer
(which still points to a freed object) is AddRef()d.
Oops, we’re invoking a method on an object that has been freed;
no good can come of that.

If you’re lucky, the AddRef() call crashes brilliantly so
you can debug the crash and see your error.
If you’re not lucky (and you’re usually not lucky),
the AddRef() call interprets the freed memory as if it were
still valid and increments a reference count somewhere inside
that block of memory. Congratulations, you’ve now corrupted memory.
If that’s not enough to induce a crash (at some unspecified point
in the future), when the “smart pointer”
goes out of scope or otherwise changes its referent, the invalid
m_p pointer will be Release()d, corrupting memory yet another time.

This is why “smart pointer” assignment functions must AddRef() the
incoming pointer before Release()ing the old pointer.

template <class T>
T *SetObject(T **ppt, T *ptNew)
{
 if (ptNew) (ptNew)->AddRef();
 if (*ppt) (*ppt)->Release();
 *ppt = ptNew;
 return ptNew;
}

If you look at the source code for

the ATL function AtlComPtrAssign
,
you can see that it exactly matches the above (corrected) function.

[Raymond is currently on vacation; this message was pre-recorded.]

Raymond Chen
Raymond Chen

Follow Raymond   

0 comments

Comments are closed.