Where is the memory leak from my hdc/hbitmap?

Question

so I've noticed that part of my code leaks a lot of memory when it's called and I've tried to find out where or why it leaks but I'm at a dead end.

I've tried the Visual Studio 2017 debugger to take snapshots to find out where the leak happens but according to that there aren't any major leaks. I've also tried Deleaker which I got working once which told me I leaked HDC and HBITMAP but couldn't tell me how much memory.

The first function is the GetScreenBmp where the leak might be, but aren't I releasing everything properly? I know I'm not deleting hBitmap but I need to return that. Is that where the memory leak is?

HBITMAP GetScreenBmp(HDC hdc, int screenPositionX, int screenPositionY, int screenSizeX, int screenSizeY) {
// Get screen dimensions
int nScreenWidth = GetSystemMetrics(SM_CXSCREEN);
int nScreenHeight = GetSystemMetrics(SM_CYSCREEN);
int nMousePositionX = 0, nMousePositionY = 0;

// Create compatible DC, create a compatible bitmap and copy the screen using BitBlt()
HDC hCaptureDC = CreateCompatibleDC(hdc);
HBITMAP hBitmap = CreateCompatibleBitmap(hdc, screenSizeX, screenSizeY);
HGDIOBJ hOld = SelectObject(hCaptureDC, hBitmap);
BOOL bOK = BitBlt(hCaptureDC, 0, 0, screenSizeX, screenSizeY, hdc, screenPositionX, screenPositionY, SRCCOPY | CAPTUREBLT);

SelectObject(hCaptureDC, hOld); // always select the previously selected object once done
DeleteObject(hOld);
DeleteDC(hCaptureDC);

return hBitmap;

The second part is this piece of code, which I'm not entirely sure if I'm deleting everything properly.

HDC hdc = GetDC(0);
    HBITMAP hBitmap = GetScreenBmp(hdc, currentSplitInformationArray.screenPositionX, currentSplitInformationArray.screenPositionY, currentSplitInformationArray.screenSizeX, currentSplitInformationArray.screenSizeY);
    BITMAPINFO MyBMInfo = { 0 };
    MyBMInfo.bmiHeader.biSize = sizeof(MyBMInfo.bmiHeader);

    // Get the BITMAPINFO structure from the bitmap
    if (0 == GetDIBits(hdc, hBitmap, 0, 0, NULL, &MyBMInfo, DIB_RGB_COLORS)) {
        MessageBox(NULL, "Resource not available\nDo you want to try again?", "Account Details", MB_ICONWARNING | MB_CANCELTRYCONTINUE | MB_DEFBUTTON2);
    }

    // create the bitmap buffer
    BYTE* lpPixels = new BYTE[MyBMInfo.bmiHeader.biSizeImage];

    // Better do this here - the original bitmap might have BI_BITFILEDS, which makes it
    // necessary to read the color table - you might not want this.
    MyBMInfo.bmiHeader.biCompression = BI_RGB;
    MyBMInfo.bmiHeader.biHeight = currentSplitInformationArray.screenSizeY * -1;

    // get the actual bitmap buffer
    if (0 == GetDIBits(hdc, hBitmap, 0, currentSplitInformationArray.screenSizeY, (LPVOID)lpPixels, &MyBMInfo, DIB_RGB_COLORS)) {
        MessageBox(NULL, "Resource not available\nDo you want to try again?", "Account Details", MB_ICONWARNING | MB_CANCELTRYCONTINUE | MB_DEFBUTTON2);
    }

    ::SendMessage(testingComparison, STM_SETIMAGE,
        (WPARAM)IMAGE_BITMAP, (LPARAM)hBitmap);

    DeleteObject(&MyBMInfo);
    DeleteObject(hBitmap);
    ReleaseDC(NULL, hdc);
    delete[] lpPixels;

I'm sorry in advance if this is something that has been answered before or if the answer is easily googable, but I've been trying for a few hours to fix it.

`BYTE* lpPixels = new BYTE[MyBMInfo.bmiHeader.biSizeImage];` -- Where is this the `delete [] lpPixels;`? Also, since this is C++, this could simply be `std::vector lpPixels(MyBMInfo.bmiHeader.biSizeImage);` and then use `lpPixels.data()` to point to the buffer. This spares yourself the potential for a memory leak. — PaulMcKenzie, Oct 29 '18 at 11:31
Also, use techniques such as [RAII](https://stackoverflow.com/questions/2321511/what-is-meant-by-resource-acquisition-is-initialization-raii) to release resources automatically when objects go out of scope, instead of having to remember to call `DeleteObject` or `DeleteDC` every time. — PaulMcKenzie, Oct 29 '18 at 11:35
Sorry I forgot about that part, I've updated the post with it. I'm using BYTE* instead of std::vector since in debug mode std::vector has too much extra overhead, since the code runs 60 times a second. I know in release that's removed. I'll check out that RAII. — user9625798, Oct 29 '18 at 11:54
What do you mean by std::vector having "way too much overhead"? If you moved the declaration of that vector outside of that code (hopefully you are using classes), and just call `resize()` instead of `new BYTE[]` each time, the speed of your code would increase since you wouldn't be calling the allocator each and every time. In other words, `vector::resize()` uses memory smarter than the code you have now. — PaulMcKenzie, Oct 29 '18 at 11:59
Also if any of those functions throw an exception, you get a memory leak since `delete [] lpPixels;` never gets called. All parts of RAII. — PaulMcKenzie, Oct 29 '18 at 12:01
What I mean is that when I use the program in debug mode with std::vector it runs around once a second, but if I use BYTE* it runs 60 times a second. Thus I'm assuming that debug mode adds something to std::vector but not BYTE*. Maybe overhead was a bad word to use, sorry. — user9625798, Oct 29 '18 at 14:15
Debug mode is for debugging, and the debug runtime adds iterator checks to the vector and all other containers. When you run an optimized, release version of the code, that is when you will see an increase in speed. Throwing away potential optimizations just to aid debugging speed seems ill-thought out. — PaulMcKenzie, Oct 29 '18 at 16:38
Also (and again), your code will be slower than using `vector` in a release version *if* you write the code so that there is one vector declared *outside* the function, and you call `reserve()` instead of `new/delete` over and over again. If efficiency is your goal, you should see by simple inspection that calling `new/delete` so many times will be a bottleneck. If not vector, allocate a pool of memory once, and keep using that pool of memory. If a new allocation exceeds the pool size, then and only then do you reallocate. That is basically what `vector::reserve` does. — PaulMcKenzie, Oct 29 '18 at 16:38
Hi, just wanted to thank you. I tried replacing BYTE* with std::vector in the code above and it worked flawlessly. So I tried replacing it wherever I used BYTE* and I've stopped a lot of memory leaks and I also get the added benefits of all the vector functions like size(), so once again thank you. (I don't know how to up vote your comments) — user9625798, Oct 30 '18 at 17:57

score 1 · Accepted Answer · answered Oct 29 '18 at 18:01

Okay I found the solution. The STM_SETIMAGE message returns the previous image and you have to handle it yourself. https://learn.microsoft.com/en-us/windows/desktop/Controls/stm-setimage

I should probably learn to better read the documentation next time, sorry for wasting everyone's time with this one.

::SendMessage(testingComparison, STM_SETIMAGE,
            (WPARAM)IMAGE_BITMAP, (LPARAM)hBitmap);

Fixed it by simply doing

HBITMAP oldBitmap = (HBITMAP)::SendMessage(testingComparison, STM_SETIMAGE,
        (WPARAM)IMAGE_BITMAP, (LPARAM)hBitmap);
    DeleteObject(oldBitmap);

score 0 · Answer 2 · answered Oct 29 '18 at 12:13

Use tools to track your leaks/allocations (btw. you didn't post how you found the leaks in the first place).

Since you are using visual studio c++ you may use the built-in tools. Basicly a combination of the these 3 lines can get the job done.

    _CrtSetDbgFlag ( _CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF );//to enable, safe to always set somewhere around program startup - on normal exit this will print whatever you leaked

    //_CrtDumpMemoryLeaks();//Dumps to see what has already been allocated

    //_CrtSetBreakAlloc(#number);//Use this to set breakpoint using the allocation number from heap dump to see where allocation takes place. If allocation happends before this line it will not work.

I noticed that when I debug the program the process memory goes from 0-2Gb in a few seconds and assumed there was a memory leak. I then tried using the heap profiling and taking snapshots which showed I had memory leaks outside of the loop. I then tried Deleaker and I got it working twice and both times it said I leaked HDC and HBITMAP. But I'll try again, maybe I'm just dumb and missed something. — user9625798, Oct 29 '18 at 14:11

Where is the memory leak from my hdc/hbitmap?

2 Answers2