3

When we write 156KB worth of key:value pairs to the WMI using IWbemQualifierSet::Put this method creates 2GB of dangling allocations. I've never encountered a bug like this in a Microsoft library so I'm not really sure what to do about it.

Is there some way we can mitigate it by manipulating the call stack? Or by pre-allocating the memory for the class in some kind of quarantine container to make sure it gets completely freed after the class object is destroyed?

You can skip to "Step 3" in the code to see the leak, it occurs irrespective of data type or format, including using only C types and manually managing the memory.

Update: Even if we create and destroy the class object in the loop, the leak still occurs.

Update: The size of the leak is the same irrespective of input data size. Even if we write strings that are 4000 bytes long, or 1 byte long, the amount of memory leaked is the same.

main.cpp:

// MemorLeakMinimalExample.cpp : This file contains the 'main' function. Program execution begins and ends there.
#include <string>
#include <Windows.h>
#include "wmi_connection.h"

// Memory leak in this method
// Consumes 2GB of RAM to write 32MB of data

int main()
{

    // ------------------------------------------------------
    // Step 1. Connect to WMI -------------------------------

    wmi_connection wmi;
    wmi.connect_to_wmi();


    // ------------------------------------------------------
    // Step 2. Create a new WMI class -----------------------

    IWbemClassObject* pNewClass = NULL;
    VARIANT vVariant;
    VariantInit(&vVariant);

    // Our new class
    HRESULT hres = wmi.pSvc->GetObject(
        0,
        0,
        NULL,
        &pNewClass,
        NULL
    );

    // Set class name
    vVariant.vt = VT_BSTR;
    vVariant.bstrVal = SysAllocString(L"TestClass");
    hres = pNewClass->Put(
        _bstr_t("__CLASS"),
        0,
        &vVariant,
        0
    );

    // Create the "Test" key property for the new class
    BSTR KeyProp = SysAllocString(L"TestProperty");

    hres = pNewClass->Put(
        KeyProp,
        0,
        NULL,
        CIM_STRING);

    //Attach the Key standard qualifier to the test property
    IWbemQualifierSet* pQualSet = NULL;
    vVariant.vt = VT_BOOL;
    vVariant.boolVal = TRUE;
    hres = pNewClass->GetPropertyQualifierSet(KeyProp, &pQualSet);
    hres = pQualSet->Put(L"Key", &vVariant, 0);
    

    // --------------------------------------------------------------------
    // Step 3. Write 20,000 key:value pairs -------------------------------
    
    // Key
    wchar_t key[32] = { 0 };
    wchar_t* pKey = key;
    
    // Value
    VARIANT value;
    VariantInit(&value);
    value.vt = VT_BSTR;

    // Value is always 1234
    value.bstrVal = SysAllocString(L"1234");
    

    printf("Breakpoint\n"); // Inspect memory usage here

    
    for (unsigned long long i = 0; i <= 20000; i++)
    {
        // Key name is formatted "key0", "key1", "key2", ...
        swprintf_s(pKey, 32, L"key%llu", i);

        // Write a key:value pair
        pQualSet->Put(pKey, &value, 0); // <-- memory leak
        
    }
    
    printf("Breakpoint\n"); // Inspect memory usage here

    // --------------------------------------------------------------------
    // // Step 4. Save class for viewing in sys utility Wbemtest.exe ------

    // Uncomment if you want the class written to the wmi database
    // (its exactly the size it should be for our key:value data in)

    /*
    hres = wmi.pSvc->PutClass(pNewClass, WBEM_FLAG_CREATE_OR_UPDATE, NULL, NULL);
    if (FAILED(hres))
    {
        printf("Failed to save class changes. Error code = 0x%X \n", hres);
    }
    */

    return 0;
}

wmi_connection.h:

#pragma once
#include <Windows.h>
#include <comdef.h>
#include <wbemcli.h>

