Brushes deleteObject(hBrush) in c++

user_3847487 0
Viewed 383

I'm a little confused. In general, should new Brushes be deleted after usage? I'm worried about leaking memory each time I run the following code :

    case WM_CTLCOLORLISTBOX:
{
    
    HBRUSH hbrush;
    
    hbrush = CreateSolidBrush(RGB(255, 255, 0));    // Green BackColor in ComboBox List area

    HWND hTest = (HWND)lParam;
    COMBOBOXINFO ci = { sizeof(COMBOBOXINFO) };

    GetComboBoxInfo(hComboMode, &ci);       // CBS_DROPDOWNLIST 

    if (hTest == ci.hwndItem || hTest == ci.hwndList)
    {
        HDC hdc = (HDC)wParam;

        SetTextColor(hdc, RGB(0, 0, 0));    // Black Text in ComboBox List area
        
        SetBkMode(hdc, TRANSPARENT);
        return (INT_PTR)hbrush;
    }

    // DeleteObject(hbrush);
}

Two things baffle me. First, if control goes into the if statement then, as far as I can see, as soon as the return statement executes, there is no opportunity to execute the deleteObject(hbrush) at all? The case statement is exited right at that point? Then, if that's the case, then I'll never be in a position to delete the brush since I need to pass it back as part of the return statement? BTW, this code is a "hodge/podge" of different pieces of code I found here, on SO. I need to add that, in the original code, the first line read "static HBRUSH hBrush". I purposely remove the word 'static' for the purpose of the discussion, here. Please forgive my ignorance but all I know is that 'static' will create the variable on the heap -- and not on the stack, and subsequent execution of the code will find the variable in the same state over and over again; it is not created dynamically each time as I have it now.

Where I'm confused is, if the variable is made 'static' then do I still need to 'delete' it at some point? Since it's created statically, there will not be "memory leaks"?

Sorry for the confusion. I have not been programming in c++ for a long time. I'm slowly learning the ropes. Thanks in advance for your kind help and guidance.

combobox memory-leaks winapi windows c++
1 Answers

The WM_CTLCOLORLISTBOX message is designed to leak. The system expects an implementation to produce a GDI object and hand it out. There is no callback to inform the owner of that resource when the system is done using it.

You do not get to choose whether a WM_CTLCOLORLISTBOX message handler leaks. It will, always, by design. Your only choice is whether you want to prevent that leak from causing issues due to resource exhaustion1.

The implementation provided in the question decided to make the leak hurt. It creates a new brush object for every WM_CTLCOLORLISTBOX message sent to the dialog procedure, without ever cleaning up. The system will eventually run out of GDI resources.

There are a number of ways to solve this:

Delegating responsibility

The easiest way to solve this issue is to delegate responsibility for cleanup to someone else. The system maintains a set of frequently used GDI objects accessible through GetStockObject. Since "it is not necessary to delete stock objects" there aren't any responsibilities on your end beyond returning a stock object.

But... there are only black, and gray, and white brushes provided as stock objects. What if I need a green brush? Luckily, the system has you covered. You just need to put two pieces together: Returning a DC_BRUSH and requesting the desired color through SetDCBrushColor:

    if (hTest == ci.hwndItem || hTest == ci.hwndList)
    {
        auto const hdc{ reinterpret_cast<HDC>(wParam) };

        SetTextColor(hdc, RGB(0, 0, 0));    // Black Text in ComboBox List area
        SetBkMode(hdc, TRANSPARENT);

        // Set solid brush color
        ::SetDCBrushColor(hdc, RGB(0, 255, 0));
        // Request the system to use the DC_BRUSH
        return reinterpret_cast<INT_PTR>(::GetStockObject(DC_BRUSH));
    }

Placing an upper bound on the leak count and/or duration

The previous solution is great for when the system provides everything you need already (like a solid color brush). If you do need something more complex (e.g. a pattern brush), responsibility for object creation and destruction is back on you.

Again, there's a simple solution already mentioned in the question: Using an object with static storage duration. An object with static storage duration is initialized at most once, making it a perfect fit for here. While it doesn't prevent the leak, it limits the count of leaked objects to no more than 1:

case WM_CTLCOLORLISTBOX:
{
    // Make sure this gets created at most once
    static auto const brush{::CreateSolidBrush(RGB(0, 255, 0))};

    // ...
    
    return reinterpret_cast<INT_PTR>(brush);
}

The net effect is that, no matter how many times the window procedure receives a WM_CTLCOLORLISTBOX request2, the brush object is created at most once. This places an upper bound of 1 on the leak count, thereby preventing resource exhaustion due to unbounded resource comsumption.

Similarly, it is possible to confine the duration of the leak. While you don't know the exact point in time when the system is done using the brush, you can be sure that it's safe to dispose off of it when the control's parent window is destroyed. A parent window receives a WM_NCDESTROY message "after the child windows have been destroyed".

Implementing this scheme is rather involved. It requires a fair bit of setup, such as associating data with either the window class instances or the window classes, plus state management. Broadly speaking, this solution allocates resources on demand (e.g. the first time a WM_CTLCOLORLISTBOX message arrives), and frees them when the final client has been destroyed. Getting this all fleshed out warrants a whole new Q&A.

Diagnostics

GDI resources are limited. Being conservative is crucial, and learning when you aren't is required. A quick way to verify that GDI resource usage doesn't increase over time is to use Task Manager. On the Details tab, right-click on the list-view's header control, choose "Select columns" and tick the "GDI objects" option.

If you do run into issues and want to pin-point the root cause, the OS provides the GetGuiResources API, allowing you to observe the resource count at various places in your code.

1 The system maintains a surprisingly low global (per user session) limit on GDI objects.
2 This holds true, even if the window procedure is used by more than one window instance.

user_18893290
Related