c++ - Memory leak when using IShellItem2.GetString()

Question

I'm using the following code to enumerate the contents of the Recyclebin Shell folder and get the file type of each item. The code gives the expected results but if I call the function in a loop it looks like there's some memory leak when using the IshellItem2 GetString function (see attached screenshot at the end).

• Am I cleaning up everything properly?
• Am I misinterpreting the results?

void Test1()
{
    // Get recyclebin ishellitem
    IShellItem* psiRecycleBin;
    if (SUCCEEDED(SHGetKnownFolderItem(FOLDERID_RecycleBinFolder, KF_FLAG_DEFAULT,
        NULL, IID_PPV_ARGS(&psiRecycleBin))))
    {
        // Get ishellitem contents enumerator
        IEnumShellItems* pesi;
        if (SUCCEEDED(psiRecycleBin->BindToHandler(NULL, BHID_EnumItems, IID_PPV_ARGS(&pesi))))
        {
            IShellItem* psi;
            while (pesi->Next(1, &psi, NULL) == S_OK)
            {
                // Get ishellitem2 from ishellitem
                IShellItem2* psi2;
                if (SUCCEEDED(psi->QueryInterface(IID_PPV_ARGS(&psi2))))
                {
                    // Get the item file type string
                    LPWSTR fileType = NULL;
                    if (SUCCEEDED(psi2->GetString(PKEY_ItemTypeText, &fileType)))
                    {
                        CoTaskMemFree(fileType);
                    }
                    psi2->Release();
                }
                psi->Release();
            }
            pesi->Release();
        }
        psiRecycleBin->Release();
    }
}

And I'm calling it in loop like this:

#define STRICT_TYPED_ITEMIDS

#include <shlobj.h>
#include <propkey.h>
#include <iostream>

void Test1();

int main()
{
    (void)CoInitialize(NULL);

    std::cout << "Enumerating recyclebin items..
";

    for (int ii = 0; ii < 5000; ii++)
    {
        Test1();
    }

    CoUninitialize();
    return 0;
}

When debugging this console program in VS in the memory diagnostics window this is what I get:

Thanks for the help

question from:https://stackoverflow.com/questions/65641797/memory-leak-when-using-ishellitem2-getstring

Answer 1

yes, here really exist memory leak, related to HIDDENRECYCLEBINDATAV2 structure from shell32.dll

partial definition of it:

struct HIDDENRECYCLEBINDATAV2
{
    //... some mebers
    FILETIME time;
    PWSTR pszLocationBeforeDelete = 0; // !!! not released 
    PWSTR pszLocationInRecycleBin = 0; // !!! not released

    HRESULT Serialize(PBYTE *, PUSHORT);

    static HRESULT Deserialize(
        _In_reads_bytes_opt_(cbStream) const BYTE *pbStream , 
        _In_ USHORT cbStream, 
        _Out_ HIDDENRECYCLEBINDATAV2 ** pphrbd);

    static HRESULT Initialize(HIDDENRECYCLEBINDATAV1 const *, HIDDENRECYCLEBINDATAV2**);

};

