Почему пример кода обращается к удаленной памяти в IUnknown? - PullRequest
Новая
       78

Почему пример кода обращается к удаленной памяти в IUnknown?

1 голос
/ 14 июля 2020

Довольно много примеров использования интерфейсов, таких как IUnknown, в этом примере IDocHostUIHandler, но на самом деле это не имеет значения - используйте код, подобный этому: 

 class TDocHostUIHandlerImpl : public IDocHostUIHandler
 {
 private:

    ULONG RefCount;

    public:      

    TDocHostUIHandlerImpl():RefCount(0){ }

    // IUnknown Method
    HRESULT __stdcall QueryInterface(REFIID riid, void **ppv) {
        if (IsEqualIID(riid,IID_IUnknown))
            {
            *ppv = static_cast<IUnknown*>(this);
            return S_OK;
            }
        else if (IsEqualIID(riid, IID_IDocHostUIHandler)) {
            *ppv = static_cast<IDocHostUIHandler*>(this);
            return S_OK;
            }
        else {
            *ppv = NULL;
            return E_NOINTERFACE;
            }
        }

    ULONG   __stdcall AddRef() {
        InterlockedIncrement((long*)&RefCount);
        return RefCount;
        }

    ULONG   __stdcall Release() {
        if (InterlockedDecrement((long*)&RefCount) == 0) delete this;
        return RefCount;
        }

Моя проблема здесь в с методом Release(), который удаляет реализацию интерфейса с помощью delete this, но сразу после этого делает return RefCount, который больше не относится к действительному объекту в памяти (он обращается к удаленной памяти).

Я бы предположил, что это должно быть что-то вроде 

    ULONG   __stdcall Release() {
        if (InterlockedDecrement((long*)&RefCount) == 0) { delete this; return 0; }
        return RefCount;
        }

Что также не запускает инструмент утечки ресурсов, который я использую (Codeguard в C ++ Builder). Так почему же тогда так много примеров используют первую версию, чего мне здесь не хватает?

Или это просто случай, когда "удалить это" вызывается в другом компиляторе, таком как Visual Studio, после завершения метода Release ?

Несколько примеров:

https://www.codeproject.com/Articles/4758/How-to-customize-the-context-menus-of-a-WebBrowser

addref и release в IUnknown, что они на самом деле делают?

https://bbs.csdn.net/topics/20135139

1 Ответ

2 голосов
/ 14 июля 2020

Да, вы правы, что такие примеры плохо написаны. Они должны быть написаны так, как вы описали: 

ULONG __stdcall Release()
{
    if (InterlockedDecrement((long*)&RefCount) == 0) {
        delete this;
        return 0;
    }
    return RefCount;
}

Однако для них лучше просто вернуть любой результат, возвращаемый InterlockedDecrement. Как отметил @RaymondChen в комментариях, это также решает проблему уменьшения RefCount другим потоком, потенциально уничтожая this, до достижения return, например: 

ULONG __stdcall Release()
{
    ULONG res = (ULONG) InterlockedDecrement((long*)&RefCount);
    if (res == 0) {
        delete this;
    }
    return res;
}

То же самое с AddRef(), если на то пошло: 

ULONG __stdcall AddRef()
{
    return (ULONG) InterlockedIncrement((long*)&RefCount);
}

Кстати, пример QueryInterface(), который вы показали, также написан неправильно, поскольку он не увеличивает RefCount при возврате S_OK, например: 

HRESULT __stdcall QueryInterface(REFIID riid, void **ppv)
{
    if (IsEqualIID(riid,IID_IUnknown)) {
        *ppv = static_cast<IUnknown*>(this);
        AddRef(); // <-- add this!
        return S_OK;
    }
    else if (IsEqualIID(riid, IID_IDocHostUIHandler)) {
        *ppv = static_cast<IDocHostUIHandler*>(this);
        AddRef(); // <-- add this!
        return S_OK;
    }
    else {
        *ppv = NULL;
        return E_NOINTERFACE;
    }
}

Что обычно может будет проще написать так: 

HRESULT __stdcall QueryInterface(REFIID riid, void **ppv)
{
    if (!ppv) {
        return E_POINTER;
    }

    if (IsEqualIID(riid, IID_IUnknown)) {
        *ppv = static_cast<IUnknown*>(this);
    }
    else if (IsEqualIID(riid, IID_IDocHostUIHandler)) {
        *ppv = static_cast<IDocHostUIHandler*>(this);
    }
    else {
        *ppv = NULL;
        return E_NOINTERFACE;
    }

    AddRef();
    return S_OK;
}

Я также видел, как это написано так, что объясняет плохие случаи, когда QueryInterface() вызывается на указателе NULL: 

HRESULT __stdcall QueryInterface(REFIID riid, void **ppv)
{
    if (!ppv) {
        return E_POINTER;
    }

    if (IsEqualIID(riid, IID_IUnknown)) {
        *ppv = static_cast<IUnknown*>(this);
    }
    else if (IsEqualIID(riid, IID_IDocHostUIHandler)) {
        *ppv = static_cast<IDocHostUIHandler*>(this);
    }
    else {
        *ppv = NULL;
    }

    if (*ppv) {
        AddRef();
        return S_OK;
    }

    return E_NOINTERFACE;
}
...