class wmi_connection
{
public:
    // uninteresting parts
    int connect_to_wmi()
    {
        // Initialize 
        hres = CoInitializeEx(0, COINIT_MULTITHREADED);
        if (FAILED(hres))
        {
            printf("Failed to initialize COM library. Error code = 0x%llx", (unsigned long long)hres);
            return FALSE;                  // Program has failed.
        }
        // Set general COM security levels 
        hres = CoInitializeSecurity(
            NULL,
            -1,                          // COM authentication
            NULL,                        // Authentication services
            NULL,                        // Reserved
            RPC_C_AUTHN_LEVEL_DEFAULT,   // Default authentication 
            RPC_C_IMP_LEVEL_IMPERSONATE, // Default Impersonation  
            NULL,                        // Authentication info
            EOAC_NONE,                   // Additional capabilities 
            NULL                         // Reserved
        );

        // Get the class factory for the WbemLocator object
        IClassFactory* pClassFactory = NULL;

        hres = CoGetClassObject(CSLSID_WbemLocator, CLSCTX_INPROC_SERVER, NULL, SIID_IClassFactory, (void**)&pClassFactory);

        // Create an instance of the WbemLocator object
        IUnknown* pUnk = NULL;

        hres = pClassFactory->CreateInstance(NULL, SIID_IUnknown, (void**)&pUnk);

        hres = pUnk->QueryInterface(SIID_IWbemLocator, (void**)&pLoc);

        pUnk->Release();
        pClassFactory->Release();

        // Set security levels on the proxy 
        hres = CoSetProxyBlanket(
            pSvc,                        // Indicates the proxy to set
            RPC_C_AUTHN_WINNT,           // RPC_C_AUTHN_xxx
            RPC_C_AUTHZ_NONE,            // RPC_C_AUTHZ_xxx
            NULL,                        // Server principal name 
            RPC_C_AUTHN_LEVEL_CALL,      // RPC_C_AUTHN_LEVEL_xxx 
            RPC_C_IMP_LEVEL_IMPERSONATE, // RPC_C_IMP_LEVEL_xxx
            NULL,                        // client identity
            EOAC_NONE                    // proxy capabilities 
        );

        // Connect to WMI through the IWbemLocator::ConnectServer method
        hres = pLoc->ConnectServer(
            _bstr_t(L"root\\cimv2"), // Object path of WMI namespace
            NULL,                    // User name
            NULL,                    // User password
            0,                       // Locale
            NULL,                    // Security flags
            0,                       // Authority
            0,                       // Context object 
            &pSvc                    // pointer to IWbemServices proxy
        );

        if (FAILED(hres))
        {
            printf("Could not connect. Error code = 0x%lx\n", hres);
            return 1;
        }

        printf("Connected to root\cimv2 WMI namespace\n");
        return 0;
    }