this structure hold 2 strings - file path from where it deleted ( pszLocationBeforeDelete - this is my name, i don't know original) and current file path in Recycle Bin ( pszLocationInRecycleBin - again my name)

this names allocated inside Deserialize method, by call IStream_ReadStrLong and must be freed with CoTaskMemFree. but how i found - CoTaskMemFree never called for this two strings.

pseudo code for Deserialize :

static HRESULT HIDDENRECYCLEBINDATAV2::Deserialize(
    _In_reads_bytes_opt_(cbInit) const BYTE *pbStream , 
    _In_ USHORT cbStream, 
    _Out_ HIDDENRECYCLEBINDATAV2 ** pphrbd)
{
    HRESULT hr = E_FAIL;

    if (HIDDENRECYCLEBINDATAV2 *phrbd = new HIDDENRECYCLEBINDATAV2)
    {
        if (IStream *pStream = SHCreateMemStream(pbStream, cbStream))
        {
            if (0 <= (hr = IStream_ReadStrLong(pStream, &phrbd->pszLocationBeforeDelete)) &&
                0 <= (hr = IStream_ReadStrLong(pStream, &phrbd->pszLocationInRecycleBin)))
            {
                *pphrbd = phrbd, phrbd = 0;
            }

            pStream->Release();
        }

        CoTaskMemFree(phrbd); // !! error, need delete phrbd
    }
    
    return hr;
}

and it called from CBitBucket::_ValidateItem :

HRESULT InitDeletedItem(PCWSTR pszLocationBeforeDelete, PCWSTR pszLocationBeforeDelete, DELETEDITEM *);

static HRESULT CBitBucket::_ValidateItem(_ITEMIDLIST_RELATIVE const *, DELETEDITEM ** ppdi)
{
    HIDDENRECYCLEBINDATAV2 * phrbd;
    if (0 <= HIDDENRECYCLEBINDATAV2::Deserialize(pbStream, cbStream, &phrbd))
    {
        if (DELETEDITEM * pdi = new DELETEDITEM)
        {
            if (0 <= InitDeletedItem( phrbd->pszLocationBeforeDelete,
                phrbd->pszLocationInRecycleBin, pdi))
            {
                *ppdi = pdi, pdi = 0;
            }

            if (pdi) delete pdi;
        }

        CoTaskMemFree(phrbd); // !! error, need delete phrbd
    }
}

in both functions - memory for HIDDENRECYCLEBINDATAV2 simply released with CoTaskMemFree api, but memory for strings inside this structure not released. i think need add

   HIDDENRECYCLEBINDATAV2::~HIDDENRECYCLEBINDATAV2()
   {
      CoTaskMemFree(pszLocationInRecycleBin);
      CoTaskMemFree(pszLocationBeforeDelete);
   }

to this structure and call delete instead CoTaskMemFree

---

how possible found this ? i hook RtlAllocateHeap and RtlFreeHeap before second call to Test1() (important do this not on first call, because during first call may be additional libs load, some differed initialization, etc.. - all this can distort the real result)and log all alloc/free calls in current thread. also i replace while (pesi->Next..) to if (pesi->Next..) (usually one iteration is enough ). and i found that count of alloc on 2 more than count of free. so i easy found from where this 2 allocations- inside IStream_ReadStrLong. then i set breakpoint here and easy view from where this called :

CBitBucket::_ValidateItem
  HIDDENRECYCLEBINDATAV2::Deserialize
    IStream_ReadStrLong
      CoTaskMemAlloc

partial demo code for log:

struct AI 
{
    PVOID BaseAddress;
    PVOID From;
    ULONG Size;
    ULONG Flags;
};

struct TID 
{
    AI *pi;
    ULONG nAlloc, nFree, nCells, MaxAllocDelta;
    BOOL bNotLog;

    TID()
    {
        RtlZeroMemory(this, sizeof(*this));
    }
};

BOOLEAN NTAPI hook_RtlFreeHeap(PVOID HeapHandle, ULONG Flags, PVOID BaseAddress )
{
    TID* p = RTL_FRAME<TID>::get();

    if (!p || p->bNotLog)
    {
        return RtlFreeHeap(HeapHandle, Flags, BaseAddress) != 0;
    }

    p->bNotLog = TRUE;

    if (!RtlFreeHeap(HeapHandle, Flags, BaseAddress))
    {
        __debugbreak();
    }

    if (BaseAddress)
    {
        AI* pi = p->pi;
        ULONG n = p->nCells;
        do 
        {
            if (pi->BaseAddress == BaseAddress)
            {
                pi->BaseAddress = 0;
                p->nFree++;
                break;
            }
        } while (pi++, --n);

        if (!n)
        {
            __debugbreak();
        }
    }

    p->bNotLog = FALSE;

    return TRUE;
}

PVOID NTAPI hook_RtlAllocateHeap( PVOID HeapHandle, ULONG Flags, SIZE_T Size )
{
    TID* p = RTL_FRAME<TID>::get();

    if (!p || p->bNotLog)
    {
        return RtlAllocateHeap(HeapHandle, Flags, Size);
    }

    p->bNotLog = TRUE;

    if (PVOID BaseAddress = RtlAllocateHeap(HeapHandle, Flags, Size))
    {
        AI* pi = p->pi;
        ULONG n = p->nCells;
        do 
        {
            if (!pi->BaseAddress)
            {
                pi->BaseAddress = BaseAddress;
                pi->From = _ReturnAddress();
                pi->Size = (ULONG)Size;
                pi->Flags = Flags;
                p->nAlloc++;

                ULONG k = p->nAlloc - p->nFree;
                if (k > p->MaxAllocDelta)
                {
                    p->MaxAllocDelta = k;
                }
                break;
            }
        } while (pi++, --n);

        if (!n)
        {
            __debugbreak();
        }

        p->bNotLog = FALSE;

        return BaseAddress;
    }

    return 0;
}

void TestEx()
{
    enum { cell_count = 0x1000 };
    if (AI* pi = new AI[cell_count])
    {
        Test1();// first call
        // hook RtlAllocateHeap + RtlFreeHeap
        {
            RtlZeroMemory(pi, cell_count * sizeof(AI));
            RTL_FRAME<TID> f;
            f.pi = pi;
            f.nCells = cell_count;

            Test1();// second call

            DbgPrint("%x(%x) %x
", f.nAlloc, f.nFree, f.MaxAllocDelta);

            if (f.nAlloc - f.nFree)
            {
                ULONG n = cell_count;
                AI* qi = pi;
                do 
                {
                    if (qi->BaseAddress)
                    {
                        DbgPrint("%p> %x %x
", qi->From, qi->Size, qi->Flags);
                    }
                } while (qi++, --n);
            }
        }
        delete [] pi;
    }
}