Mike Stall talks about “Don’t do complicated work in Release()”. He has a sample implementation for IUnkown::Release().

 

<quote>

 

An object’s destructor has the same caveats if Release() is often implemented as:

 

InterlockedDecrement(&m_ref)

if (m_ref == 0) { delete this; }

 

</quote>

 

There is a bug in this implementation, that m_ref is accessed after InterlockedDecrement. The problem is the object may have been deleted after InterlockedDecrement. Thus access violation exception may occur when m_ref is accessd after the memory has been reclaimed.

 

Imagine the object’s ref count is 2. There are two threads calling Release() simultaneously.

 

Thread 1                                                                      Thread 2

 

InterlockedDecrement(&m_ref);

//m_ref now is 1

           

                                                                                    InterlockedDecrement(&m_ref);

                                                                                    // m_ref now is 0

                                                                                    if (m_ref==0) {delete this;}

                                                                                    // now the object is gone

 

// access violation here, since the object is gone

if (m_ref==0) {delete this;}

 

The right implementation is

 

if (!InterlockedDecrement(&m_ref)) {delete this;}

 

We have found a few occurrences of this pattern in CLR’s code base. This bug is revealed in our internal ASP.NET stress. We have fixed all of them in CLR’s codebase since.

 

Devil is in the details.