Quiz: What's wrong with this code?

What's wrong with the following code? This is real code taken verbatim from Cordbg (see src\debug\shell\dshell.cpp in the rotor sources), except that I removed preprocessor goo to make it more readable. All the interfaces are COM-classic interfaces derived from IUnknown. You don't need to be familiar with ICorDebug to see the problem here (that's hint #1 - it's not related to some subtle API misuse), though I give some background data below for the curious. Here's a hint #2: Ever notice memory leaks in Cordbg?

 //
// Strip all references off of the given value. This simply
// dereferences through references until it hits a non-reference
// value.
// 
// On input *ppValue is an ICorDebugValue for some arbitrary variable.
// On output, *ppValue is update to yield an ICorDebugValue which is 
// the furthest down the dereference chain. 
//
// The caller requires no specific knowledge of what the *ppValue passed in is. 
// The caller must have a reference to the incoming *ppValue (which this function will release)
// and must release the outgoing *ppValue (which this function would addref).
//
HRESULT DebuggerShell::StripReferences(ICorDebugValue **ppValue,
                                       bool printAsYouGo)
{
    HRESULT hr = S_OK;
    
    while (TRUE)
    {
        ICorDebugReferenceValue *reference;
        hr = (*ppValue)->QueryInterface(IID_ICorDebugReferenceValue, 
                                        (void **) &reference);
 
        if (FAILED(hr))
        {
            hr = S_OK;
            break;
        }
 
        // Check for NULL
        BOOL isNull;
        hr = reference->IsNull(&isNull);
 
        if (FAILED(hr))
        {
            reference->Release();
            (*ppValue)->Release();
            *ppValue = NULL;
            break;
        }
 
        if (isNull)
        {
            if (printAsYouGo)
                Write(L"<null>");
            
            reference->Release();
            (*ppValue)->Release();
            *ppValue = NULL;
            break;
        }
 
        CORDB_ADDRESS realObjectPtr;
        hr = reference->GetValue(&realObjectPtr);
 
        if (FAILED(hr))
        {
            reference->Release();
            (*ppValue)->Release();
            *ppValue = NULL;
            break;
        }
 
        // Dereference the thing...
        ICorDebugValue *newValue;
        hr = reference->Dereference(&newValue);
            
        if (hr != S_OK)
        {
            if (printAsYouGo)
                if (hr == CORDBG_E_BAD_REFERENCE_VALUE)
                    Write(L"<invalid reference: 0x%p>", realObjectPtr);
                else if (hr == CORDBG_E_CLASS_NOT_LOADED)
                    Write(L"(0x%p) Note: CLR error -- referenced class "
                          L"not loaded.", realObjectPtr);
                else if (hr == CORDBG_S_VALUE_POINTS_TO_VOID)
                    Write(L"0x%p", realObjectPtr);
 
            (reference)->Release();
            ((*ppValue))->Release();
            *ppValue = NULL;
            break;
        }
 
        if (printAsYouGo)
            Write(L"(0x%08x) ", realObjectPtr);
        
        (reference)->Release();
 
        ((*ppValue))->Release();
        *ppValue = newValue;
    }
 
    return hr;
}

Here's an explanation of the identifiers involved:

  • ICorDebugValue is the debugger's representation of a variable.
  • If that variable is a reference value, then it also implements ICorDebugReferenceValue.
  • Here's what the following methods on ICorDebugReferenceValue mean:
    • IsNull(out BOOL) - returns Boolean if the reference value in the debuggee is null.
    • GetValue(out CORDB_ADDRESS) - get the address in the debuggee of what the reference points to.
    • Dereference(out ICorDebugValue) - get an ICorDebugValue describing the variable that this reference points to.