Spot the Bug - IMFOutputSchema

Spot the Bug - IMFOutputSchema

  • Comments 2

I found a simple but nasty bug the other day in this implementation of the IMFOutputSchema interface.

The symptoms: running this code outside of a debugger caused the app to crash.  Running it under a debugger (to catch the crash) caused the app to run clean.

// outputschema.h
HRESULT CreateTrustedAudioDriversOutputSchema(
    DWORD dwConfigData,
    GUID guidOriginatorID,
    IMFOutputSchema **ppMFOutputSchema
);
 

// outputschema.cpp
// ... various include files removed ...
// CMFAttributesImpl implements the IMFAttributes interface, minus the IUnknown methods
class CTrustedAudioDriversOutputSchema : public CMFAttributesImpl<IMFOutputSchema> {

friend
    HRESULT CreateTrustedAudioDriversOutputSchema(
        DWORD dwConfigData,
        GUID guidOriginatorID,
        IMFOutputSchema **ppMFOutputSchema
    );

private:
    CTrustedAudioDriversOutputSchema(DWORD dwConfigData, GUID guidOriginatorID);
    ~CTrustedAudioDriversOutputSchema();

    ULONG m_cRefCount;
    DWORD m_dwConfigData;
    GUID m_guidOriginatorID;
    
public:
    // IUnknown methods
    HRESULT STDMETHODCALLTYPE QueryInterface(
       /* [in] */ REFIID riid,
       /* [out] */ LPVOID *ppvObject
    );
    ULONG STDMETHODCALLTYPE AddRef();
    ULONG STDMETHODCALLTYPE Release();

    // IMFOutputSchema methods
    HRESULT STDMETHODCALLTYPE GetConfigurationData(__out DWORD *pdwVal);
    HRESULT STDMETHODCALLTYPE GetOriginatorID(__out GUID *pguidOriginatorID);
    HRESULT STDMETHODCALLTYPE GetSchemaType(__out GUID *pguidSchemaType);

}; // CTrustedAudioDriversOutputSchema

HRESULT CreateTrustedAudioDriversOutputSchema(
    DWORD dwConfigData,
    GUID guidOriginatorID,
    IMFOutputSchema **ppMFOutputSchema
) {
    if (NULL == ppMFOutputSchema) {
        return E_POINTER;
    }

    *ppMFOutputSchema = NULL;

    CTrustedAudioDriversOutputSchema *pSchema =
        new CTrustedAudioDriversOutputSchema(dwConfigData, guidOriginatorID);

    if (NULL == pSchema) {
        LOG(eError, _T("new CTrustedAudioDriversOutputSchema returned a NULL pointer"));
        return E_OUTOFMEMORY;
    }

    *ppMFOutputSchema = static_cast<IMFOutputSchema *>(pSchema);

    return S_OK;
} // CreateTrustedAudioDriversOutputSchema

// constructor
CTrustedAudioDriversOutputSchema::CTrustedAudioDriversOutputSchema(
    DWORD dwConfigData, GUID guidOriginatorID
)
: m_dwConfigData(dwConfigData)
, m_guidOriginatorID(guidOriginatorID)
{}

// destructor
CTrustedAudioDriversOutputSchema::~CTrustedAudioDriversOutputSchema() {}

#define RETURN_INTERFACE(T, iid, ppOut) \
    if (IsEqualIID(__uuidof(T), (iid))) { \
        this->AddRef(); \
        *(ppOut) = static_cast<T *>(this); \
        return S_OK; \
    } else {} (void)0

// IUnknown::QueryInterface
HRESULT STDMETHODCALLTYPE
    CTrustedAudioDriversOutputSchema::QueryInterface(
        /* [in] */ REFIID riid,
        /* [out] */ LPVOID *ppvObject
) {

    if (NULL == ppvObject) {
        return E_POINTER;
    }

    *ppvObject = NULL;

    RETURN_INTERFACE(IUnknown, riid, ppvObject);
    RETURN_INTERFACE(IMFAttributes, riid, ppvObject);
    RETURN_INTERFACE(IMFOutputSchema, riid, ppvObject);

    return E_NOINTERFACE;
}

// IUnknown::AddRef
ULONG STDMETHODCALLTYPE
    CTrustedAudioDriversOutputSchema::AddRef() {

    ULONG uNewRefCount = InterlockedIncrement(&m_cRefCount);

    return uNewRefCount;
}

// IUnknown::Release
ULONG STDMETHODCALLTYPE
    CTrustedAudioDriversOutputSchema::Release() {

    ULONG uNewRefCount = InterlockedDecrement(&m_cRefCount);

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

    return uNewRefCount;
}

// IMFOutputSchema::GetConfigurationData
HRESULT STDMETHODCALLTYPE
    CTrustedAudioDriversOutputSchema::GetConfigurationData(__out DWORD *pdwVal) {

    LOG(eInfo1, _T("IMFOutputSchema::GetConfigurationData called"));

    if (NULL == pdwVal) { return E_POINTER; }

    *pdwVal = m_dwConfigData;
    
    return S_OK;
}

// IMFOutputSchema::GetOriginatorID
HRESULT STDMETHODCALLTYPE
    CTrustedAudioDriversOutputSchema::GetOriginatorID(__out GUID *pguidOriginatorID) {

    LOG(eInfo1, _T("IMFOutputSchema::GetOriginatorID called"));

    if (NULL == pguidOriginatorID) { return E_POINTER; }

    *pguidOriginatorID = m_guidOriginatorID;
    
    return S_OK;
}

// IMFOutputSchema::GetSchemaType
HRESULT STDMETHODCALLTYPE
    CTrustedAudioDriversOutputSchema::GetSchemaType(__out GUID *pguidSchemaType) {

    LOG(eInfo1, _T("IMFOutputSchema::GetSchemaType called"));

    if (NULL == pguidSchemaType) { return E_POINTER; }

    *pguidSchemaType = MFPROTECTION_TRUSTEDAUDIODRIVERS;
    
    return S_OK;
}

Leave a Comment
  • Please add 6 and 2 and type the answer here:
  • Post
  • Without much review, I am thinking QueryInterface should initialize m_cRefCount.  It does not appear that the private reference count is every properly initialized.

  • Bingo.  I fixed this by updating the initializer list in the constructor:

    // constructor

    CTrustedAudioDriversOutputSchema::CTrustedAudioDriversOutputSchema(

       DWORD dwConfigData, GUID guidOriginatorID

    )

    : m_cRefCount(1)

    , m_dwConfigData(dwConfigData)

    , m_guidOriginatorID(guidOriginatorID)

    {}

    There's more than one way to do refcount initialization.  I could equally well have initialized the refcount to 0 and done an AddRef() in the factory function.

    When I ran under the debugger, the debugger initialized m_cRefCount with garbage memory, so the refcount never dropped to zero; when I ran outside of the debugger, m_cRefCount was initialized to 0, so the "delete this;" in Release() was called one Release() too soon.

Page 1 of 1 (2 items)