    IWbemServices* pSvc;

private:
    HRESULT hres = NULL;
    IWbemLocator* pLoc = NULL;
    GUID CSLSID_WbemLocator = { 0x4590f811, 0x1d3a, 0x11d0, 0x89, 0x1f, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24 };
    GUID SIID_IClassFactory = { 0x00000001, 0x0000, 0x0000, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46 };
    GUID SIID_IUnknown = { 0x00000000, 0x0000, 0x0000, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46 };
    GUID SIID_IWbemLocator = { 0xdc12a687, 0x737f, 0x11cf, 0x88, 0x4d, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24 };
};
Pulpo
  • 224
  • 2
  • 12
  • Beside sketching a poor man's fix, best is to report it. But you'll have to wait for a fix then... – aybe Jan 17 '23 at 04:11
  • I don't anticipate my client taking "maybe they'll fix it in the next 10 years" for an answer. A poor man's fix is more or less what I need, do you have any suggestions (broadly) for how I might go about that? If I need to make child processes I will, although I'd really prefer something internal.. – Pulpo Jan 17 '23 at 04:54
  • Doing it from a child process would be the easiest solution I think. Are you sure it's actually leaking committed memory? – Jonathan Potter Jan 17 '23 at 06:50
  • 2
    There's no memory leak here, you're just adding 20000 items to an in-memory dictionary (`pQualSet`). Call `pQualSet->Release()` and `pNewClass->Release()` and memory will be freed. PS: the code as you show it has lots of memory leak (no call to VariantClear, no call to SysFreeString, no call to pLoc->Release etc.) – Simon Mourier Jan 17 '23 at 07:34
  • `VARIANT`s are hard to manage. Try using [`_variant_t`](https://learn.microsoft.com/en-us/cpp/cpp/variant-t-class) instead. If nothing else it'll properly set a `VT_BOOL` value to `VARIANT_TRUE` (which is distinctly [different from `TRUE`](https://devblogs.microsoft.com/oldnewthing/20041222-00/?p=36923)). – IInspectable Jan 17 '23 at 08:24
  • @SimonMourier I assure you that I removed a lot of error handling and minor management of single-use variables for the sake of the example being more readable, and that even if you `Release()` them that memory is not freed because it has escaped management within the library. – Pulpo Jan 17 '23 at 10:10
  • @IInspectable that's incredibly useful information, thank you – Pulpo Jan 17 '23 at 10:12
  • I've tested it and memory *is* released. Your loop just adds data to a dictionary, it's normal memory increases, it won't be freed until you call release on related objects, there's nothing to "mitigate". – Simon Mourier Jan 17 '23 at 10:12
  • You've missed something then. Put the entire program in a loop and call the appropriate `Release()`s for the two objects in `main` and a destructor for `wmi_connection` and it will consume 64GB of RAM in less than 30 seconds. – Pulpo Jan 17 '23 at 10:20
  • I also see memory being freed when releasing the IWbemClassObject. Either you are missing something in the actual code (update it to ensure you actually are releasing everything) or something on your local system is interfering. – Luke Jan 17 '23 at 10:56
  • It's you who miss something. I do see lots of memory used but it's *normal*. You're just asking pQualSet to add something to it's (internal) dictionary, so it allocates memory for these 20000 objects. You just need to release the two objects I mentioned to release this allocated memory. – Simon Mourier Jan 17 '23 at 11:24
  • @SimonMourier The answer revealed the issue. Each call to `Put` places a new dictionary object inside the class without overwriting or destroying the previous one. And after a certain threshold of placement, the memory allocated for the old objects is never freed when `IWbemQualifierSet` is destroyed. Calling it a memory leak or bad code is potato-potato. – Pulpo Jan 17 '23 at 21:28
  • 1
    I'm not convinced that this is true. The proposed answer never attempts to identify the issue, and the solution may just be hiding the issue that's still there. There's a private class member in `wmi_connection` (`pLoc `) that owns resources, but the class doesn't have a d'tor to release them. If this is your actual code then it is leaking resources, and you should address that. – IInspectable Jan 18 '23 at 08:26
  • As I said, it is a significantly cut down example for the sake of S.O users being able to read it easily. If you are truly interested I will provide you with a complete PoC – Pulpo Jan 18 '23 at 08:54
  • 2
    A Q&A is wildly useless without a [mcve]. Trimming it down to the bare minimum is good. Substantially changing it (such as omitting a d'tor) isn't. If there is a d'tor, it needs to be in the code here. If there isn't, you have a resource leak. – IInspectable Jan 18 '23 at 11:39

1 Answers1

-2

I turned i into 40000000. I found that the memory footprint has indeed been increasing from Task Manager. And show this continuously. enter image description here

I added Sleep(50);in the “for” loop, “Exception thrown….” disappeared. But the memory footprint still was increasing. The process memory increased to 565MB in 6mins. enter image description here

There is a conflict between the “for” loop with Put(pKey, &value, 0);

So, using multithreading is necessary.

I modified the code, it runs well. No effect on the loop and process memory is normal. enter image description here

Here is the code, it still has many flaws, and you are welcome to correct them.

#include <string>
#include <Windows.h>
#include "wmi_connection.h"
wchar_t key[32] = { 0 };
wchar_t* pKey = key;
BSTR KeyProp = SysAllocString(L"TestProperty");
IWbemQualifierSet* pQualSet = NULL;
VARIANT value;
DWORD WINAPI ThreadProc1(LPVOID lpParameter)
{
    wmi_connection wmi;
    wmi.connect_to_wmi();
    IWbemClassObject* pNewClass = NULL;
    VARIANT vVariant;
    VariantInit(&vVariant);
    HRESULT hres = wmi.pSvc->GetObject(
        0,
        0,
        NULL,
        &pNewClass,
        NULL
    );
    vVariant.vt = VT_BSTR;
    vVariant.bstrVal = SysAllocString(L"TestClass");
    hres = pNewClass->Put(
        _bstr_t("__CLASS"),
        0,
        &vVariant,
        0
    );
    hres = pNewClass->Put(
        KeyProp,
        0,
        NULL,
        CIM_STRING);
    vVariant.vt = VT_BOOL;
    vVariant.boolVal = TRUE;
    hres = pNewClass->GetPropertyQualifierSet(KeyProp, &pQualSet);
    hres = pQualSet->Put(L"Key", &vVariant, 0);

    VARIANT value;
    VariantInit(&value);
    value.vt = VT_BSTR;

    value.bstrVal = SysAllocString(L"1234");
    printf("Breakpoint\n");
    pQualSet->Put(pKey, &value, 0);
   
    return 2;
}
DWORD WINAPI ThreadProc2(LPVOID lpParameter)
{
    for (unsigned long long i = 0; i <= 400000000; i++)

    {
           
        swprintf_s(pKey, 32, L"key%llu", i);
    }
    printf("Breakpoint\n");
    return 0;
}
int main()
{
    wmi_connection wmi;
    wmi.connect_to_wmi();

    IWbemClassObject* pNewClass = NULL;
    VARIANT vVariant;
    VariantInit(&vVariant);

    // Our new class
    HRESULT hres = wmi.pSvc->GetObject(
        0,
        0,
        NULL,
        &pNewClass,
        NULL
    );

    vVariant.vt = VT_BSTR;
    vVariant.bstrVal = SysAllocString(L"TestClass");
    hres = pNewClass->Put(
        _bstr_t("__CLASS"),
        0,
        &vVariant,
        0
    );
    BSTR KeyProp = SysAllocString(L"TestProperty");

    hres = pNewClass->Put(
        KeyProp,
        0,
        NULL,
        CIM_STRING);

    IWbemQualifierSet* pQualSet = NULL;
    vVariant.vt = VT_BOOL;
    vVariant.boolVal = TRUE;
    hres = pNewClass->GetPropertyQualifierSet(KeyProp, &pQualSet);
    hres = pQualSet->Put(L"Key", &vVariant, 0);

    HANDLE hThread[2];
    DWORD dWResult1;
    DWORD dWResult2;
    hThread[0] = CreateThread(NULL, 0, ThreadProc1, NULL, 0, NULL);
    hThread[1] = CreateThread(NULL, 0, ThreadProc2, NULL, 0, NULL);

    WaitForMultipleObjects(2, hThread, true, INFINITE);
    GetExitCodeThread(hThread[0], &dWResult1);
    GetExitCodeThread(hThread[1], &dWResult2);
    printf("Thread execution completed \n");
    getchar();
    CloseHandle(hThread[0]);
    CloseHandle(hThread[1]);
    SysFreeString((BSTR) KeyProp);//add

    printf("Breakpoint\n"); 
    
    return 0;
}
  • 1
    *"it may have other reasons. So, using multithreading is necessary"* - If you don't know what's wrong, how can you be confident that the proposed solution addresses an *unknown* issue? – IInspectable Jan 17 '23 at 12:02